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.

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

Reviewed By: christophpurrer

Differential Revision: D42574462

fbshipit-source-id: f76fa795bf471cb8a929c2efdbd814ea51927663
NickGerleman authored and facebook-github-bot committed Jan 19, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 4cdc2c4 commit 1fef376
Showing 2 changed files with 39 additions and 10 deletions.
23 changes: 13 additions & 10 deletions Libraries/Lists/VirtualizedList.js
Original file line number Diff line number Diff line change
@@ -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 comment has been minimized.

Copy link
@priyeshshah11

priyeshshah11 Feb 28, 2023

@NickGerleman This is now throwing an error "this.props" should not be accessed during state updates when using state, setState & props inside the onLayout callback which used to work on previous versions? Is this expected?

Reference

This comment has been minimized.

Copy link
@NickGerleman

NickGerleman Feb 28, 2023

Author Contributor

Please create an issue on the repo which includes more information, like a callstack.

this._onLayoutEmpty(event);
if (element.props.onLayout) {
element.props.onLayout(event);
}
},
style: StyleSheet.compose(inversionStyle, element.props.style),
})}
</VirtualizedListCellContextProvider>,
);
}

26 changes: 26 additions & 0 deletions Libraries/Lists/__tests__/VirtualizedList-test.js
Original file line number Diff line number Diff line change
@@ -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'}];

0 comments on commit 1fef376

Please sign in to comment.