Skip to content

Commit 2548d0f

Browse files
pranko17claude
pranko17
and
claude
committed
fix(layout): invalidate cached row heights on in-place data swap
When the `data` prop is replaced with a different array of items (e.g. filter / sort / category change) without remounting the list, indices that were already measured retain their `isHeightMeasured`, `height` and `minHeight` from the previous data. The ViewHolder for the new item at that position then renders against the stale `minHeight`, and the LayoutManager places the next row at a Y derived from the stale `height`. In multi-column grids this surfaces as a taller new card overflowing into the row below. Track per-index `keyExtractor` results across renders. In `processDataUpdate`, clear `isHeightMeasured` and `minHeight` for any index whose key changed, so the next `modifyLayout` pass recomputes the row against the new content. No-op when `keyExtractor` is not provided. Tests cover: per-index identity change, full array replacement, re-emit of identical keys, append (pagination), and absence of `keyExtractor`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f4278ba commit 2548d0f

2 files changed

Lines changed: 227 additions & 0 deletions

File tree

‎src/__tests__/RecyclerViewManager.test.ts‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,166 @@ describe("RecyclerViewManager", () => {
7171
expect(consoleWarnSpy).not.toHaveBeenCalled();
7272
});
7373
});
74+
75+
describe("processDataUpdate — layout invalidation on in-place data swap", () => {
76+
interface Item {
77+
id: number;
78+
}
79+
const keyExtractor = (item: Item) => `item-${item.id}`;
80+
81+
const createGridManager = (data: Item[], overrides = {}) => {
82+
const props = {
83+
data,
84+
renderItem: jest.fn(),
85+
keyExtractor,
86+
numColumns: 2,
87+
...overrides,
88+
} as FlashListProps<Item>;
89+
const manager = new RecyclerViewManager<Item>(props);
90+
manager.processDataUpdate();
91+
manager.updateLayoutParams({ width: 400, height: 900 }, 0);
92+
manager.processDataUpdate();
93+
return manager;
94+
};
95+
96+
const markAsMeasured = (
97+
manager: RecyclerViewManager<Item>,
98+
indices: number[],
99+
height: number
100+
) => {
101+
for (const i of indices) {
102+
const layout = manager.getLayout(i);
103+
layout.isHeightMeasured = true;
104+
layout.minHeight = height;
105+
layout.height = height;
106+
}
107+
};
108+
109+
it("invalidates cached height for indices whose key changed", () => {
110+
const initialData: Item[] = [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }];
111+
const manager = createGridManager(initialData);
112+
markAsMeasured(manager, [0, 1, 2, 3], 200);
113+
114+
// Replace items at index 0 and 2 (different identities)
115+
const swappedData: Item[] = [
116+
{ id: 100 },
117+
initialData[1],
118+
{ id: 300 },
119+
initialData[3],
120+
];
121+
manager.updateProps({
122+
...manager.props,
123+
data: swappedData,
124+
});
125+
manager.processDataUpdate();
126+
127+
expect(manager.getLayout(0).isHeightMeasured).toBe(false);
128+
expect(manager.getLayout(0).minHeight).toBeUndefined();
129+
expect(manager.getLayout(2).isHeightMeasured).toBe(false);
130+
expect(manager.getLayout(2).minHeight).toBeUndefined();
131+
132+
// Unchanged identities keep their cached measurement
133+
expect(manager.getLayout(1).isHeightMeasured).toBe(true);
134+
expect(manager.getLayout(1).minHeight).toBe(200);
135+
expect(manager.getLayout(3).isHeightMeasured).toBe(true);
136+
expect(manager.getLayout(3).minHeight).toBe(200);
137+
});
138+
139+
it("invalidates every index when the whole array is replaced", () => {
140+
const initialData: Item[] = [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }];
141+
const manager = createGridManager(initialData);
142+
markAsMeasured(manager, [0, 1, 2, 3], 200);
143+
144+
// Category switch — entirely different items at every position
145+
const swappedData: Item[] = [
146+
{ id: 10 },
147+
{ id: 20 },
148+
{ id: 30 },
149+
{ id: 40 },
150+
];
151+
manager.updateProps({
152+
...manager.props,
153+
data: swappedData,
154+
});
155+
manager.processDataUpdate();
156+
157+
for (let i = 0; i < swappedData.length; i++) {
158+
expect(manager.getLayout(i).isHeightMeasured).toBe(false);
159+
expect(manager.getLayout(i).minHeight).toBeUndefined();
160+
}
161+
});
162+
163+
it("does not invalidate when the same items are re-emitted at the same positions", () => {
164+
const initialData: Item[] = [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }];
165+
const manager = createGridManager(initialData);
166+
markAsMeasured(manager, [0, 1, 2, 3], 200);
167+
168+
// New array reference, same identities — e.g. a query refetch returning equal data
169+
const sameData: Item[] = initialData.map((item) => ({ id: item.id }));
170+
manager.updateProps({
171+
...manager.props,
172+
data: sameData,
173+
});
174+
manager.processDataUpdate();
175+
176+
for (let i = 0; i < sameData.length; i++) {
177+
expect(manager.getLayout(i).isHeightMeasured).toBe(true);
178+
expect(manager.getLayout(i).minHeight).toBe(200);
179+
}
180+
});
181+
182+
it("does not invalidate measured heights for indices that survived an append", () => {
183+
const initialData: Item[] = [{ id: 1 }, { id: 2 }];
184+
const manager = createGridManager(initialData);
185+
markAsMeasured(manager, [0, 1], 200);
186+
187+
const appendedData: Item[] = [...initialData, { id: 3 }, { id: 4 }];
188+
manager.updateProps({
189+
...manager.props,
190+
data: appendedData,
191+
});
192+
manager.processDataUpdate();
193+
194+
// Pre-existing indices: keys unchanged, so invalidation must not reset
195+
// `isHeightMeasured` or `height` (the per-row normalization that runs
196+
// when new indices are appended is a separate concern).
197+
expect(manager.getLayout(0).isHeightMeasured).toBe(true);
198+
expect(manager.getLayout(0).height).toBe(200);
199+
expect(manager.getLayout(1).isHeightMeasured).toBe(true);
200+
expect(manager.getLayout(1).height).toBe(200);
201+
});
202+
203+
it("is a no-op when keyExtractor is not provided", () => {
204+
const initialData: Item[] = [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }];
205+
const props = {
206+
data: initialData,
207+
renderItem: jest.fn(),
208+
numColumns: 2,
209+
} as FlashListProps<Item>;
210+
const manager = new RecyclerViewManager<Item>(props);
211+
manager.processDataUpdate();
212+
manager.updateLayoutParams({ width: 400, height: 900 }, 0);
213+
manager.processDataUpdate();
214+
markAsMeasured(manager, [0, 1, 2, 3], 200);
215+
216+
const swappedData: Item[] = [
217+
{ id: 10 },
218+
{ id: 20 },
219+
{ id: 30 },
220+
{ id: 40 },
221+
];
222+
manager.updateProps({
223+
...manager.props,
224+
data: swappedData,
225+
});
226+
manager.processDataUpdate();
227+
228+
// Without stable keys we cannot tell positions apart safely, so we leave
229+
// the cached measurements alone (matching the pre-fix behavior).
230+
for (let i = 0; i < swappedData.length; i++) {
231+
expect(manager.getLayout(i).isHeightMeasured).toBe(true);
232+
expect(manager.getLayout(i).minHeight).toBe(200);
233+
}
234+
});
235+
});
74236
});

