diff --git a/README.md b/README.md index ed14a94..38cb6fa 100644 --- a/README.md +++ b/README.md @@ -94,7 +94,7 @@ const merge = createMerger({ `keyBy` maps a list path to its identity field. Identity values must be stable, unique strings or numbers. Matching is strict, so `1` and `"1"` are different items, while `0` and `""` are valid identities. Missing, duplicate, `NaN`, and non-string/non-number identities throw. -`replace` makes a path swap wholesale instead of deep-merging or reconciling. A wholesale swap never recurses, so `createMerger` rejects any policy nested below a replaced path. The `"order.items[]"` form is the item-swap idiom: the list still matches items by identity, but each matched item is replaced by its incoming value instead of merged: +`replace` makes a path swap wholesale instead of deep-merging or reconciling. A wholesale swap never recurses, so `createMerger` rejects any policy nested below a replaced path. Keying and replacing the same path is rejected too: one says to enter the list and the other says it is opaque, so they contradict rather than nest. The `"order.items[]"` form is the item-swap idiom: the list still matches items by identity, but each matched item is replaced by its incoming value instead of merged: ```ts const replaceItems = createMerger({ @@ -182,7 +182,7 @@ Paths are dot-separated property names. `[]` means “inside each keyed item of There are no wildcards, indices, root tokens, or escaping. Properties containing `.`, `[`, or `]` are not addressable in v1. The root cannot be keyed; wrap a top-level array in an object when it needs reconciliation. -All configuration is validated when the merger is created: bad grammar, reserved names, duplicates, `[]` segments under lists that have no key, and policies made unreachable by a broader `replace` all throw. Paths are never checked against `T` or runtime data. +All configuration is validated when the merger is created: bad grammar, reserved names, duplicates, `[]` segments under lists that have no key, policies made unreachable by a broader `replace`, and a path that is both keyed and replaced all throw. Paths are never checked against `T` or runtime data. ## Semantics diff --git a/src/paths.ts b/src/paths.ts index d4b614f..1368818 100644 --- a/src/paths.ts +++ b/src/paths.ts @@ -87,11 +87,24 @@ export function compileOptions(options: MergeOptions): CompiledOptions { // itself; the reverse nesting ('[]' under a keyBy list) is the // load-bearing item-swap idiom and passes. for (const keyed of keyPolicies) { - if (isPrefix(policy.segments, keyed.segments)) { + if (!isPrefix(policy.segments, keyed.segments)) continue; + // Equal paths do not nest. Neither policy shadows the other: one says + // the list is opaque and the other says to enter it, so the config + // contradicts itself rather than stranding a subtree. + // A keyBy path never ends in '[]', so appending it always names the + // item-swap spelling, which is not guessable from the error. Name the + // spelling and stop there: what else the caller has configured decides + // whether the rest compiles, and this message cannot know that. + if (policy.segments.length === keyed.segments.length) { throw new KeyfoldConfigError( - `replace path '${policy.path}' makes keyBy path '${keyed.path}' unreachable`, + `path '${policy.path}' cannot be both keyed and replaced: keyBy enters the list, ` + + `replace treats it as opaque; replacing matched items instead is spelled ` + + `'${policy.path}[]'`, ); } + throw new KeyfoldConfigError( + `replace path '${policy.path}' makes keyBy path '${keyed.path}' unreachable`, + ); } for (const other of replacePolicies) { if ( diff --git a/test/config.test.ts b/test/config.test.ts index c50ada9..dadc14c 100644 --- a/test/config.test.ts +++ b/test/config.test.ts @@ -46,7 +46,6 @@ describe("configuration validation", () => { }); const unreachableOptions: MergeOptions[] = [ - { keyBy: { "order.items": "id" }, replace: ["order.items"] }, { keyBy: { "order.items": "id" }, replace: ["order"] }, { keyBy: { "order.items": "id", "order.items[].components": "sku" }, @@ -61,6 +60,30 @@ describe("configuration validation", () => { }, ); + test("names the item-swap idiom when one path is both keyed and replaced", () => { + // Nothing is stranded here, so 'unreachable' would misdescribe it: the two + // policies contradict each other at the same node. The message has to name + // the item-swap spelling, because that spelling is not guessable. + const collide = () => + createMerger({ keyBy: { "order.items": "id" }, replace: ["order.items"] }); + + expect(collide).toThrow(KeyfoldConfigError); + expect(collide).toThrow(/cannot be both keyed and replaced/); + expect(collide).toThrow(/'order\.items\[\]'/); + expect(collide).not.toThrow(/unreachable/); + }); + + test("names the item-swap spelling without prescribing the rest of the config", () => { + // The message reports one collision; it cannot know whether the caller's + // other policies also collide. Naming a spelling stays true either way, + // where telling them what to do would not. + const alsoCollidesElsewhere = () => + createMerger({ keyBy: { items: "id" }, replace: ["items", "items[].parts"] }); + + expect(alsoCollidesElsewhere).toThrow(/'items\[\]'/); + expect(alsoCollidesElsewhere).not.toThrow(/\b(drop|remove|use replace path)\b/); + }); + test("rejects a replace path shadowed by a broader replace path", () => { expect(() => createMerger({ replace: ["a", "a.b"] })).toThrow(/unreachable/); expect(() => createMerger({ replace: ["a.b", "a"] })).toThrow(/unreachable/);