Skip to content

Commit

Permalink
Remove dependence on VirtualizedList listKey prop
Browse files Browse the repository at this point in the history
Summary:
Following up on the previous diff to remove the only usage of `listKey` for persistent association, we can remove the need for a manual `listKey`, and instead rely on per-instance association via refs.

A followup change will remove the existing usages.

Changelog:
[General][Removed] - Remove VirtualizedList `listKey` prop

Reviewed By: p-sun

Differential Revision: D39466677

fbshipit-source-id: 6b49f45c987fff9836918ba833fbb16f24414ff8
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Sep 20, 2022
1 parent c5217f1 commit 010da67
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 324 deletions.
72 changes: 72 additions & 0 deletions Libraries/Lists/ChildListCollection.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow strict
* @format
*/

import invariant from 'invariant';

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

add(list: TList, cellKey: string): void {
invariant(
!this._childrenToCellKey.has(list),
'Trying to add already present child list',
);

const cellLists = this._cellKeyToChildren.get(cellKey) ?? new Set();
cellLists.add(list);
this._cellKeyToChildren.set(cellKey, cellLists);

this._childrenToCellKey.set(list, cellKey);
}

remove(list: TList): void {
const cellKey = this._childrenToCellKey.get(list);
invariant(cellKey != null, 'Trying to remove non-present child list');
this._childrenToCellKey.delete(list);

const cellLists = this._cellKeyToChildren.get(cellKey);
invariant(cellLists, '_cellKeyToChildren should contain cellKey');
cellLists.delete(list);

if (cellLists.size === 0) {
this._cellKeyToChildren.delete(cellKey);
}
}

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

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

anyInCell(cellKey: string, fn: TList => boolean): boolean {
const listSet = this._cellKeyToChildren.get(cellKey) ?? [];
for (const list of listSet) {
if (fn(list)) {
return true;
}
}
return false;
}

