Skip to content

Commit ceb9775

Browse files
pgraugpgraug
committed
fix: explicit discriminator mapping wins over fallback in nested allOf
When a schema extends a parent whose allOf contains both a $ref schema (with a discriminator that doesn't list the child) and an inline schema (with a discriminator that does explicitly map the child), the generated type was getting the wrong $type literal — the schema name fallback from the $ref discriminator — instead of the value from the explicit inline mapping. The root cause is that findDiscriminatorsInSchema traverses allOf entries in order, so the $ref schema's discriminator is seen first. Since the child isn't in that mapping, discriminatorValues falls back to the schema name, marks the property name as seen, and the correct inline discriminator is then skipped as a duplicate. Fix: track whether each collected discriminator value came from an explicit mapping match or a schema-name fallback. If a later discriminator for the same property has an explicit match while the existing one is only a fallback, replace it. Fallback-only cases (schema genuinely not in any mapping) are unaffected. Applied to both the 3.0.x and 3.1.x parsers, with a minimal reproduction spec and snapshot test for each.
1 parent ef871e4 commit ceb9775

10 files changed

Lines changed: 367 additions & 55 deletions

File tree

‎packages/openapi-ts-tests/main/test/3.0.x.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,14 @@ describe(`OpenAPI ${version}`, () => {
217217
}),
218218
description: 'handles nested allOf with discriminators',
219219
},
220+
{
221+
config: createConfig({
222+
input: 'discriminator-allof-inline.json',
223+
output: 'discriminator-allof-inline',
224+
}),
225+
description:
226+
'handles allOf where inline schema discriminator mapping should take priority over $ref discriminator fallback',
227+
},
220228
{
221229
config: createConfig({
222230
input: 'discriminator-non-string.yaml',

‎packages/openapi-ts-tests/main/test/3.1.x.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,14 @@ describe(`OpenAPI ${version}`, () => {
243243
}),
244244
description: 'handles nested allOf with discriminators',
245245
},
246+
{
247+
config: createConfig({
248+
input: 'discriminator-allof-inline.json',
249+
output: 'discriminator-allof-inline',
250+
}),
251+
description:
252+
'handles allOf where inline schema discriminator mapping should take priority over $ref discriminator fallback',
253+
},
246254
{
247255
config: createConfig({
248256
input: 'discriminator-non-string.yaml',

‎packages/openapi-ts-tests/main/test/__snapshots__/3.0.x/discriminator-allof-inline/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// This file is auto-generated by @hey-api/openapi-ts
2+
3+
export type { Bar, Baz, ClientOptions, Foo, GetFoosData, GetFoosResponse, GetFoosResponses, Qux } from './types.gen';

‎packages/openapi-ts-tests/main/test/__snapshots__/3.0.x/discriminator-allof-inline/types.gen.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// This file is auto-generated by @hey-api/openapi-ts
2+
3+
export type ClientOptions = {
4+
baseUrl: `${string}://${string}` | (string & {});
5+
};
6+
7+
export type Foo = {
8+
$type: string;
9+
foo?: string;
10+
};
11+
12+
export type Bar = Omit<Foo, '$type'> & {
13+
$type: 'FooBar';
14+
bar?: string;
15+
};
16+
17+
export type Baz = Omit<Foo, '$type'> & {
18+
baz?: string;
19+
$type: 'FooBaz';
20+
};
21+
22+
export type Qux = Omit<Bar, '$type'> & {
23+
qux?: string;
24+
$type: 'BarQux';
25+
};
26+
27+
export type GetFoosData = {
28+
body?: never;
29+
path?: never;
30+
query?: never;
31+
url: '/foos';
32+
};
33+
34+
export type GetFoosResponses = {
35+
/**
36+
* OK
37+
*/
38+
200: Bar | Baz | Qux;
39+
};
40+
41+
export type GetFoosResponse = GetFoosResponses[keyof GetFoosResponses];

‎packages/openapi-ts-tests/main/test/__snapshots__/3.1.x/discriminator-allof-inline/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// This file is auto-generated by @hey-api/openapi-ts
2+
3+
export type { Bar, Baz, ClientOptions, Foo, GetFoosData, GetFoosResponse, GetFoosResponses, Qux } from './types.gen';

‎packages/openapi-ts-tests/main/test/__snapshots__/3.1.x/discriminator-allof-inline/types.gen.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// This file is auto-generated by @hey-api/openapi-ts
2+
3+
export type ClientOptions = {
4+
baseUrl: `${string}://${string}` | (string & {});
5+
};
6+
7+
export type Foo = {
8+
$type: string;
9+
foo?: string;
10+
};
11+
12+
export type Bar = Omit<Foo, '$type'> & {
13+
$type: 'FooBar';
14+
bar?: string;
15+
};
16+
17+
export type Baz = Omit<Foo, '$type'> & {
18+
baz?: string;
19+
$type: 'FooBaz';
20+
};
21+
22+
export type Qux = Omit<Bar, '$type'> & {
23+
qux?: string;
24+
$type: 'BarQux';
25+
};
26+
27+
export type GetFoosData = {
28+
body?: never;
29+
path?: never;
30+
query?: never;
31+
url: '/foos';
32+
};
33+
34+
export type GetFoosResponses = {
35+
/**
36+
* OK
37+
*/
38+
200: Bar | Baz | Qux;
39+
};
40+
41+
export type GetFoosResponse = GetFoosResponses[keyof GetFoosResponses];

‎packages/shared/src/openApi/3.0.x/parser/schema.ts‎

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -436,11 +436,11 @@ const parseAllOf = ({
436436
// Collect discriminator information to add after all compositions are processed
437437
type DiscriminatorInfo = {
438438
discriminator: NonNullable<SchemaObject['discriminator']>;
439+
isExplicitMapping: boolean;
439440
isRequired: boolean;
440441
values: ReadonlyArray<string>;
441442
};
442443
const discriminatorsToAdd: Array<DiscriminatorInfo> = [];
443-
const addedDiscriminators = new Set<string>();
444444

445445
for (const compositionSchema of compositionSchemas) {
446446
const originalInAllOf = state.inAllOf;
@@ -478,13 +478,7 @@ const parseAllOf = ({
478478
schema: ref,
479479
});
480480

481-
// Process each discriminator found
482481
for (const { discriminator, oneOf } of discriminators) {
483-
// Skip if we've already collected this discriminator property
484-
if (addedDiscriminators.has(discriminator.propertyName)) {
485-
continue;
486-
}
487-
488482
const values = discriminatorValues(
489483
state.$ref,
490484
discriminator.mapping,
@@ -494,28 +488,48 @@ const parseAllOf = ({
494488
oneOf ? () => oneOf.some((o) => '$ref' in o && o.$ref === state.$ref) : undefined,
495489
);
496490

497-
if (values.length > 0) {
498-
// Check if the discriminator property is required in any of the discriminator schemas
499-
const isRequired = discriminators.some(
500-
(d) =>
501-
d.discriminator.propertyName === discriminator.propertyName &&
502-
// Check in the ref's required array or in the allOf components
503-
(ref.required?.includes(d.discriminator.propertyName) ||
504-
(ref.allOf &&
505-
ref.allOf.some((item) => {
506-
const resolvedItem =
507-
'$ref' in item ? context.resolveRef<SchemaObject>(item.$ref) : item;
508-
return resolvedItem.required?.includes(d.discriminator.propertyName);
509-
}))),
510-
);
491+
if (values.length === 0) {
492+
continue;
493+
}
511494

512-
discriminatorsToAdd.push({
513-
discriminator,
514-
isRequired,
515-
values,
516-
});
517-
addedDiscriminators.add(discriminator.propertyName);
495+
// True when state.$ref appears directly in the mapping; false when the
496+
// value fell back to the schema name because no mapping entry matched.
497+
const isExplicitMapping =
498+
discriminator.mapping !== undefined &&
499+
Object.values(discriminator.mapping).includes(state.$ref);
500+
501+
// An explicit mapping always beats a same-property fallback collected
502+
// earlier (e.g. from a grandparent discriminator that doesn't list this
503+
// schema). Replace it; otherwise skip the duplicate.
504+
const existingIndex = discriminatorsToAdd.findIndex(
505+
(d) => d.discriminator.propertyName === discriminator.propertyName,
506+
);
507+
if (existingIndex !== -1) {
508+
if (isExplicitMapping && !discriminatorsToAdd[existingIndex]!.isExplicitMapping) {
509+
discriminatorsToAdd.splice(existingIndex, 1);
510+
} else {
511+
continue;
512+
}
518513
}
514+
515+
const isRequired = discriminators.some(
516+
(d) =>
517+
d.discriminator.propertyName === discriminator.propertyName &&
518+
(ref.required?.includes(d.discriminator.propertyName) ||
519+
(ref.allOf &&
520+
ref.allOf.some((item) => {
521+
const resolvedItem =
522+
'$ref' in item ? context.resolveRef<SchemaObject>(item.$ref) : item;
523+
return resolvedItem.required?.includes(d.discriminator.propertyName);
524+
}))),
525+
);
526+
527+
discriminatorsToAdd.push({
528+
discriminator,
529+
isExplicitMapping,
530+
isRequired,
531+
values,
532+
});
519533
}
520534
}
521535
}

‎packages/shared/src/openApi/3.1.x/parser/schema.ts‎

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -531,11 +531,11 @@ const parseAllOf = ({
531531
// Collect discriminator information to add after all compositions are processed
532532
type DiscriminatorInfo = {
533533
discriminator: NonNullable<SchemaObject['discriminator']>;
534+
isExplicitMapping: boolean;
534535
isRequired: boolean;
535536
values: ReadonlyArray<string>;
536537
};
537538
const discriminatorsToAdd: Array<DiscriminatorInfo> = [];
538-
const addedDiscriminators = new Set<string>();
539539

540540
for (const compositionSchema of compositionSchemas) {
541541
const originalInAllOf = state.inAllOf;
@@ -573,13 +573,7 @@ const parseAllOf = ({
573573
schema: ref,
574574
});
575575

576-
// Process each discriminator found
577576
for (const { discriminator, oneOf } of discriminators) {
578-
// Skip if we've already collected this discriminator property
579-
if (addedDiscriminators.has(discriminator.propertyName)) {
580-
continue;
581-
}
582-
583577
const values = discriminatorValues(
584578
state.$ref,
585579
discriminator.mapping,
@@ -589,29 +583,49 @@ const parseAllOf = ({
589583
oneOf ? () => oneOf.some((o) => '$ref' in o && o.$ref === state.$ref) : undefined,
590584
);
591585

592-
if (values.length > 0) {
593-
// Check if the discriminator property is required in any of the discriminator schemas
594-
const isRequired = discriminators.some(
595-
(d) =>
596-
d.discriminator.propertyName === discriminator.propertyName &&
597-
// Check in the ref's required array or in the allOf components
598-
(ref.required?.includes(d.discriminator.propertyName) ||
599-
(ref.allOf &&
600-
ref.allOf.some((item) => {
601-
const resolvedItem = item.$ref
602-
? context.resolveRef<SchemaObject>(item.$ref)
603-
: item;
604-
return resolvedItem.required?.includes(d.discriminator.propertyName);
605-
}))),
606-
);
586+
if (values.length === 0) {
587+
continue;
588+
}
607589

608-
discriminatorsToAdd.push({
609-
discriminator,
610-
isRequired,
611-
values,
612-
});
613-
addedDiscriminators.add(discriminator.propertyName);
590+
// True when state.$ref appears directly in the mapping; false when the
591+
// value fell back to the schema name because no mapping entry matched.
592+
const isExplicitMapping =
593+
discriminator.mapping !== undefined &&
594+
Object.values(discriminator.mapping).includes(state.$ref);
595+
596+
// An explicit mapping always beats a same-property fallback collected
597+
// earlier (e.g. from a grandparent discriminator that doesn't list this
598+
// schema). Replace it; otherwise skip the duplicate.
599+
const existingIndex = discriminatorsToAdd.findIndex(
600+
(d) => d.discriminator.propertyName === discriminator.propertyName,
601+
);
602+
if (existingIndex !== -1) {
603+
if (isExplicitMapping && !discriminatorsToAdd[existingIndex]!.isExplicitMapping) {
604+
discriminatorsToAdd.splice(existingIndex, 1);
605+
} else {
606+
continue;
607+
}
614608
}
609+
610+
const isRequired = discriminators.some(
611+
(d) =>
612+
d.discriminator.propertyName === discriminator.propertyName &&
613+
(ref.required?.includes(d.discriminator.propertyName) ||
614+
(ref.allOf &&
615+
ref.allOf.some((item) => {
616+
const resolvedItem = item.$ref
617+
? context.resolveRef<SchemaObject>(item.$ref)
618+
: item;
619+
return resolvedItem.required?.includes(d.discriminator.propertyName);
620+
}))),
621+
);
622+
623+
discriminatorsToAdd.push({
624+
discriminator,
625+
isExplicitMapping,
626+
isRequired,
627+
values,
628+
});
615629
}
616630
}
617631
}

0 commit comments

Comments
 (0)