‎src/recyclerview/RecyclerViewManager.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,15 @@ export class RecyclerViewManager<T> {
3535
private _isDisposed = false;
3636
private _isLayoutManagerDirty = false;
3737
private _animationOptimizationsEnabled = false;
38+
/**
39+
* Snapshot of `keyExtractor(item, index)` results from the last
40+
* processDataUpdate call, indexed by position. Used to detect when the
41+
* item at a given index has been replaced by a different one on an
42+
* in-place data swap so we can invalidate that index's stale measured
43+
* height/minHeight and let it be remeasured against the new content.
44+
* See `invalidateChangedLayouts`.
45+
*/
46+
private prevDataKeys: string[] = [];
3847

3948
public firstItemOffset = 0;
4049
public ignoreScrollEvents = false;
@@ -301,12 +310,68 @@ export class RecyclerViewManager<T> {
301310

302311
processDataUpdate() {
303312
if (this.hasLayout()) {
313+
this.invalidateChangedLayouts();
304314
this.modifyChildrenLayout([], this.propsRef.data?.length ?? 0);
305315
if (this.hasRenderedProgressively && !this.recomputeEngagedIndices()) {
306316
// recomputeEngagedIndices will update the render stack if there are any changes in the engaged indices.
307317
// It's important to update render stack so that elements are assgined right keys incase items were deleted.
308318
this.updateRenderStack(this.engagedIndicesTracker.getEngagedIndices());
309319
}
320+
} else {
321+
this.recordCurrentDataKeys();
322+
}
323+
}
324+
325+
/**
326+
* Detects per-index identity changes between the previous and current data
327+
* arrays (using keyExtractor) and invalidates the cached measured height
328+
* for indices whose item changed.
329+
*
330+
* Without this, an in-place data swap (e.g. category/filter change) leaves
331+
* the LayoutManager believing every index is still measured at its previous
332+
* height. In a multi-column grid this leaks the previous row's tallest-item
333+
* constraint onto the new content: the ViewHolder applies the stale
334+
* `minHeight`, and the row's `y` for the next row is computed against the
335+
* stale height — so a taller new card overflows into the row below.
336+
*
337+
* Clearing `isHeightMeasured` ensures the next `modifyLayout` call sees
338+
* the index as needing a fresh recompute via
339+
* `computeEstimatesAndMinMaxChangedLayout`, and clearing `minHeight` stops
340+
* the stale constraint from being applied in the next render before the
341+
* first measurement arrives.
342+
*
343+
* No-op when `keyExtractor` is not provided (we cannot safely diff
344+
* positions without stable keys).
345+
*/
346+
private invalidateChangedLayouts(): void {
347+
if (!this.hasStableDataKeys() || !this.layoutManager) {
348+
this.recordCurrentDataKeys();
349+
return;
350+
}
351+
const newLength = this.getDataLength();
352+
const layoutCount = this.layoutManager.getLayoutCount();
353+
const compareLength = Math.min(layoutCount, newLength);
354+
for (let i = 0; i < compareLength; i++) {
355+
const oldKey = this.prevDataKeys[i];
356+
const newKey = this.getDataKey(i);
357+
if (oldKey !== undefined && oldKey !== newKey) {
358+
const layout = this.layoutManager.getLayout(i);
359+
layout.isHeightMeasured = false;
360+
layout.minHeight = undefined;
361+
}
362+
}
363+
this.recordCurrentDataKeys();
364+
}
365+
366+
private recordCurrentDataKeys(): void {
367+
if (!this.hasStableDataKeys()) {
368+
this.prevDataKeys.length = 0;
369+
return;
370+
}
371+
const newLength = this.getDataLength();
372+
this.prevDataKeys.length = newLength;
373+
for (let i = 0; i < newLength; i++) {
374+
this.prevDataKeys[i] = this.getDataKey(i);
310375
}
311376
}
312377

0 commit comments

Comments
 (0)