size(): number {
return this._childrenToCellKey.size;
}
}
143 changes: 27 additions & 116 deletions Libraries/Lists/VirtualizedList.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import type {
export type {RenderItemProps, RenderItemType, Separators};

import {
type ListDebugInfo,
VirtualizedListCellContextProvider,
VirtualizedListContext,
VirtualizedListContextProvider,
Expand All @@ -35,6 +34,7 @@ import {
} from './VirtualizeUtils';
import * as VirtualizedListInjection from './VirtualizedListInjection';
import * as React from 'react';
import ChildListCollection from './ChildListCollection';

const RefreshControl = require('../Components/RefreshControl/RefreshControl');
const ScrollView = require('../Components/ScrollView/ScrollView');
Expand Down Expand Up @@ -294,7 +294,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {

recordInteraction() {
this._nestedChildLists.forEach(childList => {
childList && childList.recordInteraction();
childList.recordInteraction();
});
this._viewabilityTuples.forEach(t => {
t.viewabilityHelper.recordInteraction();
Expand Down Expand Up @@ -349,19 +349,6 @@ class VirtualizedList extends React.PureComponent<Props, State> {
return this.context?.cellKey || 'rootList';
}

_getListKey(): string {
return this.props.listKey || this._getCellKey();
}

_getDebugInfo(): ListDebugInfo {
return {
listKey: this._getListKey(),
cellKey: this._getCellKey(),
horizontal: horizontalOrDefault(this.props.horizontal),
parent: this.context?.debugInfo,
};
}

// $FlowFixMe[missing-local-annot]
_getScrollMetrics = () => {
return this._scrollMetrics;
Expand All @@ -382,41 +369,20 @@ class VirtualizedList extends React.PureComponent<Props, State> {

_registerAsNestedChild = (childList: {
cellKey: string,
key: string,
ref: React.ElementRef<typeof React.Component>,
parentDebugInfo: ListDebugInfo,
...
}): void => {
const specificRef = ((childList.ref: any): VirtualizedList);
// Register the mapping between this child key and the cellKey for its cell
const childListsInCell =
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) {
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.\n\n' +
describeNestedLists({
...childList,
ref: specificRef,
// We're called from the child's componentDidMount, so it's safe to
// read the child's props here (albeit weird).
horizontal: !!specificRef.props.horizontal,
}),
);
}
this._nestedChildLists.set(childList.key, specificRef);

const listRef = ((childList.ref: any): VirtualizedList);
this._nestedChildLists.add(listRef, childList.cellKey);
if (this._hasInteracted) {
specificRef.recordInteraction();
listRef.recordInteraction();
}
};

_unregisterAsNestedChild = (childList: {key: string, ...}): void => {
this._nestedChildLists.delete(childList.key);
_unregisterAsNestedChild = (childList: {
ref: React.ElementRef<typeof React.Component>,
}): void => {
const listRef = ((childList.ref: any): VirtualizedList);
this._nestedChildLists.remove(listRef);
};

state: State;
Expand Down Expand Up @@ -476,28 +442,15 @@ class VirtualizedList extends React.PureComponent<Props, State> {
componentDidMount() {
if (this._isNestedWithSameOrientation()) {
this.context.registerAsNestedChild({
cellKey: 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.debugInfo,
cellKey: this.context.cellKey,
});
}
}

componentWillUnmount() {
if (this._isNestedWithSameOrientation()) {
this.context.unregisterAsNestedChild({
key: this._getListKey(),
state: {
first: this.state.first,
last: this.state.last,
frames: this._frames,
},
});
this.context.unregisterAsNestedChild({ref: this});
}
this._updateViewableItems(null);
this._updateCellsToRenderBatcher.dispose({abort: true});
Expand Down Expand Up @@ -851,7 +804,6 @@ class VirtualizedList extends React.PureComponent<Props, State> {
getOutermostParentListRef: this._getOutermostParentListRef,
registerAsNestedChild: this._registerAsNestedChild,
unregisterAsNestedChild: this._unregisterAsNestedChild,
debugInfo: this._getDebugInfo(),
}}>
{React.cloneElement(
(
Expand Down Expand Up @@ -927,8 +879,6 @@ class VirtualizedList extends React.PureComponent<Props, State> {
}

_averageCellLength = 0;
// Maps a cell key to the set of keys for all outermost child lists within that cell
_cellKeysToChildListKeys: Map<string, Set<string>> = new Map();
_cellRefs: {[string]: null | CellRenderer} = {};
_fillRateHelper: FillRateHelper;
_frames: {
Expand All @@ -949,7 +899,8 @@ class VirtualizedList extends React.PureComponent<Props, State> {
_hiPriInProgress: boolean = false; // flag to prevent infinite hiPri cell limit update
_highestMeasuredFrameIndex = 0;
_indicesToKeys: Map<number, string> = new Map();
_nestedChildLists: Map<string, VirtualizedList> = new Map();
_nestedChildLists: ChildListCollection<VirtualizedList> =
new ChildListCollection();
_offsetFromParentVirtualizedList: number = 0;
_prevParentOffset: number = 0;
// $FlowFixMe[missing-local-annot]
Expand Down Expand Up @@ -1067,13 +1018,9 @@ class VirtualizedList extends React.PureComponent<Props, State> {
};

_triggerRemeasureForChildListsInCell(cellKey: string): void {
const childListKeys = this._cellKeysToChildListKeys.get(cellKey);
if (childListKeys) {
for (let childKey of childListKeys) {
const childList = this._nestedChildLists.get(childKey);
childList && childList.measureLayoutRelativeToContainingList();
}
}
this._nestedChildLists.forEachInCell(cellKey, childList => {
childList.measureLayoutRelativeToContainingList();
});
}

measureLayoutRelativeToContainingList(): void {
Expand Down Expand Up @@ -1107,14 +1054,8 @@ class VirtualizedList extends React.PureComponent<Props, State> {

// If metrics of the scrollView changed, then we triggered remeasure for child list
// to ensure VirtualizedList has the right information.
this._cellKeysToChildListKeys.forEach(childListKeys => {
if (childListKeys) {
for (let childKey of childListKeys) {
const childList = this._nestedChildLists.get(childKey);
childList &&
childList.measureLayoutRelativeToContainingList();
}
}
this._nestedChildLists.forEach(childList => {
childList.measureLayoutRelativeToContainingList();
});
}
},
Expand Down Expand Up @@ -1547,7 +1488,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
last: Math.min(state.last + renderAhead, getItemCount(data) - 1),
};
}
if (newState && this._nestedChildLists.size > 0) {
if (newState && this._nestedChildLists.size() > 0) {
const newFirst = newState.first;
const newLast = newState.last;
// If some cell in the new state has a child list in it, we should only render
Expand All @@ -1556,21 +1497,16 @@ class VirtualizedList extends React.PureComponent<Props, State> {
// their items.
for (let ii = newFirst; ii <= newLast; ii++) {
const cellKeyForIndex = this._indicesToKeys.get(ii);
const childListKeys =
cellKeyForIndex &&
this._cellKeysToChildListKeys.get(cellKeyForIndex);
if (!childListKeys) {
if (cellKeyForIndex == null) {
continue;
}
let someChildHasMore = false;
// For each cell, need to check whether any child list in it has more elements to render
for (let childKey of childListKeys) {
const childList = this._nestedChildLists.get(childKey);
if (childList && childList.hasMore()) {
someChildHasMore = true;
break;
}
}

const someChildHasMore = this._nestedChildLists.anyInCell(
cellKeyForIndex,
childList => childList.hasMore(),
);

if (someChildHasMore) {
newState.last = ii;
break;
Expand Down Expand Up @@ -1879,31 +1815,6 @@ class CellRenderer 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: ?ListDebugInfo = 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}],
Expand Down
29 changes: 2 additions & 27 deletions Libraries/Lists/VirtualizedListContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,13 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow strict-local
* @flow strict
* @format
*/

import * as React from 'react';
import {useMemo, useContext} from 'react';

// Data propagated through nested lists (regardless of orientation) that is
// useful for producing diagnostics for usage errors involving nesting (e.g
// missing/duplicate keys).
export type ListDebugInfo = $ReadOnly<{
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,
}>;

type Context = $ReadOnly<{
cellKey: ?string,
getScrollMetrics: () => {
Expand All @@ -39,14 +27,11 @@ type Context = $ReadOnly<{
getOutermostParentListRef: () => React.ElementRef<typeof React.Component>,
registerAsNestedChild: ({
cellKey: string,
key: string,
ref: React.ElementRef<typeof React.Component>,
parentDebugInfo: ListDebugInfo,
}) => void,
unregisterAsNestedChild: ({
key: string,
ref: React.ElementRef<typeof React.Component>,
}) => void,
debugInfo: ListDebugInfo,
}>;

export const VirtualizedListContext: React.Context<?Context> =
Expand Down Expand Up @@ -89,23 +74,13 @@ export function VirtualizedListContextProvider({
getOutermostParentListRef: value.getOutermostParentListRef,
registerAsNestedChild: value.registerAsNestedChild,
unregisterAsNestedChild: value.unregisterAsNestedChild,
debugInfo: {
cellKey: value.debugInfo.cellKey,
horizontal: value.debugInfo.horizontal,
listKey: value.debugInfo.listKey,
parent: value.debugInfo.parent,
},
}),
[
value.getScrollMetrics,
value.horizontal,
value.getOutermostParentListRef,
value.registerAsNestedChild,
value.unregisterAsNestedChild,
value.debugInfo.cellKey,
value.debugInfo.horizontal,
value.debugInfo.listKey,
value.debugInfo.parent,
],
);
return (
Expand Down
Loading

0 comments on commit 010da67

Please sign in to comment.