Log VirtualizedList nesting hierarchy in listKey collision error

Summary:
## Context

When a `VirtualizedList` contains a cell which itself contains more than one `VirtualizedList` of the same orientation as the parent list, we log an error if sibling lists in a cell don't have unique `listKey`s (e.g. when the `listKey` prop isn't explicitly set). In release builds, this error does not have a component stack - nor a useful call stack - so it can be hard to track down the true source of the error in complex applications.

## This diff

Here, in addition to the generic error message, we also print the `listKey`, `cellKey` and orientation of each `VirtualizedList` in the hierarchy, from the child list upwards. This is done without significant overhead, by reusing the already-in-place context that `VirtualizedList`s use to manage nesting.

The assumption is that common strategies for deriving `listKey`s and `cellKey`s will make it possible to identify at least some lists in the hierarchy in common cases, and therefore help narrow down the search space even when component stacks are not available.

## Example

(See code in unit test)

```
A VirtualizedList contains a cell which itself contains more than one VirtualizedList of the same orientation as the parent list. You must pass a unique listKey prop to each sibling list.

VirtualizedList trace:
  Child (horizontal):
    listKey: level2
    cellKey: cell0
  Parent (horizontal):
    listKey: level1
    cellKey: cell0
  Parent (vertical):
    listKey: level0
    cellKey: rootList
```

Changelog: [Internal]

Reviewed By: TheSavior

Differential Revision: D19600366

fbshipit-source-id: 73f29507ec58a6a3f9b3f6b174a32b21dcd237a1
This commit is contained in:
Moti Zilberman 2020-01-28 11:32:02 -08:00 коммит произвёл Facebook Github Bot
Родитель 88aa2b9316
Коммит f7c6066425
2 изменённых файлов: 147 добавлений и 5 удалений

Просмотреть файл

