Skip to content

Commit 5730aed

Browse files
committed
fix(overlay): honour the immutable default for partial options
defaultApplyOptions only applied when the options argument was omitted entirely, so any partial object, applyAction(action, target, { strict: true }) or just { trace }, left options.immutable undefined, which read as falsy and applied the action in place. Callers passing a single unrelated option silently mutated their own document, contradicting the documented `immutable: true` default. The applying entry points now merge the caller's options over the defaults through withDefaults() and read the fields plainly. The uri realm documents the opposite default, so it passes its own fallback down explicitly rather than relying on undefined. Signed-off-by: Vladimir Gorej <vladimir.gorej@gmail.com>
1 parent 0b1c1f5 commit 5730aed

3 files changed

Lines changed: 66 additions & 10 deletions

File tree

packages/apidom-overlay/src/apply/realms/apidom.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,18 @@ interface InternalApplyOptions extends ApplyOptions {
3939

4040
const defaultApplyOptions: ApplyOptions = { immutable: true, strict: false };
4141

42+
/**
43+
* Merges caller options over the defaults. A partial options object leaves the
44+
* fields it omits undefined, which must still mean the documented defaults.
45+
* `immutable` is spelled out because it is the only default that is not falsy,
46+
* so a caller forwarding an undefined value must not end up mutating in place.
47+
*/
48+
const withDefaults = <T extends ApplyOptions>(options: T): T => ({
49+
...defaultApplyOptions,
50+
...options,
51+
immutable: options.immutable ?? defaultApplyOptions.immutable,
52+
});
53+
4254
/**
4355
* Default customMerge that enforces Overlay spec type compatibility.
4456
* Throws OverlayError on incompatible type combinations.
@@ -126,7 +138,8 @@ export const applyUpdateAction = (
126138
targetElement: Element,
127139
options: ApplyOptions = defaultApplyOptions,
128140
): Element => {
129-
let result = options.immutable ? cloneDeep(targetElement) : targetElement;
141+
const opts = withDefaults(options);
142+
let result = opts.immutable ? cloneDeep(targetElement) : targetElement;
130143

131144
for (const normalizedPath of normalizedPaths) {
132145
/**
@@ -141,7 +154,7 @@ export const applyUpdateAction = (
141154

142155
if (resolved === null) {
143156
// targeting root
144-
result = mergeValue(result, clonedUpdateValue, options);
157+
result = mergeValue(result, clonedUpdateValue, opts);
145158
continue;
146159
}
147160

@@ -150,16 +163,12 @@ export const applyUpdateAction = (
150163
if (isObjectElement(parent)) {
151164
const current = parent.get(key as string);
152165
if (current !== undefined) {
153-
parent.set(key as string, mergeValue(current, clonedUpdateValue, options));
166+
parent.set(key as string, mergeValue(current, clonedUpdateValue, opts));
154167
}
155168
} else if (isArrayElement(parent)) {
156169
const current = parent.get(key as number);
157170
if (current !== undefined) {
158-
(parent.content as Element[])[key as number] = mergeValue(
159-
current,
160-
clonedUpdateValue,
161-
options,
162-
);
171+
(parent.content as Element[])[key as number] = mergeValue(current, clonedUpdateValue, opts);
163172
}
164173
}
165174
}
@@ -201,7 +210,8 @@ export const applyRemoveAction = (
201210
targetElement: Element,
202211
options: ApplyOptions = defaultApplyOptions,
203212
): Element => {
204-
const result = options.immutable ? cloneDeep(targetElement) : targetElement;
213+
const opts = withDefaults(options);
214+
const result = opts.immutable ? cloneDeep(targetElement) : targetElement;
205215
// reverse document order so higher array indices are removed first (avoids index shifting)
206216
const sorted = [...normalizedPaths].reverse();
207217

packages/apidom-overlay/src/apply/realms/uri.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,10 @@ const applyOverlay = async (
9797
}
9898

9999
// apply overlay actions to the target (returns targetParseResult with result replaced)
100-
return applyOverlayApiDOM(overlayParseResult, targetParseResult, options) as ParseResultElement;
100+
return applyOverlayApiDOM(overlayParseResult, targetParseResult, {
101+
...options,
102+
immutable: options.immutable ?? defaultOptions.immutable,
103+
}) as ParseResultElement;
101104
};
102105

103106
export default applyOverlay;

packages/apidom-overlay/test/apply/realms/apidom.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,33 @@ describe('applyAction', function () {
504504
});
505505
});
506506

507+
context('partial options object', function () {
508+
specify('should keep the immutable default when other options are passed', function () {
509+
const action = refractAction({
510+
target: '$.info',
511+
update: { description: 'Added' },
512+
});
513+
const target = refract({ info: { title: 'Original' } });
514+
515+
const result = applyAction(action, target, { strict: true });
516+
517+
assert.strictEqual((toValue(result) as AnyJson).info.description, 'Added');
518+
assert.isUndefined((toValue(target) as AnyJson).info.description);
519+
});
520+
521+
specify('should keep the immutable default when immutable is undefined', function () {
522+
const action = refractAction({
523+
target: '$.info',
524+
update: { description: 'Added' },
525+
});
526+
const target = refract({ info: { title: 'Original' } });
527+
528+
applyAction(action, target, { immutable: undefined });
529+
530+
assert.isUndefined((toValue(target) as AnyJson).info.description);
531+
});
532+
});
533+
507534
context('mutable mode', function () {
508535
specify('should mutate original target on update', function () {
509536
const action = refractAction({
@@ -751,6 +778,22 @@ describe('applyOverlayApiDOM', function () {
751778
});
752779
});
753780

781+
context('partial options object', function () {
782+
specify('should keep the immutable default when other options are passed', function () {
783+
const overlay = refractOverlay1({
784+
overlay: '1.1.0',
785+
info: { title: 'Test', version: '1.0.0' },
786+
actions: [{ target: '$.info', update: { description: 'Added' } }],
787+
});
788+
const target = refract({ info: { title: 'Original' } });
789+
790+
const result = applyOverlayApiDOM(overlay, target, { strict: true });
791+
792+
assert.strictEqual((toValue(result) as AnyJson).info.description, 'Added');
793+
assert.isUndefined((toValue(target) as AnyJson).info.description);
794+
});
795+
});
796+
754797
context('empty actions', function () {
755798
specify('should return target unchanged', function () {
756799
const overlay = refractOverlay1({

0 commit comments

Comments
 (0)