fix: support $ref to definitions of shared schemas - #952
Merged
Merged
Conversation
Tony133
force-pushed
the
fix/639-shared-schema-definitions
branch
from
September 20, 2026 10:21
3073a0b to
6ad810b
Compare
$ref to definitions of shared schemas
Tony133
force-pushed
the
fix/639-shared-schema-definitions
branch
from
September 20, 2026 10:35
6ad810b to
a9d73ac
Compare
Signed-off-by: Antonio Tripodi <Tony133@users.noreply.github.com>
Tony133
marked this pull request as ready for review
September 21, 2026 19:42
Fdawgs
reviewed
Sep 22, 2026
Fdawgs
reviewed
Sep 22, 2026
mcollina
requested changes
Sep 25, 2026
mcollina
left a comment
Member
There was a problem hiding this comment.
I think we are walking the schema tree multiple times, and this is a relatively expensive operation. Could you minimize?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposal:
Swagger 2.0 and OpenAPI do not allow the
definitionskeyword inside a schema object, so the plugin drops it from shared schemas. The$refs pointing into it were left untouched though, producing dangling references such as#/definitions/def-0/definitions/foo.Moreover, the exact example of #639 (a nested
$id: '#address') currently throwsCannot redefine property: Symbol(json-schema-resolver.refToDef), because the shared schemas are mapped again asexternalSchemason everyresolve()call and their subschemas are evaluated against the wrong base URI.This PR follows the approach suggested in the review of #676: instead of moving
definitionssomewhere else inside the schema, every nested definition is hoisted to the top-leveldefinitions/components.schemas, and the references are rewritten through a map.http://foo/common.json#/definitions/foo#/components/schemas/def-0/definitions/foo(missing)#/components/schemas/def-0-foohttp://foo/common.json#address#/components/schemas/def-0address(missing)#/components/schemas/def-0-fooIn detail:
definitionsand$defsare hoisted as<schema>-<key>, at any depth (definitions nested in definitions, or in a property subschema); name collisions get a numeric suffix;$refs of the final document are rewritten with a longest-prefix match, so.../definitions/foo/properties/citykeeps its trailing pointer;#/definitions/foo,#/properties/bar,#) are made absolute before the resolution, since#becomes the root of the whole document once the schema lands there;http://foo/common.json#address, or#addressfrom inside the shared schema) are converted to the JSON pointer of the anchored subschema before the resolution, since the ref resolver appends the fragment to the definition name as it is (def-0address). This is the form used by the Fastify "Fluent Schema" guide, mentioned in the comments of JSON Schema Definitions not work #639. The route schemas are cloned, not mutated, and only when at least one anchor exists;$ids are then removed from the cloned shared schemas: nothing points to them anymore, the ref resolver would list each of them as a duplicated definition (def-1), and Swagger 2.0 does not accept a nested$id;externalSchemas, which fixes the crash.The walker is schema-aware: a property named
definitions, or data insideenum/default/examples, is not mistaken for a keyword. The final rewrite walk only runs when something has been hoisted.Known limitations:
$id/definitionsare not covered (that is Recursive type definition using Typebox in schema leads to broken swagger file #865, it can build on this);<$id of the shared schema>#anchor: a relative reference that must be resolved against a different base URI is not recognized.Note:
definitions, nested definitions,$defs, local and recursive refs, name collisions and JSON pointer escaping. Generated documents are validated withswagger-parser.definitions/$defsof shared schemas, describing the<schema>-<key>naming and how to reference an anchor.Fixes #639
Refs #675, #676