@ -336,6 +336,18 @@ type State = {
...
};
// Data propagated through nested lists (regardless of orientation) that is
// useful for producing diagnostics for usage errors involving nesting (e.g
// missing/duplicate keys).
type ListDebugInfo = {
cellKey: string,
listKey: string,
parent: ?ListDebugInfo,
// We include all ancestors regardless of orientation, so this is not always
// identical to the child's orientation.
horizontal: boolean,
};
/**
* Base implementation for the more convenient [`<FlatList>`](/react-native/docs/flatlist.html)
* and [`<SectionList>`](/react-native/docs/sectionlist.html) components, which are also better
@ -573,6 +585,10 @@ class VirtualizedList extends React.PureComponent<Props, State> {
getNestedChildState: React$PropType$Primitive<Function>,
registerAsNestedChild: React$PropType$Primitive<Function>,
unregisterAsNestedChild: React$PropType$Primitive<Function>,
debugInfo: {|
listKey: React$PropType$Primitive<string>,
cellKey: React$PropType$Primitive<string>,
|},
|},
|} = {
virtualizedCell: PropTypes.shape({
@ -585,6 +601,10 @@ class VirtualizedList extends React.PureComponent<Props, State> {
getNestedChildState: PropTypes.func,
registerAsNestedChild: PropTypes.func,
unregisterAsNestedChild: PropTypes.func,
debugInfo: PropTypes.shape({
listKey: PropTypes.string,
cellKey: PropTypes.string,
}),
}),
};
@ -627,6 +647,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
cellKey: string,
key: string,
ref: VirtualizedList,
parentDebugInfo: ListDebugInfo,
...
}) => ?ChildListState,
unregisterAsNestedChild: ({
@ -634,6 +655,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
state: ChildListState,
...
}) => void,
debugInfo: ListDebugInfo,
...
},
|} {
@ -645,6 +667,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
getNestedChildState: this._getNestedChildState,
registerAsNestedChild: this._registerAsNestedChild,
unregisterAsNestedChild: this._unregisterAsNestedChild,
debugInfo: this._getDebugInfo(),
},
};
}
@ -656,6 +679,21 @@ class VirtualizedList extends React.PureComponent<Props, State> {
);
}
_getListKey(): string {
return this.props.listKey || this._getCellKey();
}
_getDebugInfo(): ListDebugInfo {
return {
listKey: this._getListKey(),
cellKey: this._getCellKey(),
horizontal: !!this.props.horizontal,
parent: this.context.virtualizedList
? this.context.virtualizedList.debugInfo
: null,
};
}
_getScrollMetrics = () => {
return this._scrollMetrics;
};
@ -681,6 +719,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
cellKey: string,
key: string,
ref: VirtualizedList,
parentDebugInfo: ListDebugInfo,
...
}): ?ChildListState => {
// Register the mapping between this child key and the cellKey for its cell
@ -688,13 +727,18 @@ class VirtualizedList extends React.PureComponent<Props, State> {
this._cellKeysToChildListKeys.get(childList.cellKey) || new Set();
childListsInCell.add(childList.key);
this._cellKeysToChildListKeys.set(childList.cellKey, childListsInCell);
const existingChildData = this._nestedChildLists.get(childList.key);
if (existingChildData && existingChildData.ref !== null) {
console.error(
'A VirtualizedList contains a cell which itself contains ' +
'more than one VirtualizedList of the same orientation as the parent ' +
'list. You must pass a unique listKey prop to each sibling list.',
'list. You must pass a unique listKey prop to each sibling list.\n\n' +
describeNestedLists({
...childList,
// We're called from the child's componentDidMount, so it's safe to
// read the child's props here (albeit weird).
horizontal: !!childList.ref.props.horizontal,
}),
);
}
this._nestedChildLists.set(childList.key, {
@ -765,7 +809,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
if (this._isNestedWithSameOrientation()) {
const storedState = this.context.virtualizedList.getNestedChildState(
this.props.listKey || this._getCellKey(),
this._getListKey(),
);
if (storedState) {
initialState = storedState;
@ -781,8 +825,13 @@ class VirtualizedList extends React.PureComponent<Props, State> {
if (this._isNestedWithSameOrientation()) {
this.context.virtualizedList.registerAsNestedChild({
cellKey: this._getCellKey(),
key: this.props.listKey || this._getCellKey(),
key: this._getListKey(),
ref: this,
// NOTE: When the child mounts (here) it's not necessarily safe to read
// the parent's props. This is why we explicitly propagate debugInfo
// "down" via context and "up" again via this method call on the
// parent.
parentDebugInfo: this.context.virtualizedList.debugInfo,
});
}
}
@ -790,7 +839,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
componentWillUnmount() {
if (this._isNestedWithSameOrientation()) {
this.context.virtualizedList.unregisterAsNestedChild({
key: this.props.listKey || this._getCellKey(),
key: this._getListKey(),
state: {
first: this.state.first,
last: this.state.last,
@ -2105,6 +2154,31 @@ class VirtualizedCellWrapper extends React.Component<{
}
}
function describeNestedLists(childList: {
+cellKey: string,
+key: string,
+ref: VirtualizedList,
+parentDebugInfo: ListDebugInfo,
+horizontal: boolean,
...
}) {
let trace =
'VirtualizedList trace:\n' +
` Child (${childList.horizontal ? 'horizontal' : 'vertical'}):\n` +
` listKey: ${childList.key}\n` +
` cellKey: ${childList.cellKey}`;
let debugInfo = childList.parentDebugInfo;
while (debugInfo) {
trace +=
`\n Parent (${debugInfo.horizontal ? 'horizontal' : 'vertical'}):\n` +
` listKey: ${debugInfo.listKey}\n` +
` cellKey: ${debugInfo.cellKey}`;
debugInfo = debugInfo.parent;
}
return trace;
}
const styles = StyleSheet.create({
verticallyInverted: {
transform: [{scaleY: -1}],

Просмотреть файл

@ -388,4 +388,72 @@ describe('VirtualizedList', () => {
expect(onEndReached).toHaveBeenCalled();
});
it('provides a trace when a listKey collision occurs', () => {
const errors = [];
jest.spyOn(console, 'error').mockImplementation((...args) => {
// Silence the DEV-only React error boundary warning.
if ((args[0] || '').startsWith('The above error occured in the ')) {
return;
}
errors.push(args);
});
const commonProps = {
data: [{key: 'cell0'}],
getItem: (data, index) => data[index],
getItemCount: data => data.length,
renderItem: ({item}) => <item value={item.key} />,
};
try {
ReactTestRenderer.create(
<VirtualizedList
{...commonProps}
horizontal={false}
listKey="level0"
renderItem={() => (
<VirtualizedList
{...commonProps}
horizontal={true}
listKey="level1"
renderItem={() => (
<>
{/* Force a collision */}
<VirtualizedList
{...commonProps}
horizontal={true}
listKey="level2"
/>
<VirtualizedList
{...commonProps}
horizontal={true}
listKey="level2"
/>
</>
)}
/>
)}
/>,
);
expect(errors).toMatchInlineSnapshot(`
Array [
Array [
"A VirtualizedList contains a cell which itself contains more than one VirtualizedList of the same orientation as the parent list. You must pass a unique listKey prop to each sibling list.
VirtualizedList trace:
Child (horizontal):
listKey: level2
cellKey: cell0
Parent (horizontal):
listKey: level1
cellKey: cell0
Parent (vertical):
listKey: level0
cellKey: rootList",
],
]
`);
} finally {
console.error.mockRestore();
}
});
});