Skip to content

Commit 8178f61

Browse files
mrlubosmrlubos
authored
Merge pull request #3422 from j-westover/fix/bundle-sibling-schema-resolution
2 parents 937486e + 19fd80a commit 8178f61

16 files changed

Lines changed: 525 additions & 6 deletions

‎.changeset/purple-ligers-raise.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@hey-api/json-schema-ref-parser": patch
3+
"@hey-api/openapi-ts": patch
4+
---
5+
6+
**parser**: fix: resolve sibling schemas from external files during bundling

‎packages/json-schema-ref-parser/src/__tests__/bundle.test.ts‎

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,231 @@ describe('bundle', () => {
8181

8282
await expectBundledSchemaToMatchSnapshot(schema, 'redfish-like.json');
8383
});
84+
85+
describe('sibling schema resolution', () => {
86+
const specsDir = path.join(getSpecsPath(), 'json-schema-ref-parser');
87+
88+
const findSchemaByValue = (
89+
schemas: Record<string, any>,
90+
predicate: (value: any) => boolean,
91+
): [string, any] | undefined => {
92+
for (const [name, value] of Object.entries(schemas)) {
93+
if (predicate(value)) {
94+
return [name, value];
95+
}
96+
}
97+
return undefined;
98+
};
99+
100+
it('hoists sibling schemas through a bare $ref wrapper chain', async () => {
101+
const refParser = new $RefParser();
102+
const pathOrUrlOrSchema = path.join(specsDir, 'sibling-schema-root.json');
103+
const schema = (await refParser.bundle({ pathOrUrlOrSchema })) as any;
104+
105+
expect(schema.components).toBeDefined();
106+
expect(schema.components.schemas).toBeDefined();
107+
108+
const schemas = schema.components.schemas;
109+
110+
const mainSchema = findSchemaByValue(
111+
schemas,
112+
(v) => v.type === 'object' && v.properties?.name,
113+
);
114+
expect(mainSchema).toBeDefined();
115+
const [mainName, mainValue] = mainSchema!;
116+
expect(mainValue.type).toBe('object');
117+
expect(mainValue.properties.name).toEqual({ type: 'string' });
118+
119+
const enumSchema = findSchemaByValue(
120+
schemas,
121+
(v) => Array.isArray(v.enum) && v.enum.includes('active'),
122+
);
123+
expect(enumSchema).toBeDefined();
124+
const [enumName, enumValue] = enumSchema!;
125+
expect(enumValue.type).toBe('string');
126+
expect(enumValue.enum).toEqual(['active', 'inactive', 'pending']);
127+
128+
// The main schema's status property should reference the hoisted enum
129+
expect(mainValue.properties.status.$ref).toBe(`#/components/schemas/${enumName}`);
130+
131+
// The root path's schema ref should point to the hoisted main schema
132+
const rootRef = schema.paths['/test'].get.responses['200'].content['application/json'].schema;
133+
expect(rootRef.$ref).toBe(`#/components/schemas/${mainName}`);
134+
});
135+
136+
it('hoists sibling schemas through an extended $ref wrapper chain', async () => {
137+
const refParser = new $RefParser();
138+
const pathOrUrlOrSchema = path.join(specsDir, 'sibling-schema-extended-root.json');
139+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
140+
141+
try {
142+
const schema = (await refParser.bundle({ pathOrUrlOrSchema })) as any;
143+
144+
expect(schema.components).toBeDefined();
145+
expect(schema.components.schemas).toBeDefined();
146+
147+
const schemas = schema.components.schemas;
148+
149+
// The main schema should be hoisted (with the extra description merged in)
150+
const mainSchema = findSchemaByValue(
151+
schemas,
152+
(v) =>
153+
v.description === 'Wrapper that extends the versioned schema' ||
154+
(v.type === 'object' && v.properties?.name),
155+
);
156+
expect(mainSchema).toBeDefined();
157+
158+
// The sibling enum must also be hoisted (this was the bug — it was lost before the fix)
159+
const enumSchema = findSchemaByValue(
160+
schemas,
161+
(v) => Array.isArray(v.enum) && v.enum.includes('active'),
162+
);
163+
expect(enumSchema).toBeDefined();
164+
const [, enumValue] = enumSchema!;
165+
expect(enumValue.type).toBe('string');
166+
expect(enumValue.enum).toEqual(['active', 'inactive', 'pending']);
167+
168+
// No "Skipping unresolvable $ref" warnings should have been emitted
169+
const unresolvableWarnings = warnSpy.mock.calls.filter(
170+
(args) => typeof args[0] === 'string' && args[0].includes('Skipping unresolvable $ref'),
171+
);
172+
expect(unresolvableWarnings).toHaveLength(0);
173+
} finally {
174+
warnSpy.mockRestore();
175+
}
176+
});
177+
178+
it('hoists sibling schemas from a direct reference (no wrapper)', async () => {
179+
const refParser = new $RefParser();
180+
const pathOrUrlOrSchema = path.join(specsDir, 'sibling-schema-direct-root.json');
181+
const schema = (await refParser.bundle({ pathOrUrlOrSchema })) as any;
182+
183+
expect(schema.components).toBeDefined();
184+
expect(schema.components.schemas).toBeDefined();
185+
186+
const schemas = schema.components.schemas;
187+
188+
const mainSchema = findSchemaByValue(
189+
schemas,
190+
(v) => v.type === 'object' && v.properties?.name,
191+
);
192+
expect(mainSchema).toBeDefined();
193+
194+
const enumSchema = findSchemaByValue(
195+
schemas,
196+
(v) => Array.isArray(v.enum) && v.enum.includes('active'),
197+
);
198+
expect(enumSchema).toBeDefined();
199+
const [enumName, enumValue] = enumSchema!;
200+
expect(enumValue.enum).toEqual(['active', 'inactive', 'pending']);
201+
202+
const [, mainValue] = mainSchema!;
203+
expect(mainValue.properties.status.$ref).toBe(`#/components/schemas/${enumName}`);
204+
});
205+
206+
it('hoists multiple sibling schemas through an extended wrapper', async () => {
207+
const refParser = new $RefParser();
208+
const pathOrUrlOrSchema = path.join(specsDir, 'sibling-schema-multi-root.json');
209+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
210+
211+
try {
212+
const schema = (await refParser.bundle({ pathOrUrlOrSchema })) as any;
213+
214+
expect(schema.components).toBeDefined();
215+
expect(schema.components.schemas).toBeDefined();
216+
217+
const schemas = schema.components.schemas;
218+
219+
const mainSchema = findSchemaByValue(
220+
schemas,
221+
(v) => v.type === 'object' && v.properties?.health,
222+
);
223+
expect(mainSchema).toBeDefined();
224+
225+
const statusEnum = findSchemaByValue(
226+
schemas,
227+
(v) => Array.isArray(v.enum) && v.enum.includes('enabled'),
228+
);
229+
expect(statusEnum).toBeDefined();
230+
expect(statusEnum![1].enum).toEqual(['enabled', 'disabled', 'standby']);
231+
232+
const healthEnum = findSchemaByValue(
233+
schemas,
234+
(v) => Array.isArray(v.enum) && v.enum.includes('ok'),
235+
);
236+
expect(healthEnum).toBeDefined();
237+
expect(healthEnum![1].enum).toEqual(['ok', 'warning', 'critical']);
238+
239+
const [, mainValue] = mainSchema!;
240+
expect(mainValue.properties.status.$ref).toBe(`#/components/schemas/${statusEnum![0]}`);
241+
expect(mainValue.properties.health.$ref).toBe(`#/components/schemas/${healthEnum![0]}`);
242+
243+
const unresolvableWarnings = warnSpy.mock.calls.filter(
244+
(args) => typeof args[0] === 'string' && args[0].includes('Skipping unresolvable $ref'),
245+
);
246+
expect(unresolvableWarnings).toHaveLength(0);
247+
} finally {
248+
warnSpy.mockRestore();
249+
}
250+
});
251+
252+
it('handles multiple external files with same-named sibling schemas', async () => {
253+
const refParser = new $RefParser();
254+
const pathOrUrlOrSchema = path.join(specsDir, 'sibling-schema-collision-root.json');
255+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
256+
257+
try {
258+
const schema = (await refParser.bundle({ pathOrUrlOrSchema })) as any;
259+
260+
expect(schema.components).toBeDefined();
261+
expect(schema.components.schemas).toBeDefined();
262+
263+
const schemas = schema.components.schemas;
264+
const schemaNames = Object.keys(schemas);
265+
266+
const mainSchemaKey = schemaNames.find((name) => name.includes('MainSchema'));
267+
const otherSchemaKey = schemaNames.find((name) => name.includes('OtherSchema'));
268+
269+
expect(mainSchemaKey).toBeDefined();
270+
expect(otherSchemaKey).toBeDefined();
271+
272+
const statusSchemas = schemaNames.filter((name) => name.includes('Status'));
273+
expect(statusSchemas.length).toBeGreaterThanOrEqual(2);
274+
275+
const statusValues = statusSchemas.map((name) => schemas[name]);
276+
const stringStatus = statusValues.find((v: any) => v.type === 'string');
277+
const integerStatus = statusValues.find((v: any) => v.type === 'integer');
278+
279+
expect(stringStatus).toBeDefined();
280+
expect(integerStatus).toBeDefined();
281+
expect(stringStatus!.enum).toEqual(['active', 'inactive']);
282+
expect(integerStatus!.enum).toEqual([0, 1, 2]);
283+
284+
const mainSchemaValue = schemas[mainSchemaKey!];
285+
const mainStatusRef = mainSchemaValue.properties.status.$ref;
286+
expect(mainStatusRef).toMatch(/^#\/components\/schemas\/.*Status/);
287+
288+
const referencedStatus = schemas[mainStatusRef.replace('#/components/schemas/', '')];
289+
expect(referencedStatus).toBeDefined();
290+
expect(referencedStatus.type).toBe('string');
291+
expect(referencedStatus.enum).toEqual(['active', 'inactive']);
292+
293+
const otherSchemaValue = schemas[otherSchemaKey!];
294+
const otherStatusRef = otherSchemaValue.properties.code.$ref;
295+
expect(otherStatusRef).toMatch(/^#\/components\/schemas\/.*Status/);
296+
297+
const referencedOtherStatus = schemas[otherStatusRef.replace('#/components/schemas/', '')];
298+
expect(referencedOtherStatus).toBeDefined();
299+
expect(referencedOtherStatus.type).toBe('integer');
300+
expect(referencedOtherStatus.enum).toEqual([0, 1, 2]);
301+
302+
const unresolvableWarnings = warnSpy.mock.calls.filter(
303+
(args) => typeof args[0] === 'string' && args[0].includes('Skipping unresolvable $ref'),
304+
);
305+
expect(unresolvableWarnings).toHaveLength(0);
306+
} finally {
307+
warnSpy.mockRestore();
308+
}
309+
});
310+
});
84311
});

