Skip to content

Commit

Permalink
Fix invariant violation when nesting VirtualizedList inside ListEmpty…
Browse files Browse the repository at this point in the history
…Component (#35875)

Summary:
Pull Request resolved: #35875

Fixes #35871

Nested VirtualizedLists register to their parents for updates, associated to a specfific cellKey set by VirtualizedListCellContextProvider. This cellKey is usually set when rendering a cell for a data item, but we can also render a nested VirtualizedList by putting one in a ListHeaderComponent/ListFooterComponent/ListEmptyComponent.

D6603342 (a010a0c) added cellKeys when we render from a header/footer, but not ListEmptyComponent, so that association would silently fail earlier.

D39466677 (010da67) added extra invariants to child list handling, that are now triggered by this case, complaining because we are trying to unregister a child list we never successfully registered, due to a missing cellKey.

This fixes the issue by providing a cellKey for ListEmptyComponent as well. It also cleans up some of the parameterization needed from when we had two VirtualizedList implementations.

Changelog:
[General][Fixed] - Fix invariant violation when nesting VirtualizedList inside ListEmptyComponent

Differential Revision: D42574462

fbshipit-source-id: dfec4981aaaa92b2525779b0f6c643688752fd04
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Jan 18, 2023
1 parent 473eb1d commit 5efb813
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 18 deletions.
18 changes: 10 additions & 8 deletions Libraries/Lists/ChildListCollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
* @format
*/

import type VirtualizedList from './VirtualizedList';

import invariant from 'invariant';

export default class ChildListCollection<TList> {
_cellKeyToChildren: Map<string, Set<TList>> = new Map();
_childrenToCellKey: Map<TList, string> = new Map();
export default class ChildListCollection {
_cellKeyToChildren: Map<string, Set<VirtualizedList>> = new Map();
_childrenToCellKey: Map<VirtualizedList, string> = new Map();

add(list: TList, cellKey: string): void {
add(list: VirtualizedList, cellKey: string): void {
invariant(
!this._childrenToCellKey.has(list),
'Trying to add already present child list',
Expand All @@ -27,7 +29,7 @@ export default class ChildListCollection<TList> {
this._childrenToCellKey.set(list, cellKey);
}

remove(list: TList): void {
remove(list: VirtualizedList): void {
const cellKey = this._childrenToCellKey.get(list);
invariant(cellKey != null, 'Trying to remove non-present child list');
this._childrenToCellKey.delete(list);
Expand All @@ -41,22 +43,22 @@ export default class ChildListCollection<TList> {
}
}

forEach(fn: TList => void): void {
forEach(fn: VirtualizedList => void): void {
for (const listSet of this._cellKeyToChildren.values()) {
for (const list of listSet) {
fn(list);
}
}
}

forEachInCell(cellKey: string, fn: TList => void): void {
forEachInCell(cellKey: string, fn: VirtualizedList => void): void {
const listSet = this._cellKeyToChildren.get(cellKey) ?? [];
for (const list of listSet) {
fn(list);
}
}

anyInCell(cellKey: string, fn: TList => boolean): boolean {
anyInCell(cellKey: string, fn: VirtualizedList => boolean): boolean {
const listSet = this._cellKeyToChildren.get(cellKey) ?? [];
for (const list of listSet) {
if (fn(list)) {
Expand Down
23 changes: 13 additions & 10 deletions Libraries/Lists/VirtualizedList.js
Original file line number Diff line number Diff line change
Expand Up @@ -871,16 +871,19 @@ export default class VirtualizedList extends StateSafePureComponent<
<ListEmptyComponent />
)): any);
cells.push(
React.cloneElement(element, {
key: '$empty',
onLayout: (event: LayoutEvent) => {
this._onLayoutEmpty(event);
if (element.props.onLayout) {
element.props.onLayout(event);
}
},
style: StyleSheet.compose(inversionStyle, element.props.style),
}),
<VirtualizedListCellContextProvider
cellKey={this._getCellKey() + '-empty'}
key="$empty">
{React.cloneElement(element, {
onLayout: (event: LayoutEvent) => {
this._onLayoutEmpty(event);
if (element.props.onLayout) {
element.props.onLayout(event);
}
},
style: StyleSheet.compose(inversionStyle, element.props.style),
})}
</VirtualizedListCellContextProvider>,
);
}

Expand Down
26 changes: 26 additions & 0 deletions Libraries/Lists/__tests__/VirtualizedList-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,32 @@ describe('VirtualizedList', () => {
expect(component).toMatchSnapshot();
});

it('handles nested list in ListEmptyComponent', () => {
const ListEmptyComponent = (
<VirtualizedList {...baseItemProps(generateItems(1))} />
);

let component;

ReactTestRenderer.act(() => {
component = ReactTestRenderer.create(
<VirtualizedList
{...baseItemProps([])}
ListEmptyComponent={ListEmptyComponent}
/>,
);
});

ReactTestRenderer.act(() => {
component.update(
<VirtualizedList
{...baseItemProps(generateItems(5))}
ListEmptyComponent={ListEmptyComponent}
/>,
);
});
});

it('returns the viewableItems correctly in the onViewableItemsChanged callback after changing the data', () => {
const ITEM_HEIGHT = 800;
let data = [{key: 'i1'}, {key: 'i2'}, {key: 'i3'}];
Expand Down

0 comments on commit 5efb813

Please sign in to comment.