Skip to content

Commit 38a1a87

Browse files
marbemacmarbemac
authored
fix(bundleTarget): improve # $ref handling, and preserve circular indirect $refs (#115)
1 parent 3d2ab04 commit 38a1a87

File tree

2 files changed

+191
-80
lines changed

2 files changed

+191
-80
lines changed

‎src/__tests__/bundle.spec.ts

Lines changed: 122 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -599,28 +599,27 @@ describe('bundleTargetPath()', () => {
599599

600600
expect(safeStringify(result)).toEqual(
601601
safeStringify({
602+
title: 'Hello',
603+
type: 'object',
604+
properties: {
605+
Hello: {
606+
$ref: `#`,
607+
},
608+
World: {
609+
$ref: `#/${BUNDLE_ROOT}/World`,
610+
},
611+
},
602612
[BUNDLE_ROOT]: {
603-
Hello: '[Circular]',
604613
World: {
614+
title: 'World',
615+
type: 'object',
605616
properties: {
606617
name: {
607618
type: 'string',
608619
},
609620
},
610-
title: 'World',
611-
type: 'object',
612621
},
613622
},
614-
properties: {
615-
Hello: {
616-
$ref: `#/${BUNDLE_ROOT}/Hello`,
617-
},
618-
World: {
619-
$ref: `#/${BUNDLE_ROOT}/World`,
620-
},
621-
},
622-
title: 'Hello',
623-
type: 'object',
624623
}),
625624
);
626625
});
@@ -659,21 +658,20 @@ describe('bundleTargetPath()', () => {
659658

660659
expect(safeStringify(result)).toEqual(
661660
safeStringify({
662-
__bundled__: {
663-
GeographicalCoordinate: {
664-
type: 'object',
665-
},
666-
Location: '[Circular]',
667-
},
661+
type: 'object',
668662
properties: {
669663
PhysicalGeographicalCoordinate: {
670664
$ref: '#/__bundled__/GeographicalCoordinate',
671665
},
672666
RelatedLocation: {
673-
$ref: '#/__bundled__/Location',
667+
$ref: '#',
668+
},
669+
},
670+
__bundled__: {
671+
GeographicalCoordinate: {
672+
type: 'object',
674673
},
675674
},
676-
type: 'object',
677675
}),
678676
);
679677
});
@@ -920,6 +918,109 @@ describe('bundleTargetPath()', () => {
920918
expect(Array.isArray(result.components.schemas)).toBe(false);
921919
});
922920

