Skip to content

Commit d499706

Browse files
maxjayclaude
andauthored
fix: don't duplicate field-level ops when filtering by child array path (v0.17.2) (#36)
* fix: don't duplicate field-level ops when filtering by child array path flattenOpsForFilter recursed into Replace.changes even when the Replace op itself already matched the prefix, causing field-level ops to appear alongside their parent element op in the results (e.g. changing one field on a child item produced both a Replace for the child AND a loose Replace for the field). Replace with filterOpsForPrefixes: include a matching op as-is (its .changes are already attached), and only recurse into Replace.changes when the Replace itself does NOT match the prefix. https://claude.ai/code/session_017H6NzxYAwbmKxHH6QusCdh * chore: bump version to 0.17.2 https://claude.ai/code/session_017H6NzxYAwbmKxHH6QusCdh --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 414d13e commit d499706

4 files changed

Lines changed: 30 additions & 12 deletions

File tree

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@maxjay/patchwork",
3-
"version": "0.17.1",
3+
"version": "0.17.2",
44
"description": "A JSON editing engine with base/draft, diff, undo, and scoped lenses.",
55
"type": "module",
66
"main": "dist/index.js",

src/engine.array-semantics.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,4 +573,17 @@ describe('diff — includeUnchanged propagation into nested keyed arrays', () =>
573573
expect(ops).toHaveLength(1);
574574
expect(ops[0]).toMatchObject({ op: OpType.Unchanged, identity: 'DELTA' });
575575
});
576+
577+
it('field change on one child: returns one Replace op for that child — not a duplicate field-level op', () => {
578+
const e = makeOrdersEngine();
579+
e.replace('$.items[0].children[1].qty', 99); // change BETA's qty
580+
const ops = e.diff("$.items[0].children", { includeUnchanged: true });
581+
// Expect exactly one op per child — no field-level replace leaking out alongside the element Replace
582+
expect(ops).toHaveLength(3);
583+
const betaOp = ops.find((o: any) => o.identity === 'BETA');
584+
expect(betaOp).toBeDefined();
585+
expect(betaOp!.op).toBe(OpType.Replace);
586+
expect((betaOp as any).changes).toHaveLength(1); // field-level replace lives inside .changes
587+
expect(ops.filter(o => o.op === OpType.Replace)).toHaveLength(1); // not duplicated at top level
588+
});
576589
});

src/engine.ts

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -447,19 +447,24 @@ export class Engine<T extends JsonValue = JsonValue> {
447447
this.diffNode(this.base, this.draft, '$', ops, includeUnchanged, cascade, false);
448448
if (!path) return ops;
449449
const prefixes = [...new Set([...paths(this.draft, path), ...paths(this.base, path)])];
450-
// Flatten Replace.changes recursively so path filters can reach ops nested inside
451-
// changed parent elements (e.g. querying a child keyed array when its parent changed).
452-
return this.flattenOpsForFilter(ops).filter(op => prefixes.some(p => isUnderPrefix(opPath(op), p)));
450+
return this.filterOpsForPrefixes(ops, prefixes);
453451
}
454452

455-
private flattenOpsForFilter(ops: DiffOp[]): DiffOp[] {
456-
const flat: DiffOp[] = [];
453+
// Filters ops to those at-or-under any prefix. When a Replace op itself does NOT
454+
// match but its changes might (e.g. querying a child keyed array inside a changed
455+
// parent element), recurse into Replace.changes rather than skipping the whole subtree.
456+
// When a Replace op DOES match, include it without recursing — its .changes are
457+
// already attached and recursing would duplicate the field-level ops in the result.
458+
private filterOpsForPrefixes(ops: DiffOp[], prefixes: string[]): DiffOp[] {
459+
const result: DiffOp[] = [];
457460
for (const op of ops) {
458-
flat.push(op);
459-
if (op.op === OpType.Replace && op.changes?.length)
460-
flat.push(...this.flattenOpsForFilter(op.changes));
461+
if (prefixes.some(p => isUnderPrefix(opPath(op), p))) {
462+
result.push(op);
463+
} else if (op.op === OpType.Replace && op.changes?.length) {
464+
result.push(...this.filterOpsForPrefixes(op.changes, prefixes));
465+
}
461466
}
462-
return flat;
467+
return result;
463468
}
464469

465470
private moveOrCopy(from: string, to: string, isMove: boolean): void {

0 commit comments

Comments
 (0)