From a53512fda4d68e099ae2305de6e33eebbd6381d4 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Tue, 2 Aug 2022 01:30:21 -0700 Subject: [PATCH] VirtualizedList up-to-date state: Thread props to _createViewToken() Summary: This diff is part of an overall stack, meant to fix incorrect usage of `setState()` in `VirtualizedList`, which triggers new invariant checks added in `VirtualizedList_EXPERIMENTAL`. See the stack summary below for more information on the broader change. ## Diff Summary `_createViewToken()` is called during state changes. Use explicit props passed in. ## Stack Summary `VirtualizedList`'s component state is a set of cells to render. This state is set via the `setState()` class component API. The main "tick" function `VirtualizedList._updateCellsToRender()` calculates this new state using a combination of the current component state, and instance-local state like maps, measurement caches, etc. From: https://reactjs.org/docs/state-and-lifecycle.html#state-updates-may-be-asynchronous --- > React may batch multiple setState() calls into a single update for performance. Because this.props and this.state may be updated asynchronously, you should not rely on their values for calculating the next state. For example, this code may fail to update the counter: ``` // Wrong this.setState({ counter: this.state.counter + this.props.increment, }); ``` > To fix it, use a second form of setState() that accepts a function rather than an object. That function will receive the previous state as the first argument, and the props at the time the update is applied as the second argument: ``` // Correct this.setState((state, props) => ({ counter: state.counter + props.increment })); ``` --- `_updateCellsToRender()` transitively calls many functions which will read directly from `this.props` or `this.state` instead of the value passed by the state updater. This intermittently fires invariant violations, when there is a mismatch. This diff migrates all usages of `props` and `state` during state update to the values provied in `setState()`. To prevent future mismatch, and to provide better clarity on when it is safe to use `this.props`, `this.state`, I overrode `setState` to fire an invariant violation if it is accessed when it is unsafe to: {F756963772} Changelog: [Internal][Fixed] - Thread props to _createViewToken() Reviewed By: genkikondo Differential Revision: D38294339 fbshipit-source-id: dc215e267f126a3789742c14c35479f509822710 --- Libraries/Lists/ViewabilityHelper.js | 17 ++++++++++++++--- Libraries/Lists/VirtualizedList_EXPERIMENTAL.js | 10 +++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/Libraries/Lists/ViewabilityHelper.js b/Libraries/Lists/ViewabilityHelper.js index c70aa56362006b..9c402c44107bc5 100644 --- a/Libraries/Lists/ViewabilityHelper.js +++ b/Libraries/Lists/ViewabilityHelper.js @@ -189,7 +189,11 @@ class ViewabilityHelper { offset: number, ... }, - createViewToken: (index: number, isViewable: boolean) => ViewToken, + createViewToken: ( + index: number, + isViewable: boolean, + props: FrameMetricProps, + ) => ViewToken, onViewableItemsChanged: ({ viewableItems: Array, changed: Array, @@ -236,6 +240,7 @@ class ViewabilityHelper { * see the error delete this comment and run Flow. */ this._timers.delete(handle); this._onUpdateSync( + props, viewableIndices, onViewableItemsChanged, createViewToken, @@ -247,6 +252,7 @@ class ViewabilityHelper { this._timers.add(handle); } else { this._onUpdateSync( + props, viewableIndices, onViewableItemsChanged, createViewToken, @@ -269,13 +275,18 @@ class ViewabilityHelper { } _onUpdateSync( + props: FrameMetricProps, viewableIndicesToCheck: Array, onViewableItemsChanged: ({ changed: Array, viewableItems: Array, ... }) => void, - createViewToken: (index: number, isViewable: boolean) => ViewToken, + createViewToken: ( + index: number, + isViewable: boolean, + props: FrameMetricProps, + ) => ViewToken, ) { // Filter out indices that have gone out of view since this call was scheduled. viewableIndicesToCheck = viewableIndicesToCheck.filter(ii => @@ -284,7 +295,7 @@ class ViewabilityHelper { const prevItems = this._viewableItems; const nextItems = new Map( viewableIndicesToCheck.map(ii => { - const viewable = createViewToken(ii, true); + const viewable = createViewToken(ii, true, props); return [viewable.key, viewable]; }), ); diff --git a/Libraries/Lists/VirtualizedList_EXPERIMENTAL.js b/Libraries/Lists/VirtualizedList_EXPERIMENTAL.js index e2d355e48cfa21..3e2682f15a2a79 100644 --- a/Libraries/Lists/VirtualizedList_EXPERIMENTAL.js +++ b/Libraries/Lists/VirtualizedList_EXPERIMENTAL.js @@ -1754,13 +1754,17 @@ class VirtualizedList extends React.PureComponent { }); }; - _createViewToken = (index: number, isViewable: boolean) => { - const {data, getItem} = this.props; + _createViewToken = ( + index: number, + isViewable: boolean, + props: FrameMetricProps, + ) => { + const {data, getItem} = props; const item = getItem(data, index); return { index, item, - key: this._keyExtractor(item, index, this.props), + key: this._keyExtractor(item, index, props), isViewable, }; };