Skip to content

Commit 532cc60

Browse files
committed
Sync ItemSeparator props on cell re-render in SectionList
ItemWithSeparator stored leadingItem, trailingItem, and section info in useState seeded only by the first render's props. When the same cell key re-rendered with different data (e.g. after a section reorder), the state was never updated, so ItemSeparatorComponent kept receiving the original items — including undefined when the previous row was at a section boundary. Re-derive the base separator props from current props at render time and update state when they drift, using the standard React pattern for prop-derived state. The setSelfUpdatePropsCallback API used for cross-cell highlight propagation is preserved. Adds a regression test that reorders a SectionList and asserts the trailing separator below each item reflects the new neighbours. Fixes #55708
1 parent 11d894d commit 532cc60

2 files changed

Lines changed: 100 additions & 8 deletions

File tree

packages/virtualized-lists/Lists/VirtualizedSectionList.js

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -532,24 +532,49 @@ function ItemWithSeparator<ItemT>(
532532

533533
const [separatorHighlighted, setSeparatorHighlighted] = useState(false);
534534

535-
const [leadingSeparatorProps, setLeadingSeparatorProps] = useState<
536-
ItemWithSeparatorCommonProps<ItemT>,
537-
>({
535+
const propsLeadingSeparatorProps: ItemWithSeparatorCommonProps<ItemT> = {
538536
leadingItem: props.leadingItem,
539537
leadingSection: props.leadingSection,
540538
section: props.section,
541539
trailingItem: props.item,
542540
trailingSection: props.trailingSection,
543-
});
544-
const [separatorProps, setSeparatorProps] = useState<
545-
ItemWithSeparatorCommonProps<ItemT>,
546-
>({
541+
};
542+
const propsSeparatorProps: ItemWithSeparatorCommonProps<ItemT> = {
547543
leadingItem: props.item,
548544
leadingSection: props.leadingSection,
549545
section: props.section,
550546
trailingItem: props.trailingItem,
551547
trailingSection: props.trailingSection,
552-
});
548+
};
549+
550+
const [leadingSeparatorProps, setLeadingSeparatorProps] = useState<
551+
ItemWithSeparatorCommonProps<ItemT>,
552+
>(propsLeadingSeparatorProps);
553+
const [separatorProps, setSeparatorProps] = useState<
554+
ItemWithSeparatorCommonProps<ItemT>,
555+
>(propsSeparatorProps);
556+
557+
// Same cell can re-render with new leading/trailing items (e.g. after a
558+
// section reorder). Sync derived state from props during render so
559+
// ItemSeparatorComponent doesn't receive stale leadingItem/trailingItem.
560+
if (
561+
leadingSeparatorProps.leadingItem !== propsLeadingSeparatorProps.leadingItem ||
562+
leadingSeparatorProps.trailingItem !== propsLeadingSeparatorProps.trailingItem ||
563+
leadingSeparatorProps.leadingSection !== propsLeadingSeparatorProps.leadingSection ||
564+
leadingSeparatorProps.section !== propsLeadingSeparatorProps.section ||
565+
leadingSeparatorProps.trailingSection !== propsLeadingSeparatorProps.trailingSection
566+
) {
567+
setLeadingSeparatorProps(propsLeadingSeparatorProps);
568+
}
569+
if (
570+
separatorProps.leadingItem !== propsSeparatorProps.leadingItem ||
571+
separatorProps.trailingItem !== propsSeparatorProps.trailingItem ||
572+
separatorProps.leadingSection !== propsSeparatorProps.leadingSection ||
573+
separatorProps.section !== propsSeparatorProps.section ||
574+
separatorProps.trailingSection !== propsSeparatorProps.trailingSection
575+
) {
576+
setSeparatorProps(propsSeparatorProps);
577+
}
553578

554579
useEffect(() => {
555580
setSelfHighlightCallback(cellKey, setSeparatorHighlighted);

packages/virtualized-lists/Lists/__tests__/VirtualizedSectionList-test.js

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,73 @@ describe('VirtualizedSectionList', () => {
202202
expect(component).toMatchSnapshot();
203203
});
204204

205+
it('passes fresh leadingItem and trailingItem to ItemSeparatorComponent after sections reorder', async () => {
206+
// Regression test for https://github.com/facebook/react-native/issues/55708.
207+
// ItemWithSeparator stored leadingItem/trailingItem in useState seeded from
208+
// the first render, so when the same cell key re-rendered with reordered
209+
// data the separator kept the stale items (or undefined when the previous
210+
// row was at a section boundary).
211+
const separatorPropsByKey: {
212+
[string]: Array<{leading: ?string, trailing: ?string}>,
213+
} = {};
214+
const RecordingSeparator = (props: $FlowFixMe) => {
215+
const itemKey = props.leadingItem?.key ?? '__head__';
216+
separatorPropsByKey[itemKey] = separatorPropsByKey[itemKey] ?? [];
217+
separatorPropsByKey[itemKey].push({
218+
leading: props.leadingItem?.key,
219+
trailing: props.trailingItem?.key,
220+
});
221+
return <separator />;
222+
};
223+
224+
const initial = [
225+
// $FlowFixMe[incompatible-type]
226+
{title: 's', data: [{key: 'a'}, {key: 'b'}, {key: 'c'}]},
227+
];
228+
const reordered = [
229+
// $FlowFixMe[incompatible-type]
230+
{title: 's', data: [{key: 'b'}, {key: 'a'}, {key: 'c'}]},
231+
];
232+
233+
let component;
234+
await ReactTestRenderer.act(() => {
235+
component = ReactTestRenderer.create(
236+
<VirtualizedSectionList
237+
sections={initial}
238+
// $FlowFixMe[missing-local-annot]
239+
renderItem={({item}) => <item value={item.key} />}
240+
getItem={(data, key) => data[key]}
241+
getItemCount={data => data.length}
242+
ItemSeparatorComponent={RecordingSeparator}
243+
/>,
244+
);
245+
});
246+
247+
await ReactTestRenderer.act(() => {
248+
nullthrows(component).update(
249+
<VirtualizedSectionList
250+
sections={reordered}
251+
// $FlowFixMe[missing-local-annot]
252+
renderItem={({item}) => <item value={item.key} />}
253+
getItem={(data, key) => data[key]}
254+
getItemCount={data => data.length}
255+
ItemSeparatorComponent={RecordingSeparator}
256+
/>,
257+
);
258+
});
259+
260+
// Last render of separator below "b" must reflect the reordered list:
261+
// the item below b is now a, so trailing must be 'a'.
262+
const lastBelowB = nullthrows(separatorPropsByKey['b']).at(-1);
263+
expect(lastBelowB?.leading).toBe('b');
264+
expect(lastBelowB?.trailing).toBe('a');
265+
266+
// Last render of separator below "a" must reflect a→c.
267+
const lastBelowA = nullthrows(separatorPropsByKey['a']).at(-1);
268+
expect(lastBelowA?.leading).toBe('a');
269+
expect(lastBelowA?.trailing).toBe('c');
270+
});
271+
205272
describe('scrollToLocation', () => {
206273
const ITEM_HEIGHT = 100;
207274

0 commit comments

Comments
 (0)