Skip to content

Commit 33fcdca

Browse files
committed
refactor(paths): one identityOf/findByIdentity for all identity matching
Self-review finding #3: 'read this element's identity' and 'find the element with this identity' were each implemented inline five times (diff map building, items, canonicalizeSegs, resolveCanonical, ghostInsertIndex). They now have exactly one definition each in paths.ts; every matcher routes through them. No behavior change — all 253 tests pass untouched. https://claude.ai/code/session_017WayuSMxaaoipNgVpHccCT
1 parent c7fb2f1 commit 33fcdca

2 files changed

Lines changed: 24 additions & 15 deletions

File tree

‎src/engine.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ export class Engine<T extends JsonValue = JsonValue> {
467467
.map(r => r.segments[segments.length] as number));
468468
const sequence = draftArr
469469
.filter((_, i) => !removedIdx.has(i))
470-
.map(item => identityOf(item, seg));
470+
.map(item => identityOf(item, seg.key));
471471
arr = { segments, sequence };
472472
arrays.set(parentKey, arr);
473473
}
@@ -805,7 +805,7 @@ export class Engine<T extends JsonValue = JsonValue> {
805805
private buildIdentityMap(arr: JsonValue[], key: string, segs: Seg[]): Map<JsonValue, JsonValue> {
806806
const map = new Map<JsonValue, JsonValue>();
807807
for (const item of arr) {
808-
const id = isPlainObject(item) ? item[key] : undefined;
808+
const id = identityOf(item, key);
809809
if (id === undefined || (id !== null && typeof id === 'object')) {
810810
throw new Error(
811811
`diff: item in keyed array at ${segsToPath(segs)} has no primitive '${key}' identity`

‎src/paths.ts‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,15 @@ export function canonicalizeSegs(
124124
for (const seg of segments) {
125125
if (typeof seg === 'number') {
126126
const key = Array.isArray(cur) ? keyMap.get(pattern) : undefined;
127-
const item = Array.isArray(cur) ? cur[seg] : undefined;
128-
if (key === '$self') out.push({ key: null, value: item });
129-
else if (key !== undefined && isPlainObject(item) && item[key] !== undefined) out.push({ key, value: item[key] });
130-
else out.push(seg);
127+
if (key === '$self') {
128+
out.push({ key: null, value: cur[seg] });
129+
} else if (key !== undefined) {
130+
const id = identityOf(cur[seg], key);
131+
if (id !== undefined) out.push({ key, value: id });
132+
else out.push(seg);
133+
} else {
134+
out.push(seg);
135+
}
131136
pattern = patternElement(pattern);
132137
} else {
133138
out.push(seg);
@@ -149,9 +154,7 @@ export function resolveCanonical(doc: JsonValue, segs: Seg[]): (string | number)
149154
if (cur === null || typeof cur !== 'object') return undefined;
150155
if (typeof seg === 'object') {
151156
if (!Array.isArray(cur)) return undefined;
152-
const idx = cur.findIndex((item: any) =>
153-
seg.key === null ? item === seg.value : isPlainObject(item) && item[seg.key] === seg.value,
154-
);
157+
const idx = findByIdentity(cur, seg);
155158
if (idx === -1) return undefined;
156159
out.push(idx);
157160
cur = cur[idx];
@@ -168,10 +171,16 @@ export function resolveCanonical(doc: JsonValue, segs: Seg[]): (string | number)
168171
return out;
169172
}
170173

171-
// Reads the identity of an array element under a given identity segment's
172-
// key ($self: the item is its own identity).
173-
export function identityOf(item: JsonValue, seg: IdentitySeg): JsonValue | undefined {
174-
return seg.key === null ? item : isPlainObject(item) ? item[seg.key] : undefined;
174+
// Reads the identity of an array element under a key (null = $self: the
175+
// item is its own identity). The single definition of what "identity" means
176+
// for an element — every matcher in the codebase goes through this.
177+
export function identityOf(item: JsonValue, key: string | null): JsonValue | undefined {
178+
return key === null ? item : isPlainObject(item) ? item[key] : undefined;
179+
}
180+
181+
// Index of the element matching an identity segment, or -1.
182+
export function findByIdentity(arr: JsonValue[], seg: IdentitySeg): number {
183+
return arr.findIndex(item => identityOf(item, seg.key) === seg.value);
175184
}
176185

177186
// Where a reverted removal re-inserts: directly after the nearest preceding
@@ -180,9 +189,9 @@ export function identityOf(item: JsonValue, seg: IdentitySeg): JsonValue | undef
180189
// un-deleted element comes back next to the elements it lived beside instead
181190
// of teleporting to the end of the list.
182191
export function ghostInsertIndex(baseArr: JsonValue[], sequence: Array<JsonValue | undefined>, seg: IdentitySeg): number {
183-
const baseIdx = baseArr.findIndex(item => identityOf(item, seg) === seg.value);
192+
const baseIdx = findByIdentity(baseArr, seg);
184193
for (let i = baseIdx - 1; i >= 0; i--) {
185-
const neighborId = identityOf(baseArr[i], seg);
194+
const neighborId = identityOf(baseArr[i], seg.key);
186195
const seqIdx = neighborId === undefined ? -1 : sequence.indexOf(neighborId);
187196
if (seqIdx !== -1) return seqIdx + 1;
188197
}

0 commit comments

Comments
 (0)