921+
it('should rewrite $refs that point at the bundleRoot, to #', () => {
922+
const document = {
923+
__target__: {
924+
type: 'object',
925+
properties: {
926+
circular: {
927+
$ref: '#/__target__',
928+
},
929+
},
930+
},
931+
};
932+
933+
const result = bundleTarget({
934+
document,
935+
path: '#/__target__',
936+
});
937+
938+
expect(result).toEqual({
939+
type: 'object',
940+
properties: {
941+
circular: {
942+
$ref: '#',
943+
},
944+
},
945+
});
946+
});
947+
948+
it('should handle when the target is a direct $ref to itself', () => {
949+
const document = {
950+
__target__: {
951+
$ref: '#/__target__',
952+
description: 'Test',
953+
},
954+
};
955+
956+
const result = bundleTarget({
957+
document,
958+
path: '#/__target__',
959+
});
960+
961+
expect(result).toEqual({
962+
$ref: '#',
963+
description: 'Test',
964+
});
965+
});
966+
967+
it('should handle and preserve circular indirect relationships', () => {
968+
const document = {
969+
definitions: {
970+
User: {
971+
$ref: '#/definitions/Editor',
972+
description: 'Some User with a basic set of permissions',
973+
},
974+
Admin: {
975+
$ref: '#/definitions/User',
976+
description: 'Some User with an elevated set of permissions',
977+
},
978+
Editor: {
979+
$ref: '#/definitions/Admin',
980+
description: 'Some User with write permissions',
981+
},
982+
},
983+
__target__: {
984+
type: 'object',
985+
properties: {
986+
user: {
987+
$ref: '#/definitions/Editor',
988+
},
989+
},
990+
required: ['user'],
991+
},
992+
};
993+
994+
const result = bundleTarget({
995+
document,
996+
path: '#/__target__',
997+
});
998+
999+
expect(result).toEqual({
1000+
type: 'object',
1001+
properties: {
1002+
user: {
1003+
$ref: `#/${BUNDLE_ROOT}/Editor`,
1004+
},
1005+
},
1006+
required: ['user'],
1007+
[BUNDLE_ROOT]: {
1008+
User: {
1009+
$ref: `#/${BUNDLE_ROOT}/Editor`,
1010+
description: 'Some User with a basic set of permissions',
1011+
},
1012+
Admin: {
1013+
$ref: `#/${BUNDLE_ROOT}/User`,
1014+
description: 'Some User with an elevated set of permissions',
1015+
},
1016+
Editor: {
1017+
$ref: `#/${BUNDLE_ROOT}/Admin`,
1018+
description: 'Some User with write permissions',
1019+
},
1020+
},
1021+
});
1022+
});
1023+
9231024
describe('when custom keyProvider is provided', () => {
9241025
it('should work', () => {
9251026
const document = {

‎src/bundle.ts

Lines changed: 69 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -67,89 +67,99 @@ const bundle = (document: unknown, bundleRoot: JsonPath, errorsRoot: JsonPath, k
6767
const $refTarget = pointerToPath(path);
6868
const objectToBundle = get(document, $refTarget);
6969

70-
traverse(cur ? cur : objectToBundle, ({ parent }) => {
71-
if (hasRef(parent) && isLocalRef(parent.$ref)) {
72-
const $ref = parent.$ref;
73-
if (errorsObj[$ref]) return;
70+
traverse(cur ? cur : objectToBundle, {
71+
onEnter: ({ value: parent }) => {
72+
if (hasRef(parent) && isLocalRef(parent.$ref)) {
73+
const $ref = parent.$ref;
74+
if (errorsObj[$ref]) return;
75+
76+
if ($ref === path) {
77+
bundledRefInventory[$ref] = '#';
78+
}
7479

75-
if (bundledRefInventory[$ref]) {
76-
parent.$ref = bundledRefInventory[$ref];
80+
if (bundledRefInventory[$ref]) {
81+
parent.$ref = bundledRefInventory[$ref];
7782

78-
// no need to continue, this $ref has already been bundled
79-
return;
80-
}
83+
// no need to continue, this $ref has already been bundled
84+
return;
85+
}
8186

82-
let _path;
83-
let inventoryPath;
84-
let inventoryKey;
85-
let inventoryRef;
87+
let _path;
88+
let inventoryPath;
89+
let inventoryKey;
90+
let inventoryRef;
8691

87-
try {
88-
_path = pointerToPath($ref);
92+
try {
93+
_path = pointerToPath($ref);
8994

90-
let _inventoryKey;
91-
if (keyProvider) {
92-
_inventoryKey = keyProvider({ document, path: _path });
93-
}
95+
let _inventoryKey;
96+
if (keyProvider) {
97+
_inventoryKey = keyProvider({ document, path: _path });
98+
}
9499

95-
if (!_inventoryKey) {
96-
_inventoryKey = defaultKeyProvider({ document, path: _path });
97-
}
100+
if (!_inventoryKey) {
101+
_inventoryKey = defaultKeyProvider({ document, path: _path });
102+
}
98103

99-
inventoryKey = _inventoryKey;
104+
inventoryKey = _inventoryKey;
100105

101-
let i = 1;
102-
while (takenKeys.has(inventoryKey)) {
103-
i++;
104-
inventoryKey = `${_inventoryKey}_${i}`;
106+
let i = 1;
107+
while (takenKeys.has(inventoryKey)) {
108+
i++;
109+
inventoryKey = `${_inventoryKey}_${i}`;
105110

106-
if (i > 20) {
107-
throw new Error(`Keys ${_inventoryKey}_2 through ${_inventoryKey}_${20} already taken.`);
111+
if (i > 20) {
112+
throw new Error(`Keys ${_inventoryKey}_2 through ${_inventoryKey}_${20} already taken.`);
113+
}
108114
}
109-
}
110115

111-
takenKeys.add(inventoryKey);
116+
takenKeys.add(inventoryKey);
112117

113-
inventoryPath = [...bundleRoot, inventoryKey];
118+
inventoryPath = [...bundleRoot, inventoryKey];
114119

115-
inventoryRef = pathToPointer(inventoryPath);
116-
} catch (error) {
117-
errorsObj[$ref] = error instanceof Error ? error.message : String(error);
118-
}
120+
inventoryRef = pathToPointer(inventoryPath);
121+
} catch (error) {
122+
errorsObj[$ref] = error instanceof Error ? error.message : String(error);
123+
}
119124

120-
// Ignore invalid $refs and carry on
121-
if (!_path || !inventoryPath || !inventoryRef) return;
125+
// Ignore invalid $refs and carry on
126+
if (!_path || !inventoryPath || !inventoryRef) return;
122127

123-
let bundled$Ref: unknown;
124-
if (typeof document === 'object' && document !== null) {
125-
try {
126-
bundled$Ref = resolveInlineRef(Object(document), $ref);
127-
} catch {
128+
let bundled$Ref: unknown;
129+
if (typeof document === 'object' && document !== null) {
130+
// check the simple way first, to preserve these relationships when possible
128131
bundled$Ref = get(document, _path);
132+
133+
if (!bundled$Ref) {
134+
try {
135+
// if we could not find it with a simple lookup, check for deep refs etc via resolveInlineRef
136+
bundled$Ref = resolveInlineRef(Object(document), $ref);
137+
} catch {}
138+
}
129139
}
130-
}
131140

132-
if (bundled$Ref !== void 0) {
133-
bundledRefInventory[$ref] = inventoryRef;
134-
parent.$ref = inventoryRef;
141+
if (bundled$Ref !== void 0) {
142+
bundledRefInventory[$ref] = inventoryRef;
143+
parent.$ref = inventoryRef;
135144

136-
if (!has(bundledObj, inventoryPath)) {
137-
if (Array.isArray(bundled$Ref)) {
138-
set(bundledObj, inventoryPath, new Array(bundled$Ref.length).fill(null));
139-
} else if (typeof bundled$Ref === 'object') {
140-
setWith(bundledObj, inventoryPath, {}, Object);
141-
}
145+
if (!has(bundledObj, inventoryPath)) {
146+
if (Array.isArray(bundled$Ref)) {
147+
set(bundledObj, inventoryPath, new Array(bundled$Ref.length).fill(null));
148+
} else if (typeof bundled$Ref === 'object') {
149+
setWith(bundledObj, inventoryPath, {}, Object);
150+
}
142151

143-
set(bundledObj, inventoryPath, bundled$Ref);
152+
set(bundledObj, inventoryPath, bundled$Ref);
144153

145-
if (!stack[$ref]) {
146-
stack[$ref] = true;
147-
_bundle(path, stack, bundled$Ref, bundledRefInventory, bundledObj, errorsObj);
148-
stack[$ref] = false;
154+
if (!stack[$ref]) {
155+
stack[$ref] = true;
156+
_bundle(path, stack, bundled$Ref, bundledRefInventory, bundledObj, errorsObj);
157+
stack[$ref] = false;
158+
}
149159
}
150160
}
151161
}
152-
}
162+
},
153163
});
154164

155165
const finalObjectToBundle = get(bundledObj, bundleRoot);

0 commit comments

Comments
 (0)