Skip to content

Commit 78b3323

Browse files
authored
fix(circular): call onCircular callback for all occurrences of circular $refs (#408)
1 parent 037d93e commit 78b3323

File tree

4 files changed

+139
-8
lines changed

4 files changed

+139
-8
lines changed

lib/dereference.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,11 +251,14 @@ function dereference$Ref<S extends object = JSONSchema, O extends ParserOptions<
251251
// overhaul.
252252
if (typeof cache.value === "object" && "$ref" in cache.value && "$ref" in $ref) {
253253
if (cache.value.$ref === $ref.$ref) {
254+
// Fire onCircular for cached circular refs so callers are notified of every occurrence
255+
foundCircularReference(path, $refs, options);
254256
return cache;
255257
} else {
256-
// no-op
258+
// no-op - fall through to re-process (handles external ref edge case)
257259
}
258260
} else {
261+
foundCircularReference(path, $refs, options);
259262
return cache;
260263
}
261264
}

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

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
import { describe, it } from "vitest";
1+
import { describe, it, expect } from "vitest";
22
import $RefParser from "../../../lib/index.js";
33
import helper from "../../utils/helper.js";
44
import path from "../../utils/path.js";
55

6-
import { expect } from "vitest";
7-
86
describe("Schema with an extensive amount of circular $refs", () => {
97
it("should dereference successfully", async () => {
108
const circularRefs = new Set<string>();
@@ -31,8 +29,9 @@ describe("Schema with an extensive amount of circular $refs", () => {
3129
},
3230
});
3331

34-
// Ensure that a circular $ref **was** dereferenced.
35-
expect(circularRefs).toHaveLength(23);
32+
// With circular: true (default), circular $refs are replaced with the resolved object.
33+
// onCircular fires for each $ref location pointing to a circular target (118 unique paths).
34+
expect(circularRefs.size).toBe(118);
3635
expect(schema.components?.schemas?.Customer?.properties?.customerNode).toStrictEqual({
3736
type: "array",
3837
items: {
@@ -74,8 +73,11 @@ describe("Schema with an extensive amount of circular $refs", () => {
7473
},
7574
});
7675

77-
// Ensure that a circular $ref was **not** dereferenced.
78-
expect(circularRefs).toHaveLength(23);
76+
// With circular: 'ignore', circular $refs remain as { $ref: "..." } objects.
77+
// onCircular fires for each $ref location (same 118 paths as above), PLUS 55 additional
78+
// "interior paths" - $refs inside circular schemas that get re-encountered when the
79+
// containing schema is accessed from multiple entry points.
80+
expect(circularRefs.size).toBe(173);
7981
expect(schema.components?.schemas?.Customer?.properties?.customerNode).toStrictEqual({
8082
type: "array",
8183
items: {
@@ -104,4 +106,54 @@ describe("Schema with an extensive amount of circular $refs", () => {
104106
expect(parser.$refs.circular).to.equal(true);
105107
}
106108
});
109+
110+
it("should expose path differences between circular: true and circular: 'ignore'", async () => {
111+
const SCHEMA_PATH = "test/specs/circular-extensive/schema.json";
112+
113+
// Collect paths with circular: true (default)
114+
const pathsTrue = new Set<string>();
115+
await new $RefParser().dereference(path.rel(SCHEMA_PATH), {
116+
dereference: {
117+
onCircular: (ref: string) => pathsTrue.add(ref.split("#")[1]),
118+
},
119+
});
120+
121+
// Collect paths with circular: 'ignore'
122+
const pathsIgnore = new Set<string>();
123+
await new $RefParser().dereference(path.rel(SCHEMA_PATH), {
124+
dereference: {
125+
circular: "ignore",
126+
onCircular: (ref: string) => pathsIgnore.add(ref.split("#")[1]),
127+
},
128+
});
129+
130+
// Verify the counts
131+
expect(pathsTrue.size).toBe(118);
132+
expect(pathsIgnore.size).toBe(173);
133+
134+
// All paths in 'true' mode should also be in 'ignore' mode
135+
const pathsOnlyInTrue = [...pathsTrue].filter((p) => !pathsIgnore.has(p));
136+
expect(pathsOnlyInTrue).toHaveLength(0);
137+
138+
// 'ignore' mode has 55 additional paths not found in 'true' mode
139+
const pathsOnlyInIgnore = [...pathsIgnore].filter((p) => !pathsTrue.has(p));
140+
expect(pathsOnlyInIgnore).toHaveLength(55);
141+
142+
// These extra paths are "interior paths" within circular schemas that get
143+
// re-visited because $ref objects allow re-entry from different traversal routes.
144+
// With circular: true, these paths aren't reported because the same object
145+
// instance is detected by parents.has() which doesn't trigger onCircular.
146+
//
147+
// Example extra paths (interior of circular schemas reached via different routes):
148+
// Customer contains customerNode.items → CustomerNode (circular).
149+
// - In 'true' mode: Customer.customerNode.items becomes the resolved CustomerNode object.
150+
// When Customer is accessed from another route, customerNode.items is the same object
151+
// instance already in `parents`, so no onCircular fires for that interior path.
152+
// - In 'ignore' mode: Customer.customerNode.items remains { $ref: "..." }.
153+
// When Customer is accessed from another route, the $ref is re-encountered and
154+
// triggers onCircular via cache hit, reporting the interior path.
155+
expect(pathsOnlyInIgnore).toContain("/components/schemas/Customer/properties/customerNode/items");
156+
expect(pathsOnlyInIgnore).toContain("/components/schemas/Customer/properties/customerExternalReference/items");
157+
expect(pathsOnlyInIgnore).toContain("/components/schemas/Node/properties/configWcCodeNode/items");
158+
});
107159
});
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { describe, it, expect } from "vitest";
2+
import $RefParser from "../../../lib/index.js";
3+
import path from "../../utils/path.js";
4+
5+
/**
6+
* Tests that onCircular is called for EVERY occurrence of a circular $ref,
7+
* not just the first detection. The schema has 5 $refs to #/definitions/Node:
8+
* 1. definitions/Node/properties/self (self-reference)
9+
* 2. definitions/Container/properties/primaryNode
10+
* 3. definitions/Container/properties/secondaryNode
11+
* 4. definitions/Container/properties/nodeList/items
12+
* 5. root
13+
*/
14+
describe("Schema with multiple occurrences of the same circular $ref target", () => {
15+
const SCHEMA_PATH = "test/specs/circular-multi-occurrence/schema.yaml";
16+
17+
const EXPECTED_PATH_SUFFIXES = [
18+
"definitions/Node/properties/self",
19+
"definitions/Container/properties/primaryNode",
20+
"definitions/Container/properties/secondaryNode",
21+
"definitions/Container/properties/nodeList/items",
22+
"/root",
23+
] as const;
24+
25+
it.each([
26+
{ mode: "true (default)", circular: true as const },
27+
{ mode: "'ignore'", circular: "ignore" as const },
28+
])("should call onCircular for every occurrence (circular: $mode)", async ({ circular }) => {
29+
const circularRefs: string[] = [];
30+
31+
const parser = new $RefParser();
32+
await parser.dereference(path.rel(SCHEMA_PATH), {
33+
dereference: {
34+
circular,
35+
onCircular: (refPath: string) => circularRefs.push(refPath),
36+
},
37+
});
38+
39+
// Exactly 5 calls, all unique paths
40+
expect(circularRefs).toHaveLength(5);
41+
expect(new Set(circularRefs).size).toBe(5);
42+
expect(parser.$refs.circular).toBe(true);
43+
44+
// Each expected path suffix should appear exactly once
45+
for (const expectedSuffix of EXPECTED_PATH_SUFFIXES) {
46+
const matches = circularRefs.filter((p) => p.endsWith(expectedSuffix));
47+
expect(matches, `Expected exactly one path ending with "${expectedSuffix}"`).toHaveLength(1);
48+
}
49+
});
50+
});
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Test schema: Multiple occurrences of the same circular $ref target
2+
# Verifies onCircular is called for ALL 5 occurrences, not just the first.
3+
4+
definitions:
5+
Node:
6+
type: object
7+
properties:
8+
name:
9+
type: string
10+
self:
11+
$ref: "#/definitions/Node" # 1. Self-reference
12+
13+
Container:
14+
type: object
15+
properties:
16+
primaryNode:
17+
$ref: "#/definitions/Node" # 2. First external ref
18+
secondaryNode:
19+
$ref: "#/definitions/Node" # 3. Second external ref
20+
nodeList:
21+
type: array
22+
items:
23+
$ref: "#/definitions/Node" # 4. Array items ref
24+
25+
root:
26+
$ref: "#/definitions/Node" # 5. Root-level ref

0 commit comments

Comments
 (0)