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 (facebook#35875)

Summary:
Pull Request resolved: facebook#35875

Fixes facebook#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 (facebook@a010a0c) added cellKeys when we render from a header/footer, but not ListEmptyComponent, so that association would silently fail earlier.

D39466677 (facebook@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.