Summary:
`VirtualizedList_EXPERIMENTAL` is a fork of VirtualizedList. It was originally created for the purpose of keyboard/a11y fixes, which required the list to allow rendering discontiguous regions of cells. So, the large part of the refactor is in the data structure used to represent state (now a sparse bitmask), and building out the render function to operate off the bitmask.
This structure allows pre-rendering areas where we know focus must be able to be synchronously be moved to. This is exercised on platforms like Windows and macOS which fire view level focus events. The new implementation otherwise aims to preserve near-exact list behavior as previously.
Apart from the structure change, the refactor has made the code (subjectively) a lot easier to reason about, and there are now stronger internal invariants than before. The bitmask structure also enables new approaches for nested list stability, and implementing OffScreen.
**Why ship it now?**
Having two implementations has multiple times prevented other changes to VirtualizedList, so it is incurring a constant engineer cost.
At this point, we have a lot of reason to be confident in the maturity of the new list implementation (see the test plan below). So, reconciling the implementations not only unlocks the enhancements of the new list, but unblocks future changes to VirtualizedList.
## Test Plan
1. **Unit tests:** A few dozen unit tests were added to VirtualizedList, validating the regions it renders in response to injected input, across its API surface. Both VirtualizedList implementations must pass the same tests, around expected behavior.
2. **RNTester:** Scenarios in RNTester were manually validated, and all worked as before
3. **New invariants:** I added a lot of internal logic checks and bounds checks as invariant violations, to try to produce signal if any part of the refactor went unexpected in the wild.
4. **MSFT Rollout:** I rolled out the changes as a fork of VirtualizedList at MSFT to a couple of high-usage surfaces using Android, iOS, and Windows on Paper. Some invariant violations were surfaced and fixed, but telemetry showed solid system and scenario health with the change.
5. **Meta import:** Fixed all land-blocking test failures when using the new list. These were confined to updating snapshots, and adding additional checks for invalid input.
6. **Panel apps:** Manually validated top-level surfaces of panel apps in debug/release modes. Fixed a couple of invariant violations and novel usages. Profiled new JS structures against an expensive list with Hermes profiler.
7. **Facebook App Rollout:** After some manual validation of top-level surfaces, the change was rolled out to Facebook for Android and iOS as an experiment. New invariant violations were fixed, and the change has sat in production for several weeks at this point, while measuring impact to metrics like error rate, responsiveness, and impressions.
This is the first introduction to all of OSS, and some Meta internal apps. This means there may still be novel usages of VirtualizedList that were not picked up during testing, but I think there has been enough diligence to roll out what has been tested, and forward fix anything unexpected that might still come up.
Changelog:
[General][Changed] - Ship VirtualizedList_EXPERIMENTAL
Reviewed By: javache
Differential Revision: D40259791
fbshipit-source-id: 63eee9381d197a1e38ae663b2158436ff135c0e1
Summary:
Found and removed duplicates of the word "the" in comments.
## Changelog
[Internal] [Removed] – Removed duplicates of the word "the" in comments.
Pull Request resolved: https://github.com/facebook/react-native/pull/34807
Test Plan: Not applicable.
Reviewed By: yungsters, cipolleschi
Differential Revision: D39880587
fbshipit-source-id: b7277aa70604902929903c31ab69d4c532f2667a
Summary:
React Native's TS definitions are currently mostly stored in one monolithic file. This change splits the definitions up to correspond to the source files they came from, and are placed next to the source files. I think this should help inform, and make it easy to update the TS declarations when touching the Flow file.
I noticed as part of the change that the typings have not yet removed many APIs that were removed from RN. This is bad, since it means using the removed/non-functional API doesn't cause typechecker errors. Locating typings next to source should prevent that from being able to happen.
The organization here means individual TS declarations can declare what will be in the RN entrypoint, which is a little confusing. Seems like a good potential next refactor, beyond the literal translation I did.
Changelog:
[General][Changed] - Place TS Declarations Alongside Source Files
Reviewed By: lunaleaps, rshest
Differential Revision: D39796598
fbshipit-source-id: b36366466fd1976bdd2d4c8f7a4104a33c457a07
Summary:
`verifyVirtualizedList` chokes on missing displayName in DEV, due to a mistake in D39648806 (https://github.com/facebook/react-native/commit/a0d1585cdb5ae86e5d4e5bc51fdfa9ae7e049b4b). Not sure why this isn't triggered in Jest tests (maybe it is able to fill displayName via transform?), but this fixes the issue in dev mode.
Changelog:
[Internal][Fixed] - Fix crash in DEV when VirtualizedList_EXPERIMENTAL is loaded
Reviewed By: mdvacca
Differential Revision: D39670991
fbshipit-source-id: b1a784aa3fffdc5f2f025eb1f689307132aad9b6
Summary:
VirtualizedList uses a combination of ES6 and CommonJS imports/exports. This moves fully to ES6 imports and exports, in the experimental version (which should soon replace the original).
Changelog:
[Internal][Changed] - Move VirtualizedList_EXPERIMENTAL to ES6 import/export
Reviewed By: rshest
Differential Revision: D39648806
fbshipit-source-id: a5da40f62a6010cd5b9894b02a91af86d3a8b995
Summary:
VirtualizedList is large and complicated, getting larger and more complicated. This splits out a subcomponent, the cell renderer into its own file, since it is relatively isolated already. This uses the copy from VirtualizedList_EXPERIMENTAL, whose only real difference is exposing focus capture events to the containing VirtualizedList.
Changelog:
[Internal][Changed] - Extract VirtualizedListCellRenderer
Reviewed By: rshest
Differential Revision: D39648087
fbshipit-source-id: bb7c2eff0c658713c256650596f86e8788019baf
Summary:
Now that the prop noops, remove the usages of the property. This is the point of no return for the prop.
Replaced listKey usage with a combination of `flow check`, regex replace, manual inspection.
Changelog:
[General][Removed] - Remove usages of listKey
Reviewed By: fred2028
Differential Revision: D39589089
fbshipit-source-id: 722ebb7dce038a2709656394b7d736e72f488bb7
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
Summary:
When `getItemLayout` is not supplied, VirtualizedList doesn't know the measurements for its child items ahead of time. It caches most recent cell measurements so that an item which is virtualized away is associated with its correct height. VirtualizedList will not consume space for an item it has not yet rendered/knows measurements for.
This also means an initially created list will only take up the height of its `initialNumToRender()`. This can cause scroll jumping for the scenario of nested VirtualizedLists of the same orientation, since a virtualized away child list will shrink (compared to the spacer), then re-expand, once returned to.
There is logic in the VirtualizedList constructor to load cached frame metrics based on supplied listKey, to enable reloading most recent state. Though locally it seems to be reading the context too early in the lifecycle (compared to other successful checks during componentDidMount, render). It was moved from `componentDidMount` to `constructor` as part of the initial change (https://www.internalfb.com/diff/D6330846 (https://github.com/facebook/react-native/commit/2668dc8e1be7bf93e65ca2e11c87cbeef3310c3e)?dst_version_fbid=131968984136932&transaction_fbid=1578362385612542), and I think has likely been unreliable or dead since.
Restoring state was the only usage of a durable key for nested lists beyond their lifetime. So removing the dead code lets us remove the need for a `listKey`, which has been error-prone. This still leaves the problem present since inception, but there may be alternate solutions like scroll anchoring.
Changelog:
[Internal][Removed] - Remove dead code for restoring state to nested VirtualizedList
Reviewed By: ryancat
Differential Revision: D39466678
fbshipit-source-id: fc2f39802e5cdf9b920974333be64c74211b99a9
Summary:
This adds a a couple of tests to run existing behavior where VirtualizedList avoids batched renders until it has seen scroll metrics when the list is not initialized to the zero position.
Changelog:
[Internal][Added] - Add VirtualizedList tests for handling non-zero initialScrollIndex before scroll metrics
Reviewed By: yungsters
Differential Revision: D39247250
fbshipit-source-id: 530f58f9c7421949c388048c376fc5b0ab7835d9
Summary:
This replaces all direct references to `ReactNative` within the `react-native` package to use `findNodeHandle` with a reference obtained from `RendererProxy`, which will allow us to select the correct renderer.
Changelog: [internal]
Reviewed By: javache
Differential Revision: D39270689
fbshipit-source-id: a39875281ba7b7b1b00128564124b6adcacebc4d
Summary:
FillRateHelper accesses frame metrics based on passed state/cellsAroundViewport on scroll. cellsAroundViewport may be [0, -1] for no items. Usually this is captured by the check that item count is zero, but it is possible to have items without yet expanding state to render the items. This adds a check to bail early if we are not yet rendering any viewport-related cells.
Also renamed "state" to "cellsAroundViewport" for more consistent naming with the new list, where it is no longer the only state.
Reviewed By: javache
Differential Revision: D39345705
fbshipit-source-id: 198ab46ff2c8e8fe64076c9150edd4914dd745d7
Summary: VirtualizedList would more gracefully handle out of range cells than VirtualizedList_EXPERIMENTAL, which treats it as an invariant violation. D39244112 (https://github.com/facebook/react-native/commit/7aa203beda3cd358703c2fa535ed045771761612) attempted to fix an issue where recalculation of cells around viewport can include out of range cells, but it is still showing up later. This change adds a bounds check to the remaining branch we control, and an assertion that `computeWindowedRenderLimits` is not returing something out of range to discover if that is the cause.
Reviewed By: yungsters
Differential Revision: D39267445
fbshipit-source-id: 64c99da28b5b01ef61784079b586e355f73764a1
Summary:
Non-integer `initialScrollIndex` or values to `scrollToIndex` would produce a reasonable result, with the caveat that it always falls back to layout estimation (will only be correct when all items are the same size), and breaks if getItemLayout() is supplied. It has usage though, so this diff adds proper support for non-integer scrollIndex, to offset a given amount into the length of the specific cell.
This overlaps a bit with the optional `viewOffset` and `viewPosition` arguments in `scrollToIndex`, but there isn't really the equivalent API for `initialScrollIndex`.
Changelog:
[General][Added]- Add proper support for fractional scrollIndex in VirtualizedList
Reviewed By: yungsters
Differential Revision: D39271100
fbshipit-source-id: 4d93887eed4497e9f6abcd1a6117ac7fdaebf2b1
Summary: VirtualizedList would more gracefully handle out of range cells than VirtualizedList_EXPERIMENTAL, which treats it as an invariant violation. This attempts to fix an issue where recalculation of cells around viewport can include out of range cells.
Reviewed By: rshest
Differential Revision: D39244112
fbshipit-source-id: 20fe6ea552035061d9d00720fcab77b29a785771
Summary:
Cleans up a few minor Flow suppression comments in `FlatList`. This reveals a new Flow error that is the result of `FlatList`'s props object being inexact whereas `VirtualizedList`'s props object is exact. For now, I introduce another -- but more specific -- Flow suppression for that type error.
The code changes should not have any consequential behavior change.
Changelog:
[Internal]
Reviewed By: NickGerleman
Differential Revision: D38646228
fbshipit-source-id: f4f9b0ad95323157ff1519353b38e8486adc841d
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
This adds a `StateSafePureComponent`, which will override `setState`, along with `this.props` and `this.state`, in order to add runtime enforcement that `this.props` and `this.state` are not accessed during a state update where we should be relying on parameter instead. This should be landed after the fixes for unsafe `this.props`/`this.state` usage.
## 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][Changed] - Enable setState() hooks
Reviewed By: rshest
Differential Revision: D38294338
fbshipit-source-id: d04ff39f68df90adc9f4680887d308d997903675
Summary:
Fixes https://github.com/facebook/react-native/issues/34034.
The FlatList doesn't call renderItem on nullish values when numColumns > 1, but it does when numColumns is not set (or equals 1).
I think the behavior should be consistent, so I updated the code so renderItems is called for every items.
I believe the condition `item != null` was here to make sure renderItem isn't called for index outside of data range, so I replaced it with `itemIndex < data.length`.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[General] [Fixed] - Fix FlatList not calling render items for nullish values when numColumns > 1
Pull Request resolved: https://github.com/facebook/react-native/pull/34205
Test Plan:
- I added a failing test corresponding to the issue, and the test now succeeds.
- I used the same code as in the test on a newly initialized app on RN 0.69 and made sure renderItem was called for every items as expected.
Reviewed By: NickGerleman
Differential Revision: D38185103
Pulled By: lunaleaps
fbshipit-source-id: 4baa55caef9574c91c43c047f9e419016ceb39db
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
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
`_onCellFocusCapture()` derives state from existing state, so it should be wrapped in a state updater to compare against the latest value in the batch.
## 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] - Use correct form of setState() in _onCellFocusCapture()
Reviewed By: genkikondo
Differential Revision: D38293583
fbshipit-source-id: 1d0e5152e6c1757408e39dff225e07accc5ee49f
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
Use supplied props in `_keyExtractor()`, which is used to map between frame index and identity.
## 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 _keyExtractor()
Reviewed By: genkikondo
Differential Revision: D38293586
fbshipit-source-id: e50823bacdf45adad3b8b655d66d305a1762e9b8
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
This forwards props to `__getFrameMetricsApprox()` and `_getFrameMetrics()` . This is called in a variery of places, so we need to pass `FrameMetricProps` through more places, and update public/test usage.
## 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 __getFrameMetrics()
Reviewed By: genkikondo
Differential Revision: D38293591
fbshipit-source-id: c1499d722b69eb4b5953124ee8b8c3d15e912d93
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
Use passed props in `computeWindowedRenderLimits()`, which is called during a state update.
## 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 computeWindowedRenderLimits()
Reviewed By: genkikondo
Differential Revision: D38293588
fbshipit-source-id: 1330a003c1354bbd63c0b7b33a013e85e96fdc64
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
Change `_updateViewableItems()` to accept an explicit set of props, and a CellRenderMask, instead of using `this.props` and `this.state`. The eventual sink are the `_getFrameMetric*` functions, so a minimal projection of props is added that we can pass through to it.
## 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][Changed] - Move Props to VirtualizedListProps
Reviewed By: genkikondo
Differential Revision: D38293587
fbshipit-source-id: 164fd4af7c370d905af53b0e9aafb3592d8659cc
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
Remove `this.props` usage during state update where we can just use the props directly.
## 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] - Remove _isVirtualizationDisabled()
Reviewed By: genkikondo
Differential Revision: D38293589
fbshipit-source-id: af18f867fc880d5363134fe0d6f985efaf8be6c5
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
This moves public VirtualizedList types to a new file, shared between both `VirtualizedList`, and `VirtualizedList_EXPERIMENTAL`. This allows us to make public interface changes in a single place.
## 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][Changed] - Move Props to VirtualizedListProps
Reviewed By: genkikondo
Differential Revision: D38293585
fbshipit-source-id: 344d94466a2741ee67eb5754de5506eb3e265c5b
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
Replace usages of `this.state` and `this.props` where we are already passed the newer revision of the state we want.
## 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] - Remove bad usage in _adjustCellsAroundViewport()
Reviewed By: genkikondo
Differential Revision: D38293584
fbshipit-source-id: 807f36600d2fd82c87205195dd61956785bdbd2c
Summary:
VirtualizedList state is represented in terms of [first, last] ranges, where it is possible to express a zero-cell range by having [n, n-1]. This includes some awkward examples, like [0, -1] being valid.
CellRenderMask assumes `addCells` is called with at least one cell, with VirtualizedList previously guarding against adding the no cell case. This guard is present for adding main cell regions, but not for `_initialRenderRegion()`, which can be overridden to have a zero length as well.
This moves the `CellRenderMask` to be permissive of zero-length cell regions, and removes the caller guard.
Changelog:
[Internal][Fixed] - Allow empty cell ranges in CellRenderMask
Reviewed By: rshest
Differential Revision: D38184444
fbshipit-source-id: d2dadfdd9628b24f894126d63b1eb93f6387b877
Summary:
Existing code may pass negative values for `initialScrollIndex`, which the previous implemnentation handled gracefully. Handle the case gracefully in VirtualizedList_EXPERIMENTAL as well.
Changelog:
[Internal][Fixed] - Gracefully handle negative initialScrollIndex in VirtualizedList_EXPERIMENTAL
Reviewed By: rshest
Differential Revision: D38183625
fbshipit-source-id: 9aea91c4cf3ce2190d769f34ce728a109efc88d4
Summary:
If FlatList is passed non-array data it will return `undefined` as the result of `getItemCount()` to VirtualizedList. This change makes it return `0` instead, to signify there are no valid items to attempt to accces.
There are a set of invariants on properties passed to FlatList, to curb incorrect types at runtime, but there is existing code which runs into the condition.
Changelog:
[Internal][Fixed] - Guard FlatList getItemCount() against non-array data
Reviewed By: javache
Differential Revision: D38198351
fbshipit-source-id: 9efd0df7eeeba17078e2c838d470c4b0d621b9a0
Summary:
VirtualizedList_EXPERIMENTAL has a different flow signature from VirtualizedList. This does not cause an error in unit tests, which are untyped, but prevents injecting VirtualizedList_EXPERIMENTAL via the current API from flowtype. This diff updates the injection interface to allow a more relaxed type, validating safety at runtime.
Changelog:
[Internal][Changed] - Allow injecting looser VirtualizedList
Reviewed By: javache
Differential Revision: D38143682
fbshipit-source-id: 054bbc5092823f4e4413d69d3d72a7ecdd71a6a2
Summary:
# This Change
https://github.com/react-native-community/discussions-and-proposals/pull/335 discussed a set of problems with VirtualizedList and focus. These were seen as severe externally for a11y on desktop. The issues center on users of keyboard and accessibility tools, where users expect to be able to move focus in an uninterrupted loop.
The design and implementation evolved to be a bit more general, and without any API-surface behavior changes. It was implemented and rolled out externally as a series of changes. The remaining changes that were not upstreamed into RN are rolled into https://github.com/facebook/react-native/pull/32646
This diff brings this change into the repo, as a separate copy of VirtualizedList, to measure its impact to guardrail metrics, without yet making it the default implementation. The intention is for this to be temporary, until there is confidence the implementation is correct.
## List Implementation (more on GitHub)
This change makes it possible to synchronously move native focus to arbitrary items in a VirtualizedList. This is implemented by switching component state to a sparse bitset. This was previously implemented and upstreamed as `CellRenderMask`.
A usage of this is added, to keep the last focused item rendered. This allows the focus loop to remain unbroken, when scrolling away, or tab loops which leave/re-enter the list.
VirtualizedList tracks the last focused cell through the capture phase of `onFocus`. It will keep the cell, and a viewport above and below the last focused cell rendered, to allow movement to it without blanking (without using too much memory).
## Experimentation Implementation
A mechanism is added to gate the change via VirtualizedListInjection, mirroring the approach taken for Switch with D27381306 (https://github.com/facebook/react-native/commit/683b825b327e7fa71e87e3d613cb9acd37c24288).
It allows VirtualizedList to delegate to a global override. It has a slight penalty to needing to import both modules, but means code which imports VirtualizedList directly is affected the changes.
Changelog:
[Internal][Added] - Add VirtualizedList_EXPERIMENTAL (CellRenderMask Usage)
Reviewed By: lunaleaps
Differential Revision: D38020408
fbshipit-source-id: ad0aaa6791f3f4455e3068502a2841f3ffb40b41
Summary:
This change is in preparation of adding a separate `VirtualizedList_EXPERIMENTAL` component.
Both the original, and experimental lists use `VirtualizedListContext`, which itself references back to the VirtualizedList class type. VirtualizedList private methods are currently included in the type system, and are called in other VirtualizedList code (see https://github.com/facebook/react-native/commit/b2f871a6fa9c92dd0712055384b9eca6d828e37d). This prevents Flow from seeing the two classes are compatible if "private" methods change.
My first attempt was to parameterize the context, to allow both `VirtualizedList`, and `VirtualizedList_EXPERIMENTAL` to use the same code without sacrificing type safety or adding further duplication. This added more complexity than it is worth, so I am instead loosening the type on VirtualizedListContext to pass around a more generic handle.
Changelog:
[Internal][Changed] - Allow VirtualizedListContext to support different component type
Reviewed By: javache
Differential Revision: D38017086
fbshipit-source-id: 91e8f6ab2591d3ae9b7f9263711b4a39b78f68e0
Summary: Add annotations to function parameters required for Flow's Local Type Inference project. This codemod prepares the codebase to match Flow's new typechecking algorithm. The new algorithm will make Flow more reliable and predictable.
Reviewed By: bradzacher
Differential Revision: D37388949
fbshipit-source-id: cdcbc98035ce9b6994842005ea46df42de54f9b8
Summary: Add annotations to function parameters required for Flow's Local Type Inference project. This codemod prepares the codebase to match Flow's new typechecking algorithm. The new algorithm will make Flow more reliable and predicatable.
Reviewed By: bradzacher
Differential Revision: D37363351
fbshipit-source-id: a9d3df7db6f9d094ac2ce81aae1f3ab4f62b243a
Summary: Add annotations to function parameters required for Flow's Local Type Inference project. This codemod prepares the codebase to match Flow's new typechecking algorithm. The new algorithm will make Flow more reliable and predicatable.
Reviewed By: evanyeung
Differential Revision: D37353648
fbshipit-source-id: e5a0c685ced85a8ff353d578b373f836b376bb28
Jiamin Zhu
Sam Zhou
Nick Gerleman
Tim Yung
Luna Wei
Vic
Nick Gerleman
Rujin Cao
Rubén Norte
Anne Pham
Pieter Vanderwerff
Tim Yung
Genki Kondo
fabriziobertoglio1987
Antoine Doubovetzky
Kacie Bawiec