Skip to content

Commit d7e45f9

Browse files
dinwwwhdinwwwh
authored
fix(json-schema): keep sibling union branches that share a $defs entry (#1927)
Sibling `anyOf`/`oneOf` branches that reference the same `$defs` entry no longer collapse into one when a union is flattened. `flattenJsonUnionSchemaInternal` tracked in-progress `$ref`s in a Set it mutated in place, and since `Set.prototype.add` returns the same set, a `$ref` being resolved leaked out of its own recursion path and persisted into later sibling branches. Any branch pointing at an entry an earlier sibling had already touched hit the cycle guard and was silently dropped. Copying the set per recursion path scopes the guard to the path actually being walked, matching what the sibling function `extractJsonObjectSchemaEntriesInternal` already does. ## Fixes The reported case now keeps both branches with their own keywords instead of returning only the first: ```ts flattenJsonUnionSchema({ $defs: { Shared: { type: 'string' } }, anyOf: [ { $ref: '#/$defs/Shared', minLength: 2 }, { $ref: '#/$defs/Shared', maxLength: 5 }, ], }) ``` Downstream, OpenAPI documents generated from unions whose members share a `$ref` no longer lose members: detailed output schemas keep every response status, and request/response bodies keep every variant. Cycle detection stays intact and still terminates, including on mutually recursive `$defs`. ## Performance The in-place mutation was acting as accidental global memoization, so flattening is now exponential in union nesting depth in the worst case. This only bites on shapes far outside normal contracts, because flattening recurses through `anyOf`/`oneOf` and top-level `$ref` only, never into `properties` or `items`. A union nested 7 deep with 3 branches per level measures ~20 ms; a synthetic depth-20 chain of unions-of-unions takes seconds. Generation happens at build/startup on developer-authored schemas, and the previous cost was paid in silently wrong output, so the trade seemed worth making rather than keeping a faster incorrect traversal. Worth noting for anyone who revisits this: a memo would have to be keyed on the resolved definition *plus* the path-relevant ref set. Keying on schema identity alone would collapse legitimately distinct branches and reintroduce exactly this bug. ## Testing Three regression tests, each confirmed to fail before the change and pass after: the reported repro, a shared `$defs` entry that is itself a union (2 branches to 4), and mutual `A` to `B` recursion, which also pins that cycle detection stays path-scoped and terminating. Full suite (3251 tests), `pnpm type:check` and `pnpm lint` pass. Closes #1926
1 parent 0c7ec80 commit d7e45f9

2 files changed

Lines changed: 50 additions & 1 deletion

File tree

‎packages/json-schema/src/composition-utils.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,38 @@ describe('flattenJsonUnionSchema', () => {
839839
])
840840
})
841841

842+
it('flattens every sibling branch that references the same $defs entry', () => {
843+
const schema = {
844+
$defs: { Shared: { type: 'string' } },
845+
anyOf: [
846+
{ $ref: '#/$defs/Shared', minLength: 2 },
847+
{ $ref: '#/$defs/Shared', maxLength: 5 },
848+
],
849+
} satisfies JsonSchema
850+
851+
expect(flattenJsonUnionSchema(schema)).toEqual([
852+
{ $defs: schema.$defs, $ref: '#/$defs/Shared', minLength: 2 },
853+
{ $defs: schema.$defs, $ref: '#/$defs/Shared', maxLength: 5 },
854+
])
855+
})
856+
857+
it('flattens every sibling branch that references the same union $defs entry', () => {
858+
const schema = {
859+
$defs: { Shared: { anyOf: [{ type: 'number' }, { type: 'boolean' }] } },
860+
anyOf: [
861+
{ $ref: '#/$defs/Shared', description: 'first' },
862+
{ $ref: '#/$defs/Shared', description: 'second' },
863+
],
864+
} satisfies JsonSchema
865+
866+
expect(flattenJsonUnionSchema(schema)).toEqual([
867+
{ $defs: schema.$defs, description: 'first', type: 'number' },
868+
{ $defs: schema.$defs, description: 'first', type: 'boolean' },
869+
{ $defs: schema.$defs, description: 'second', type: 'number' },
870+
{ $defs: schema.$defs, description: 'second', type: 'boolean' },
871+
])
872+
})
873+
842874
it('flattens transitive local $ref union branches', () => {
843875
const schema = {
844876
$defs: {
@@ -882,6 +914,23 @@ describe('flattenJsonUnionSchema', () => {
882914
])
883915
})
884916

917+
it('flattening mutually recursive union $ref branches', () => {
918+
const schema = {
919+
$defs: {
920+
A: { anyOf: [{ $ref: '#/$defs/B' }, { type: 'string' }] },
921+
B: { anyOf: [{ $ref: '#/$defs/A' }, { type: 'number' }] },
922+
},
923+
anyOf: [{ $ref: '#/$defs/A' }, { $ref: '#/$defs/B' }],
924+
} satisfies JsonSchema
925+
926+
expect(flattenJsonUnionSchema(schema)).toEqual([
927+
{ $defs: schema.$defs, $ref: '#/$defs/B' },
928+
{ $defs: schema.$defs, type: 'string' },
929+
{ $defs: schema.$defs, $ref: '#/$defs/A' },
930+
{ $defs: schema.$defs, type: 'number' },
931+
])
932+
})
933+
885934
it('dedupe json schemas result', () => {
886935
const schema = {
887936
$defs: { Shared: { anyOf: [{ type: 'string' }, { type: 'boolean' }] } },

‎packages/json-schema/src/composition-utils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ function flattenJsonUnionSchemaInternal(
380380
const resolved = resolveJsonSchemaRootLocalRef(schema)
381381

382382
if (resolved !== schema) {
383-
const result = flattenJsonUnionSchemaInternal(resolved, resolvingRefs.add(schema.$ref))
383+
const result = flattenJsonUnionSchemaInternal(resolved, new Set(resolvingRefs).add(schema.$ref))
384384
if (result.length > 1) {
385385
return result
386386
}

0 commit comments

Comments
 (0)