Commit Graph

5 Commits

Author SHA1 Message Date
Nick Gerleman 03b4764836 VirtualizedList up-to-date state: Move Props to VirtualizedListProps
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
2022-08-01 14:56:24 -07:00
Nick Gerleman f8e1fedf3d Allow injecting looser VirtualizedList
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
2022-07-27 14:27:52 -07:00
Nick Gerleman 479053cb3c Add VirtualizedList_EXPERIMENTAL (CellRenderMask Usage)
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
2022-07-26 20:06:58 -07:00
David Vacca 8eeb01686f Ship virtualizedlist remeasure fix
Summary:
This diff ships the virtualizedlist remeasure fix implemented in D27003249 (https://github.com/facebook/react-native/commit/1bc06f18c613f9a85d5b631493a09682524016f2)

For more details about experiment see: https://fb.workplace.com/groups/rn.engineering/permalink/2831770330485832/

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D28072308

fbshipit-source-id: c23c958ca9f2991236cb5a5c02084722aa6de919
2021-04-28 20:16:23 -07:00
David Vacca 1bc06f18c6 Fix impression logging in VirtualizedList
Summary:
This diff forces a remeasure of the children of VirtualizedLists everytime the VirtualizedList is measured.
The goal is to ensure that nested VirtualizedList has the correct "scroll information" all the time, scroll information information is used by ViewabilityHelper.computeViewableItems method to determine if Items of the list are visible to the user.

This new code is controlled by the MC: rn_core:enable_virtualizedlist_remeasure_children_if_needed.

changelog: [internal] internal

// I used an internal changelog because this is under a MC

Reviewed By: lunaleaps

Differential Revision: D27003249

fbshipit-source-id: f9452ceb27683b0f595dd4bffdcced0ecf6bb0b5
2021-03-12 15:33:51 -08:00