Skip to content

Commit bfeea95

Browse files
committed
perf(groups): identity-preserving mapObjectById and updateObjects walk
Both tree walks always returned new arrays / new group nodes even when no descendant actually changed. That broke React identity for the unaffected portions of the tree, defeating any downstream memoisation. Added the 'changed' flag pattern: the function returns the original array when nothing in it (or below it) was touched, otherwise the rebuilt one. Group nodes likewise keep their reference when their children walk returns by reference. Picked up four Group.test cases to lock the identity behaviour: - mapper applied at the matching node - top-level array preserved when id is missing - untouched group subtrees preserved by reference - mapper that returns the same node leaves the array untouched
1 parent 58d759a commit bfeea95

3 files changed

Lines changed: 68 additions & 12 deletions

File tree

‎src/store/labelStore.ts‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -318,19 +318,26 @@ export const useLabelStore = create<LabelState>()(
318318
set((state) => {
319319
if (updates.length === 0) return {};
320320
// Single tree walk that applies every queued change in one
321-
// pass. The previous reduce-over-mapObjectById approach was
322-
// O(updates × tree) and fired per frame during multi-object
323-
// drag; this is O(tree).
321+
// pass: O(tree) instead of O(updates × tree). Identity-
322+
// preserving — subtrees with no matching id keep their
323+
// reference so React memoisation can skip them.
324324
const updateMap = new Map(updates.map((u) => [u.id, u.changes]));
325-
const applyUpdates = (nodes: LabelObject[]): LabelObject[] =>
326-
nodes.map((n) => {
325+
const applyUpdates = (nodes: LabelObject[]): LabelObject[] => {
326+
let changed = false;
327+
const next = nodes.map((n) => {
327328
const changes = updateMap.get(n.id);
328-
const updated = changes ? applyObjectChanges(n, changes) : n;
329+
let updated = changes ? applyObjectChanges(n, changes) : n;
329330
if (isGroup(updated)) {
330-
return { ...updated, children: applyUpdates(updated.children) };
331+
const nextChildren = applyUpdates(updated.children);
332+
if (nextChildren !== updated.children) {
333+
updated = { ...updated, children: nextChildren };
334+
}
331335
}
336+
if (updated !== n) changed = true;
332337
return updated;
333338
});
339+
return changed ? next : nodes;
340+
};
334341
return updateCurrentObjects(state, (objs) => applyUpdates(objs));
335342
}),
336343

‎src/types/Group.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
detachObjectById,
1111
isSelfOrDescendant,
1212
canGroupSelection,
13+
mapObjectById,
1314
type GroupObject,
1415
} from './Group';
1516
import type { LabelObject } from '../registry';
@@ -200,6 +201,38 @@ describe('Group helpers', () => {
200201
});
201202
});
202203

204+
describe('mapObjectById', () => {
205+
it('applies the mapper to the matching node', () => {
206+
const tree = [leaf('a'), leaf('b')];
207+
const next = mapObjectById(tree, 'a', (o) => ({ ...o, x: 99 }));
208+
expect(next[0]?.x).toBe(99);
209+
expect(next[1]).toBe(tree[1]); // sibling unchanged
210+
});
211+
212+
it('preserves the top-level array reference when no id matches', () => {
213+
const tree = [leaf('a'), leaf('b')];
214+
const next = mapObjectById(tree, 'missing', (o) => ({ ...o, x: 99 }));
215+
expect(next).toBe(tree);
216+
});
217+
218+
it('preserves untouched group subtrees by reference', () => {
219+
const inner = leaf('inside');
220+
const tree = [group('g1', [inner]), group('g2', [leaf('other')])];
221+
const next = mapObjectById(tree, 'inside', (o) => ({ ...o, x: 5 }));
222+
// g2 wasn't touched — its node identity stays.
223+
expect(next[1]).toBe(tree[1]);
224+
// g1 is rebuilt (its children array changed), but the unchanged
225+
// sibling inside g1 — there's none — would also keep identity.
226+
expect(next[0]).not.toBe(tree[0]);
227+
});
228+
229+
it('preserves the array reference when the mapper returns the same node', () => {
230+
const tree = [leaf('a')];
231+
const next = mapObjectById(tree, 'a', (o) => o);
232+
expect(next).toBe(tree);
233+
});
234+
});
235+
203236
describe('expandSelection', () => {
204237
it('passes leaf ids through unchanged', () => {
205238
expect(expandSelection([leaf('a'), leaf('b')], ['a'])).toEqual(['a']);

‎src/types/Group.ts‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,19 +92,35 @@ export function selectionTargetId(objects: LabelObject[], id: string): string {
9292
* Returns a new tree with the node identified by `id` replaced by
9393
* `mapper(node)`. Walks recursively into groups so this is the one
9494
* code path the store needs to mutate either top-level objects or
95-
* leaves nested inside groups. Unmatched leaves keep their object
96-
* identity so per-object React memoisation still works.
95+
* leaves nested inside groups.
96+
*
97+
* Identity-preserving: subtrees that don't contain the target id —
98+
* and the top-level array itself when no match is found — keep their
99+
* original references. That lets React memoisation skip unaffected
100+
* branches when a single leaf updates.
97101
*/
98102
export function mapObjectById(
99103
objects: LabelObject[],
100104
id: string,
101105
mapper: (obj: LabelObject) => LabelObject,
102106
): LabelObject[] {
103-
return objects.map((o) => {
104-
if (o.id === id) return mapper(o);
105-
if (isGroup(o)) return { ...o, children: mapObjectById(o.children, id, mapper) };
107+
let changed = false;
108+
const next = objects.map((o) => {
109+
if (o.id === id) {
110+
const updated = mapper(o);
111+
if (updated !== o) changed = true;
112+
return updated;
113+
}
114+
if (isGroup(o)) {
115+
const nextChildren = mapObjectById(o.children, id, mapper);
116+
if (nextChildren !== o.children) {
117+
changed = true;
118+
return { ...o, children: nextChildren };
119+
}
120+
}
106121
return o;
107122
});
123+
return changed ? next : objects;
108124
}
109125

110126
/**

0 commit comments

Comments
 (0)