Skip to content

Commit ede790b

Browse files
AlpAlp
authored andcommitted
test: add re-render identity preservation tests for ArrayView and React
Adds comprehensive tests verifying that the immutable applyChange pipeline preserves object identity for unchanged nodes (enabling React.memo) while correctly bubbling new references up for changed descendants. NOTE: Several tests fail on main due to expandNode eagerly materializing all relationships (breaking identity). These pass with PR rocicorp#5605 which removes expandNode. The two PRs should be merged together. * ArrayView flat list: edit/add/remove preserve sibling identity * Relationship propagation: child edit/add/remove bubble new refs to parent * 3-level deep: grandchild edit bubbles through entire ancestor chain * React snapshot identity: getSnapshot stability, sentinel reuse * React.memo render counting: only changed row children re-render * Data flash prevention: no empty snapshots between data updates
1 parent cc7147b commit ede790b

File tree

2 files changed

+648
-0
lines changed

2 files changed

+648
-0
lines changed
Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
import React, {memo, useSyncExternalStore} from 'react';
2+
import {createRoot, type Root} from 'react-dom/client';
3+
import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest';
4+
import {queryInternalsTag, type QueryImpl} from './bindings.ts';
5+
import {ViewStore} from './use-query.tsx';
6+
import type {
7+
ErroredQuery,
8+
Query,
9+
ResultType,
10+
Schema,
11+
Zero,
12+
} from './zero.ts';
13+
14+
// ─── Test helpers ───────────────────────────────────────────────────────────
15+
16+
function newMockQuery(query: string, singular = false): Query<string, Schema> {
17+
return {
18+
[queryInternalsTag]: true,
19+
hash: () => query,
20+
format: {singular},
21+
} as unknown as QueryImpl<string, Schema>;
22+
}
23+
24+
type MockView = {
25+
listeners: Set<
26+
(data: unknown, resultType: ResultType, error?: ErroredQuery) => void
27+
>;
28+
addListener(
29+
cb: (data: unknown, resultType: ResultType, error?: ErroredQuery) => void,
30+
): () => void;
31+
destroy(): void;
32+
updateTTL(): void;
33+
};
34+
35+
function newView(): MockView {
36+
return {
37+
listeners: new Set(),
38+
addListener(cb) {
39+
this.listeners.add(cb);
40+
return () => { this.listeners.delete(cb); };
41+
},
42+
destroy() { this.listeners.clear(); },
43+
updateTTL() {},
44+
};
45+
}
46+
47+
function newMockZero(clientID: string): Zero<Schema, undefined, unknown> {
48+
const view = newView();
49+
return {
50+
clientID,
51+
materialize: vi.fn().mockImplementation(() => view),
52+
} as unknown as Zero<Schema, undefined, unknown>;
53+
}
54+
55+
function getListeners(zero: Zero<Schema, undefined, unknown>, index = 0) {
56+
const result = vi.mocked(zero.materialize).mock.results[index]?.value as MockView | undefined;
57+
if (!result) throw new Error('materialize was not called');
58+
return result.listeners;
59+
}
60+
61+
function createView(viewStore: ViewStore, suffix: string) {
62+
const q = newMockQuery(`q-${suffix}`);
63+
const zero = newMockZero(`client-${suffix}`);
64+
const view = viewStore.getView(zero, q, true, 'forever');
65+
const cleanup = view.subscribeReactInternals(() => {});
66+
return {view, zero, q, cleanup};
67+
}
68+
69+
// ─── Snapshot identity ──────────────────────────────────────────────────────
70+
71+
describe('Snapshot identity', () => {
72+
beforeEach(() => { vi.useFakeTimers(); });
73+
afterEach(() => { vi.useRealTimers(); });
74+
75+
test('getSnapshot returns same reference without data changes, new reference after', () => {
76+
const viewStore = new ViewStore();
77+
const {view, zero, cleanup} = createView(viewStore, 'identity');
78+
79+
// Same ref before data
80+
expect(view.getSnapshot()).toBe(view.getSnapshot());
81+
82+
// Push data
83+
getListeners(zero).forEach(cb => cb([{id: '1'}], 'unknown'));
84+
const withData = view.getSnapshot();
85+
86+
// Same ref after data (no further changes)
87+
expect(view.getSnapshot()).toBe(withData);
88+
89+
// New ref after new data
90+
getListeners(zero).forEach(cb => cb([{id: '1'}, {id: '2'}], 'unknown'));
91+
expect(view.getSnapshot()).not.toBe(withData);
92+
93+
cleanup();
94+
});
95+
96+
test('empty snapshots use sentinel objects (no spurious re-renders)', () => {
97+
const viewStore = new ViewStore();
98+
const {view, zero, cleanup} = createView(viewStore, 'sentinel');
99+
100+
const empty1 = view.getSnapshot();
101+
getListeners(zero).forEach(cb => cb([], 'unknown'));
102+
const empty2 = view.getSnapshot();
103+
104+
// Same sentinel reference for repeated empty data
105+
expect(empty1).toBe(empty2);
106+
107+
// Singular empty sentinel
108+
const qSingular = newMockQuery('singular', true);
109+
const zeroSingular = newMockZero('client-singular');
110+
const singular = viewStore.getView(zeroSingular, qSingular, true, 'forever');
111+
const cleanupSingular = singular.subscribeReactInternals(() => {});
112+
113+
const s1 = singular.getSnapshot();
114+
getListeners(zeroSingular).forEach(cb => cb(undefined, 'unknown'));
115+
expect(singular.getSnapshot()).toBe(s1);
116+
117+
cleanup();
118+
cleanupSingular();
119+
});
120+
121+
test('row identity preserved in snapshot: unchanged rows keep same reference', () => {
122+
const viewStore = new ViewStore();
123+
const {view, zero, cleanup} = createView(viewStore, 'row-identity');
124+
const listeners = getListeners(zero);
125+
126+
const row1 = {id: '1', name: 'Alice'};
127+
const row2 = {id: '2', name: 'Bob'};
128+
listeners.forEach(cb => cb([row1, row2], 'unknown'));
129+
130+
// Update only row2, keep row1 as same object
131+
const row2Updated = {id: '2', name: 'Bob Updated'};
132+
listeners.forEach(cb => cb([row1, row2Updated], 'unknown'));
133+
134+
const data = view.getSnapshot()[0] as Array<{id: string; name: string}>;
135+
expect(data[0]).toBe(row1);
136+
expect(data[1]).toBe(row2Updated);
137+
138+
cleanup();
139+
});
140+
});
141+
142+
// ─── Data flash prevention ──────────────────────────────────────────────────
143+
144+
describe('No data flash (data→empty→data)', () => {
145+
beforeEach(() => { vi.useFakeTimers(); });
146+
afterEach(() => { vi.useRealTimers(); });
147+
148+
test('snapshot never goes empty between data updates', () => {
149+
const viewStore = new ViewStore();
150+
const {view, zero, cleanup} = createView(viewStore, 'flash');
151+
const listeners = getListeners(zero);
152+
153+
const snapshots: unknown[] = [];
154+
view.subscribeReactInternals(() => {
155+
snapshots.push((view.getSnapshot()[0] as unknown[]).length);
156+
});
157+
158+
listeners.forEach(cb => cb([{id: '1'}], 'unknown'));
159+
listeners.forEach(cb => cb([{id: '1'}, {id: '2'}], 'unknown'));
160+
161+
// After first data, no snapshot should ever be empty
162+
let hadData = false;
163+
for (const len of snapshots) {
164+
if ((len as number) > 0) hadData = true;
165+
if (hadData) expect(len).toBeGreaterThan(0);
166+
}
167+
168+
cleanup();
169+
});
170+
171+
test('stale snapshot preserved after view destroy (no empty flash on remount)', () => {
172+
const viewStore = new ViewStore();
173+
const {view, zero, cleanup} = createView(viewStore, 'destroy-flash');
174+
175+
getListeners(zero).forEach(cb => cb([{id: '1'}], 'complete'));
176+
expect((view.getSnapshot()[0] as unknown[]).length).toBe(1);
177+
178+
cleanup();
179+
vi.advanceTimersByTime(15); // past 10ms cleanup timeout
180+
181+
// Stale snapshot still has data, not empty
182+
expect((view.getSnapshot()[0] as unknown[]).length).toBe(1);
183+
});
184+
});
185+
186+
// ─── React.memo render counting ─────────────────────────────────────────────
187+
188+
describe('React.memo child render counting', () => {
189+
let root: Root;
190+
let element: HTMLDivElement;
191+
let unique = 0;
192+
193+
beforeEach(() => {
194+
vi.useRealTimers();
195+
element = document.createElement('div');
196+
document.body.appendChild(element);
197+
root = createRoot(element);
198+
unique++;
199+
});
200+
201+
afterEach(() => {
202+
root.unmount();
203+
document.body.removeChild(element);
204+
});
205+
206+
test('only the changed row child re-renders; unchanged rows skip', async () => {
207+
const viewStore = new ViewStore();
208+
const q = newMockQuery(`react-memo-${unique}`);
209+
const zero = newMockZero(`client-memo-${unique}`);
210+
const parentRenders = {current: 0};
211+
const childRenders: Record<string, number> = {};
212+
213+
type Row = {id: string; name: string};
214+
215+
const ChildRow = memo(function ChildRow({row}: {row: Row}) {
216+
childRenders[row.id] = (childRenders[row.id] ?? 0) + 1;
217+
return <div data-testid={`row-${row.id}`}>{row.name}</div>;
218+
});
219+
220+
function Parent() {
221+
const viewRef = viewStore.getView(zero, q, true, 'forever');
222+
const [data] = useSyncExternalStore(
223+
viewRef.subscribeReactInternals,
224+
viewRef.getSnapshot,
225+
viewRef.getSnapshot,
226+
);
227+
parentRenders.current++;
228+
return (
229+
<div>{((data ?? []) as Row[]).map(row => <ChildRow key={row.id} row={row} />)}</div>
230+
);
231+
}
232+
233+
root.render(<Parent />);
234+
await expect.poll(() => parentRenders.current).toBeGreaterThanOrEqual(1);
235+
236+
// Push initial data
237+
const row1 = {id: '1', name: 'Alice'};
238+
const row2 = {id: '2', name: 'Bob'};
239+
getListeners(zero).forEach(cb => cb([row1, row2], 'unknown'));
240+
241+
await expect.poll(() => element.querySelector('[data-testid="row-1"]')?.textContent).toBe('Alice');
242+
243+
const rendersAfterData = parentRenders.current;
244+
const child1After = childRenders['1'] ?? 0;
245+
246+
// Update only row2, keep row1 as same reference
247+
getListeners(zero).forEach(cb => cb([row1, {id: '2', name: 'Bob Updated'}], 'unknown'));
248+
249+
await expect.poll(() => element.querySelector('[data-testid="row-2"]')?.textContent).toBe('Bob Updated');
250+
251+
// Parent re-renders (new snapshot tuple)
252+
expect(parentRenders.current).toBeGreaterThan(rendersAfterData);
253+
// Unchanged row1 child does NOT re-render
254+
expect(childRenders['1']).toBe(child1After);
255+
// Changed row2 child DOES re-render
256+
expect(childRenders['2']).toBeGreaterThan(1);
257+
});
258+
});

0 commit comments

Comments
 (0)