‎packages/json-schema-ref-parser/src/bundle.ts‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,31 @@ const inventory$Ref = <S extends object = JSONSchema>({
157157
pointer = $refs._resolve($refPath, pathFromRoot, options);
158158
} catch (error) {
159159
if (error instanceof MissingPointerError) {
160-
// Log warning but continue - common in complex schema ecosystems
161-
console.warn(`Skipping unresolvable $ref: ${$refPath}`);
162-
return;
160+
// The ref couldn't be resolved in the target file. This commonly
161+
// happens when a wrapper file redirects via $ref to a versioned
162+
// file, and the bundler's crawl path retains the wrapper URL.
163+
// Try resolving the hash fragment against other files in $refs
164+
// that might contain the target schema.
165+
const hash = url.getHash($refPath);
166+
if (hash) {
167+
const baseFile = url.stripHash($refPath);
168+
for (const filePath of Object.keys($refs._$refs)) {
169+
if (filePath === baseFile) continue;
170+
try {
171+
pointer = $refs._resolve(filePath + hash, pathFromRoot, options);
172+
if (pointer) break;
173+
} catch {
174+
// try next file
175+
}
176+
}
177+
}
178+
if (!pointer) {
179+
console.warn(`Skipping unresolvable $ref: ${$refPath}`);
180+
return;
181+
}
182+
} else {
183+
throw error;
163184
}
164-
throw error; // Re-throw unexpected errors
165185
}
166186

167187
if (pointer) {
@@ -217,8 +237,19 @@ const inventory$Ref = <S extends object = JSONSchema>({
217237
inventory.push(newEntry);
218238
inventoryLookup.add(newEntry);
219239

220-
// Recursively crawl the resolved value
240+
// Recursively crawl the resolved value.
241+
// When the resolution followed a $ref chain to a different file,
242+
// use the resolved file as the base path so that local $ref values
243+
// (e.g. #/components/schemas/SiblingSchema) inside the resolved
244+
// value resolve against the correct file.
221245
if (!existingEntry || external) {
246+
let crawlPath = pointer.path;
247+
248+
const originalFile = url.stripHash($refPath);
249+
if (file !== originalFile) {
250+
crawlPath = file + url.getHash(pointer.path);
251+
}
252+
222253
crawl({
223254
$refs,
224255
indirections: indirections + 1,
@@ -227,7 +258,7 @@ const inventory$Ref = <S extends object = JSONSchema>({
227258
key: null,
228259
options,
229260
parent: pointer.value,
230-
path: pointer.path,
261+
path: crawlPath,
231262
pathFromRoot,
232263
resolvedRefs,
233264
visitedObjects,

‎specs/json-schema-ref-parser/sibling-schema-collision-other.json‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{
2+
"components": {
3+
"schemas": {
4+
"OtherSchema": {
5+
"type": "object",
6+
"properties": {
7+
"code": {
8+
"$ref": "#/components/schemas/Status"
9+
}
10+
}
11+
},
12+
"Status": {
13+
"type": "integer",
14+
"enum": [0, 1, 2]
15+
}
16+
}
17+
}
18+
}

‎specs/json-schema-ref-parser/sibling-schema-collision-root.json‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
{
2+
"openapi": "3.1.0",
3+
"info": { "title": "Collision Test", "version": "1.0.0" },
4+
"paths": {
5+
"/main": {
6+
"get": {
7+
"responses": {
8+
"200": {
9+
"description": "ok",
10+
"content": {
11+
"application/json": {
12+
"schema": {
13+
"$ref": "sibling-schema-collision-wrapper.json#/components/schemas/MainSchema"
14+
}
15+
}
16+
}
17+
}
18+
}
19+
}
20+
},
21+
"/other": {
22+
"get": {
23+
"responses": {
24+
"200": {
25+
"description": "ok",
26+
"content": {
27+
"application/json": {
28+
"schema": {
29+
"$ref": "sibling-schema-collision-other.json#/components/schemas/OtherSchema"
30+
}
31+
}
32+
}
33+
}
34+
}
35+
}
36+
}
37+
}
38+
}

‎specs/json-schema-ref-parser/sibling-schema-collision-versioned.json‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{
2+
"components": {
3+
"schemas": {
4+
"MainSchema": {
5+
"type": "object",
6+
"properties": {
7+
"status": {
8+
"$ref": "#/components/schemas/Status"
9+
}
10+
}
11+
},
12+
"Status": {
13+
"type": "string",
14+
"enum": ["active", "inactive"]
15+
}
16+
}
17+
}
18+
}

‎specs/json-schema-ref-parser/sibling-schema-collision-wrapper.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"components": {
3+
"schemas": {
4+
"MainSchema": {
5+
"$ref": "sibling-schema-collision-versioned.json#/components/schemas/MainSchema"
6+
}
7+
}
8+
}
9+
}

0 commit comments

Comments
 (0)