Skip to content

Commit 382e927

Browse files
authored
fix: allow $defs and fix circular pointers issue with $ref in root (#383)
1 parent 5bf87ad commit 382e927

17 files changed

+397
-27
lines changed

docs/options.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ JSON Schema $Ref Parser comes with built-in support for HTTP and HTTPS, as well
6969
| `file.order` `http.order` | `number` | Resolvers run in a specific order, relative to other resolvers. For example, a resolver with `order: 5` will run _before_ a resolver with `order: 10`. If a resolver is unable to successfully resolve a path, then the next resolver is tried, until one succeeds or they all fail.<br><br>You can change the order in which resolvers run, which is useful if you know that most of your file references will be a certain type, or if you add [your own custom resolver](plugins/resolvers.md) that you want to run _first_. |
7070
| `file.canRead` `http.canRead` | `boolean`, `RegExp`, `string`, `array`, `function` | Determines which resolvers will be used for which files.<br><br>A regular expression can be used to match files by their full path. A string (or array of strings) can be used to match files by their file extension. Or a function can be used to perform more complex matching logic. See the [custom resolver](plugins/resolvers.md) docs for details. |
7171
| `http.headers` | `object` | You can specify any HTTP headers that should be sent when downloading files. For example, some servers may require you to set the `Accept` or `Referrer` header. |
72-
| `http.timeout` | `number` | The amount of time (in milliseconds) to wait for a response from the server when downloading files. The default is 60 seconds. |
72+
| `http.timeout` | `number` | The amount of time (in milliseconds) to wait for a response from the server when downloading files. The default is 60 seconds. |
7373
| `http.redirects` | `number` | The maximum number of HTTP redirects to follow per file. The default is 5. To disable automatic following of redirects, set this to zero. |
7474
| `http.withCredentials` | `boolean` | Set this to `true` if you're downloading files from a CORS-enabled server that requires authentication |
7575

lib/bundle.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ function crawl<S extends object = JSONSchema, O extends ParserOptions<S> = Parse
7676
const keys = Object.keys(obj).sort((a, b) => {
7777
// Most people will expect references to be bundled into the the "definitions" property,
7878
// so we always crawl that property first, if it exists.
79-
if (a === "definitions") {
79+
if (a === "definitions" || a === "$defs") {
8080
return -1;
81-
} else if (b === "definitions") {
81+
} else if (b === "definitions" || b === "$defs") {
8282
return 1;
8383
} else {
8484
// Otherwise, crawl the keys based on their length.
@@ -216,8 +216,14 @@ function remap(inventory: InventoryEntry[]) {
216216
} else {
217217
// Determine how far each $ref is from the "definitions" property.
218218
// Most people will expect references to be bundled into the the "definitions" property if possible.
219-
const aDefinitionsIndex = a.pathFromRoot.lastIndexOf("/definitions");
220-
const bDefinitionsIndex = b.pathFromRoot.lastIndexOf("/definitions");
219+
const aDefinitionsIndex = Math.max(
220+
a.pathFromRoot.lastIndexOf("/definitions"),
221+
a.pathFromRoot.lastIndexOf("/$defs"),
222+
);
223+
const bDefinitionsIndex = Math.max(
224+
b.pathFromRoot.lastIndexOf("/definitions"),
225+
b.pathFromRoot.lastIndexOf("/$defs"),
226+
);
221227

222228
if (aDefinitionsIndex !== bDefinitionsIndex) {
223229
// Give higher priority to the $ref that's closer to the "definitions" property

lib/dereference.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ function dereference$Ref<S extends object = JSONSchema, O extends ParserOptions<
251251
//
252252
// This check is not perfect and the design of the dereference caching mechanism needs a total
253253
// overhaul.
254-
if (typeof cache.value === 'object' && '$ref' in cache.value && '$ref' in $ref) {
254+
if (typeof cache.value === "object" && "$ref" in cache.value && "$ref" in $ref) {
255255
if (cache.value.$ref === $ref.$ref) {
256256
return cache;
257257
} else {

lib/index.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,12 +192,7 @@ export class $RefParser<S extends object = JSONSchema, O extends ParserOptions<S
192192
public resolve(schema: S | string | unknown, options: O): Promise<$Refs<S, O>>;
193193
public resolve(schema: S | string | unknown, options: O, callback: $RefsCallback<S, O>): Promise<void>;
194194
public resolve(path: string, schema: S | string | unknown, options: O): Promise<$Refs<S, O>>;
195-
public resolve(
196-
path: string,
197-
schema: S | string | unknown,
198-
options: O,
199-
callback: $RefsCallback<S, O>,
200-
): Promise<void>;
195+
public resolve(path: string, schema: S | string | unknown, options: O, callback: $RefsCallback<S, O>): Promise<void>;
201196
async resolve() {
202197
const args = normalizeArgs<S, O>(arguments);
203198

lib/pointer.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import * as url from "./util/url.js";
55
import { JSONParserError, InvalidPointerError, MissingPointerError, isHandledError } from "./util/errors.js";
66
import type { JSONSchema } from "./types";
77

8-
export const nullSymbol = Symbol('null');
8+
export const nullSymbol = Symbol("null");
99

1010
const slashes = /\//g;
1111
const tildes = /~/g;
@@ -101,10 +101,6 @@ class Pointer<S extends object = JSONSchema, O extends ParserOptions<S> = Parser
101101
this.path = Pointer.join(this.path, tokens.slice(i));
102102
}
103103

104-
if (typeof this.value === "object" && this.value !== null && !isRootPath(pathFromRoot) && "$ref" in this.value) {
105-
return this;
106-
}
107-
108104
const token = tokens[i];
109105

110106
if (this.value[token] === undefined || (this.value[token] === null && i === tokens.length - 1)) {
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
11
$ref: ./circular-external-direct-child.yaml#/foo
2+
foo:
3+
type: object

test/specs/circular-external-direct/circular-external-direct.spec.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ describe("Schema with direct circular (recursive) external $refs", () => {
1212
path.rel("test/specs/circular-external-direct/circular-external-direct-root.yaml"),
1313
);
1414
expect(schema).to.equal(parser.schema);
15-
expect(schema).to.deep.equal(parsedSchema.schema);
1615
expect(parser.$refs.paths()).to.deep.equal([
1716
path.abs("test/specs/circular-external-direct/circular-external-direct-root.yaml"),
1817
]);
@@ -29,6 +28,6 @@ describe("Schema with direct circular (recursive) external $refs", () => {
2928
expect(schema).to.equal(parser.schema);
3029
expect(schema).to.deep.equal(dereferencedSchema);
3130
// The "circular" flag should be set
32-
expect(parser.$refs.circular).to.equal(true);
31+
expect(parser.$refs.circular).to.equal(false);
3332
});
3433
});
Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
11
export default {
2-
$ref: "./circular-external-direct-root.yaml#/foo",
2+
foo: {
3+
type: "object",
4+
},
5+
type: "object",
36
};
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
export default {
22
schema: {
3-
$ref: "./circular-external-direct-child.yaml#/foo",
3+
foo: {
4+
type: "object",
5+
},
46
},
57
};

test/specs/circular-external/circular-external.spec.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,22 +57,22 @@ describe("Schema with circular (recursive) external $refs", () => {
5757
const parser = new $RefParser();
5858

5959
const schema = await parser.dereference(path.rel("test/specs/circular-external/circular-external.yaml"), {
60-
dereference: { circular: 'ignore' },
60+
dereference: { circular: "ignore" },
6161
});
6262

6363
expect(schema).to.equal(parser.schema);
6464
expect(schema).not.to.deep.equal(dereferencedSchema);
6565
expect(parser.$refs.circular).to.equal(true);
6666
// @ts-expect-error TS(2532): Object is possibly 'undefined'.
67-
expect(schema.definitions.pet.title).to.equal('pet');
67+
expect(schema.definitions.pet.title).to.equal("pet");
6868
// @ts-expect-error TS(2532): Object is possibly 'undefined'.
69-
expect(schema.definitions.thing).to.deep.equal({ $ref: 'circular-external.yaml#/definitions/thing'});
69+
expect(schema.definitions.thing).to.deep.equal({ $ref: "circular-external.yaml#/definitions/thing" });
7070
// @ts-expect-error TS(2532): Object is possibly 'undefined'.
71-
expect(schema.definitions.person).to.deep.equal({ $ref: 'definitions/person.yaml'});
71+
expect(schema.definitions.person).to.deep.equal({ $ref: "definitions/person.yaml" });
7272
// @ts-expect-error TS(2532): Object is possibly 'undefined'.
73-
expect(schema.definitions.parent).to.deep.equal({ $ref: 'definitions/parent.yaml'});
73+
expect(schema.definitions.parent).to.deep.equal({ $ref: "definitions/parent.yaml" });
7474
// @ts-expect-error TS(2532): Object is possibly 'undefined'.
75-
expect(schema.definitions.child).to.deep.equal({ $ref: 'definitions/child.yaml'});
75+
expect(schema.definitions.child).to.deep.equal({ $ref: "definitions/child.yaml" });
7676
});
7777

7878
it('should throw an error if "options.dereference.circular" is false', async () => {

test/specs/defs/defs.spec.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { describe, it } from "vitest";
2+
import { expect } from "vitest";
3+
import $RefParser from "../../../lib/index.js";
4+
import schemaA from "./schemaA.json";
5+
import schemaB from "./schemaB.json";
6+
7+
describe("Defs", () => {
8+
it("definitions and $defs should both work", async () => {
9+
const parser = new $RefParser();
10+
11+
const resultA = await parser.dereference(schemaA, { mutateInputSchema: false });
12+
await expect(resultA).toMatchFileSnapshot("dereferencedA.json");
13+
14+
const resultB = await parser.dereference(schemaB, { mutateInputSchema: false });
15+
await expect(resultB).toMatchFileSnapshot("dereferencedB.json");
16+
});
17+
});

test/specs/defs/dereferencedA.json

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
{
2+
"$schema": "http://json-schema.org/draft-07/schema#",
3+
"definitions": {
4+
"SupplierPriceElement": {
5+
"type": "object",
6+
"properties": {
7+
"fee": {
8+
"type": "object",
9+
"properties": {
10+
"modificationFee": {
11+
"type": "object",
12+
"properties": {
13+
"amount": {
14+
"type": "number",
15+
"format": "float"
16+
}
17+
}
18+
}
19+
}
20+
},
21+
"purchaseRate": {
22+
"allOf": [
23+
{
24+
"type": "object",
25+
"properties": {
26+
"amount": {
27+
"type": "number",
28+
"format": "float"
29+
}
30+
}
31+
},
32+
{
33+
"type": "number",
34+
"format": "float"
35+
}
36+
]
37+
}
38+
}
39+
},
40+
"AllFees": {
41+
"type": "object",
42+
"properties": {
43+
"modificationFee": {
44+
"type": "object",
45+
"properties": {
46+
"amount": {
47+
"type": "number",
48+
"format": "float"
49+
}
50+
}
51+
}
52+
}
53+
},
54+
"MonetaryAmount": {
55+
"type": "object",
56+
"properties": {
57+
"amount": {
58+
"type": "number",
59+
"format": "float"
60+
}
61+
}
62+
},
63+
"Amount": {
64+
"type": "number",
65+
"format": "float"
66+
},
67+
"InDetailParent": {
68+
"allOf": [
69+
{
70+
"type": "object",
71+
"properties": {
72+
"amount": {
73+
"type": "number",
74+
"format": "float"
75+
}
76+
}
77+
},
78+
{
79+
"type": "number",
80+
"format": "float"
81+
}
82+
]
83+
}
84+
},
85+
"type": "object",
86+
"properties": {
87+
"fee": {
88+
"type": "object",
89+
"properties": {
90+
"modificationFee": {
91+
"type": "object",
92+
"properties": {
93+
"amount": {
94+
"type": "number",
95+
"format": "float"
96+
}
97+
}
98+
}
99+
}
100+
},
101+
"purchaseRate": {
102+
"allOf": [
103+
{
104+
"type": "object",
105+
"properties": {
106+
"amount": {
107+
"type": "number",
108+
"format": "float"
109+
}
110+
}
111+
},
112+
{
113+
"type": "number",
114+
"format": "float"
115+
}
116+
]
117+
}
118+
}
119+
}

0 commit comments

Comments
 (0)