feat(reference): add Overlay parsers, derefer and resolve strategies#189
feat(reference): add Overlay parsers, derefer and resolve strategies#189
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds first-class Overlay 1.x support to @speclynx/apidom-reference by introducing Overlay JSON/YAML parsers plus Overlay-specific resolve/dereference strategies (including extends handling), and wiring them into the saturated configuration with accompanying tests and documentation.
Changes:
- Added
overlay-json-1andoverlay-yaml-1parsers with optional parsing of the Overlayextendstarget. - Added
overlay-1dereference + resolve strategies, including optional dereferencing of theextendstarget (and deduplication when parse-phase already produced an extends result). - Updated saturated configuration, package exports/deps, docs, and tests (including snapshots); also improved Arazzo dereference to avoid duplicate source-description results.
Reviewed changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/apidom-reference/test/resolve/strategies/overlay-1/extends/index.ts | Adds resolve strategy tests for Overlay extends behavior. |
| packages/apidom-reference/test/resolve/strategies/overlay-1/extends/fixtures/overlay.json | Overlay fixture with extends for resolve tests. |
| packages/apidom-reference/test/resolve/strategies/overlay-1/extends/fixtures/overlay-no-extends.json | Overlay fixture without extends for resolve tests. |
| packages/apidom-reference/test/resolve/strategies/overlay-1/extends/fixtures/openapi.json | OpenAPI fixture used as an Overlay extends target. |
| packages/apidom-reference/test/parse/parsers/overlay-yaml-1/index.ts | Adds Overlay YAML parser test coverage including extends parsing + metadata. |
| packages/apidom-reference/test/parse/parsers/overlay-yaml-1/fixtures/sample-overlay.yaml | Sample Overlay YAML fixture for parser tests. |
| packages/apidom-reference/test/parse/parsers/overlay-yaml-1/fixtures/extends/overlay.yaml | Overlay YAML fixture with extends for parser tests. |
| packages/apidom-reference/test/parse/parsers/overlay-yaml-1/fixtures/extends/openapi.yaml | OpenAPI YAML fixture used as an Overlay YAML extends target. |
| packages/apidom-reference/test/parse/parsers/overlay-json-1/index.ts | Adds Overlay JSON parser test coverage including extends parsing + metadata. |
| packages/apidom-reference/test/parse/parsers/overlay-json-1/fixtures/sample-overlay.json | Sample Overlay JSON fixture for parser tests. |
| packages/apidom-reference/test/parse/parsers/overlay-json-1/fixtures/extends/overlay.json | Overlay JSON fixture with extends for parser tests. |
| packages/apidom-reference/test/parse/parsers/overlay-json-1/fixtures/extends/openapi.json | OpenAPI JSON fixture used as an Overlay JSON extends target. |
| packages/apidom-reference/test/dereference/strategies/overlay-1/extends/index.ts | Adds Overlay dereference tests for extends, metadata attachment, and snapshot of dereferenced $refs. |
| packages/apidom-reference/test/dereference/strategies/overlay-1/extends/fixtures/overlay.json | Overlay fixture with extends for dereference tests. |
| packages/apidom-reference/test/dereference/strategies/overlay-1/extends/fixtures/overlay-no-extends.json | Overlay fixture without extends for dereference tests. |
| packages/apidom-reference/test/dereference/strategies/overlay-1/extends/fixtures/openapi.json | OpenAPI fixture containing $ref used for dereference snapshot validation. |
| packages/apidom-reference/test/dereference/strategies/overlay-1/extends/snapshots/index.mjs.snap | Snapshot of dereferenced extends target document $ref resolution. |
| packages/apidom-reference/test/dereference/strategies/arazzo-1/source-descriptions/index.ts | Adds snapshot coverage and duplicate-prevention test for Arazzo sourceDescriptions dereference. |
| packages/apidom-reference/test/dereference/strategies/arazzo-1/source-descriptions/snapshots/index.mjs.snap | Snapshot for dereferenced Arazzo source description document. |
| packages/apidom-reference/src/resolve/strategies/overlay-1/index.ts | Introduces Overlay resolve strategy delegating to Overlay dereference to build ReferenceSet. |
| packages/apidom-reference/src/parse/parsers/overlay-yaml-1/index.ts | Introduces Overlay YAML parser wrapper + optional extends parsing hook. |
| packages/apidom-reference/src/parse/parsers/overlay-yaml-1/extends.ts | Re-exports format-agnostic parseExtends logic for YAML parser. |
| packages/apidom-reference/src/parse/parsers/overlay-json-1/index.ts | Introduces Overlay JSON parser wrapper + optional extends parsing hook. |
| packages/apidom-reference/src/parse/parsers/overlay-json-1/extends.ts | Implements parseExtends logic to parse and attach an Overlay extends target parse result. |
| packages/apidom-reference/src/dereference/strategies/overlay-1/index.ts | Introduces Overlay dereference strategy with optional extends dereferencing. |
| packages/apidom-reference/src/dereference/strategies/overlay-1/extends.ts | Implements dereferenceExtends, including reuse of parse-phase result and duplicate avoidance. |
| packages/apidom-reference/src/dereference/strategies/arazzo-1/index.ts | Prevents duplicate source-description results when both parse + dereference phases are enabled. |
| packages/apidom-reference/src/configuration/saturated.ts | Wires Overlay parsers/strategies into the default saturated configuration. |
| packages/apidom-reference/README.md | Documents new Overlay parsers and Overlay resolve/dereference strategy options and behavior. |
| packages/apidom-reference/package.json | Exposes new entrypoints for Overlay parsers/strategies and adds Overlay dependencies. |
| packages/apidom-parser-adapter-overlay-yaml-1/README.md | Expands adapter documentation (API, mediaTypes, detect, usage). |
| packages/apidom-parser-adapter-overlay-json-1/README.md | Expands adapter documentation (API, mediaTypes, detect, usage). |
| package-lock.json | Lockfile update reflecting dependency graph changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const annotation = new AnnotationElement( | ||
| `Error parsing extends target "${retrievalURI}": ${message}`, | ||
| ); | ||
| annotation.classes.push('error'); |
There was a problem hiding this comment.
In the error path, an AnnotationElement is pushed to the overlay parseResult, but nothing is attached to the extends element meta. This makes it hard for consumers to correlate the failure back to the extends field and it diverges from patterns like parseSourceDescriptions (which always sets meta.parseResult even on failure). Consider setting extendsElement.meta.set('parseResult', <ParseResultElement containing the annotation>) (or clearing it) in the catch branch so callers can always inspect extendsElement.meta.get('parseResult') for success/failure details.
| annotation.classes.push('error'); | |
| annotation.classes.push('error'); | |
| // wrap the error annotation in a ParseResultElement and attach it to extends element meta | |
| const errorParseResult = new ParseResultElement(); | |
| errorParseResult.classes.push('extends'); | |
| errorParseResult.setMetaProperty('retrievalURI', retrievalURI); | |
| errorParseResult.push(annotation); | |
| extendsElement.meta.set('parseResult', errorParseResult); | |
| // also push the annotation to the overlay ParseResult for backward compatibility |
| `Error dereferencing extends target "${retrievalURI}": ${message}`, | ||
| ); | ||
| annotation.classes.push('error'); | ||
| parseResult.push(annotation); |
There was a problem hiding this comment.
If dereferencing the extends target fails, the catch branch only appends an AnnotationElement to the overlay parseResult and leaves extendsElement.meta.parseResult untouched. When the parse phase already attached a parseResult, this can leave stale (non-dereferenced) data in extendsElement.meta.get('parseResult') while also emitting an error annotation. Consider setting/overwriting extendsElement.meta.parseResult to a ParseResultElement that contains the error annotation (and optionally removing any existing 'extends' ParseResultElement entries) so consumers have a single, consistent place to look for the extends dereference outcome.
| parseResult.push(annotation); | |
| // wrap error annotation in a ParseResultElement and attach it to extendsElement.meta | |
| const errorParseResult = new ParseResultElement([annotation]); | |
| errorParseResult.classes.push('extends'); | |
| errorParseResult.setMetaProperty('retrievalURI', retrievalURI); | |
| extendsElement.meta.set('parseResult', errorParseResult); | |
| // ensure overlay parseResult only contains the error extends result | |
| const cleanedOnError = parseResult.reject( | |
| (item) => isParseResultElement(item) && includesClasses(item, ['extends']), | |
| ); | |
| parseResult.content = cleanedOnError.content; | |
| parseResult.push(errorParseResult); |
| A `ParseResultElement` is also attached to the extends element's meta as `'parseResult'`. | ||
|
|
||
| ##### Error handling | ||
|
|
||
| The extends parsing uses annotations instead of throwing errors, allowing parsing to continue | ||
| even when the target document cannot be resolved. Errors are reported as `AnnotationElement` instances | ||
| with an `'error'` class within the parse result. |
There was a problem hiding this comment.
The docs state that a ParseResultElement is attached to the extends element meta as parseResult, but the current implementation only sets this on success; on failures it pushes an AnnotationElement into the outer ParseResult without attaching anything to extends.meta. Please clarify the error-case behavior here (either document that extends.meta.parseResult is only present on success, or update the implementation to always attach a ParseResultElement containing error annotations).
| A `ParseResultElement` is also attached to the extends element's meta as `'parseResult'`. | |
| ##### Error handling | |
| The extends parsing uses annotations instead of throwing errors, allowing parsing to continue | |
| even when the target document cannot be resolved. Errors are reported as `AnnotationElement` instances | |
| with an `'error'` class within the parse result. | |
| When the extends target document is successfully resolved and parsed, a `ParseResultElement` is also attached to the extends element's meta as `'parseResult'`. | |
| ##### Error handling | |
| The extends parsing uses annotations instead of throwing errors, allowing parsing to continue | |
| even when the target document cannot be resolved or parsed. | |
| On failure, no `parseResult` is attached to the extends element's meta; instead, errors are reported as `AnnotationElement` instances with an `'error'` class within the outer parse result. |
No description provided.