Commit Graph

54 Commits

Author SHA1 Message Date
Sam Zhou 11d9b9cef0 Prepare to make React.Element fully opaque (#40798)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/40798

Changelog: [Internal]

Reviewed By: sullenor

Differential Revision: D50194982

fbshipit-source-id: 21d2f86dd787952ceed9a4f862a3fcaa2b063349
2023-10-12 08:43:32 -07:00
Alex Hunt f40bb9331c Bump package versions for next major (#39764)
Summary:
#publish-packages-to-npm

Bump all package versions to `0.74.0` (next major release) for `main`, following instructions at https://reactnative.dev/contributing/release-branch-cut-and-rc0#12-bump-minor-version-of-all-monorepo-packages-in-main.

```sh
yarn bump-all-updated-packages --release-branch-cutoff
```

Changelog: [Internal]

Pull Request resolved: https://github.com/facebook/react-native/pull/39764

Test Plan: —

Reviewed By: robhogan

Differential Revision: D49871039

Pulled By: huntie

fbshipit-source-id: f4df6efeae4d8a9209e7aae7b9e6fea3d15793b5
2023-10-11 08:36:23 -07:00
Nick Gerleman 22a7b8dd37 Remove code to support bottom-up layout events in horizontal RTL (#39646)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39646

We can dramatically simplify this code and remove quirks/hacks, now that we can assume layout events are always fired top down.

Changelog: [Internal]

Reviewed By: yungsters

Differential Revision: D49628669

fbshipit-source-id: 7de5bbc4597eba1c59aaa7672c70e76d2786c7ef
2023-10-10 11:43:14 -07:00
Nick Gerleman a2fb46ec0d Fix invariant violation when using viewability callbacks with horizontal RTL FlatList on Paper (#39335)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39335

In RTL we must have scrollview content length in order to resolve cell metrics. This means that on Paper, where layout events are bottom up, we cannot immediately calculate viewability in response to cell metric changes, as we may not yet have an accurate length of laid out list content.

This change makes us defer calculation of viewability changes in this case via `setTimeout()`, to run in a single batch after the next layout events are fired.

 ---

We need container dimensions to resolve the right-edge relative child directions. It is tricky to do this at a guaranteed right time with onLayout model on Paper.
When we are laying out children, the first layout on Paper looks like:

1. Child is laid out

2. Container is laid out

However, we will never see container onLayout if a child layout does not change the dimensions of the parent container. This will be the common case of subsequent child layouts, where the spacer size was accurate.

I.e. we may or may not ever see content laid out, but can only rely on having both offsets be up to date if we trigger calculation after the container layout would have happened. This is not an issue on Fabric where layout events are fired top-down, or for the most common cases of VirtualizedList, where we run calculations in idle batches that will happen after layout is set, but ends up causing problems for two scenarios I didn't originally account for:

We may recalculate cells to render immediately instead scheduling a later update if the list thinks blanking is immediate (high priority render). This means we cannot do an immediate update in response to cell layout, but we can in response to events batched after layout events are all dispatched, or in worst case delay in Paper RTL.
We do not batch/schedule viewability calculations in response to cell layout in the same wasy as we do for calculating cells to render, but do need them to trigger recalculation.

The way less hacky, but much more invasive solution, that would simplify a lot of this, would be to include parentWidth and parentHeight in onLayout events for Paper (and Fabric for consistency), so that we don't need to rely on event ordering, or sometimes not firing. I thought this would be too much at first, if we didn't have other use-cases, but am more and more tempted to tear down a lot of what we have here to do that instead, since this is not going to be able to rely on useLayoutEffect or IntersectionObserver in today's VirtualizedList because it will need to support Paper for the forseeable future..

Changelog:
[General][Fixed] - Fix invariant violation when using viewability callbacks with horizontal RTL FlatList on Paper

Reviewed By: yungsters

Differential Revision: D49072963

fbshipit-source-id: accd33e2c50935bb67700d94820f6418f130fe08
2023-09-11 18:35:50 -07:00
Nick Gerleman 4f8a8ce316 Fix inverted contentOffset in scroll events in RTL (#39031)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39031

`UIScrollView` `contentOffset` is flow-relative, so `x` is relative to the right edge of the screen. This coordinate space disagrees with layout events, `scrollTo` coordinates, and other platforms.

This applies the same logic we use for inverting `scrollTo` coordinates to invert `contentOffset` in scroll events, in both Paper and Fabric. We then remove the iOS specific workaround we have in VirtualizedList.

I did not test `contentInset` as part of this, whose structure has explicit edges like `left` and `right`.

Changelog:
[iOS][Fixed] - Fix inverted `contentOffset` in scroll events in RTL

Reviewed By: rozele

Differential Revision: D48379915

fbshipit-source-id: 8a9cbb01608e79ef3b179a76fbe3997a0cd23845
2023-08-21 18:03:43 -07:00
Nick Gerleman c522837d9e ListMetricAggregator UTs - contentLength caching (#38738)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38738

UTs about returned cached content length and cached content length requirement in RTL.

Changelog: [Internal]

Reviewed By: rozele

Differential Revision: D47978634

fbshipit-source-id: 509b2b60cae4a8755745710fb5872da27c8ce7a3
2023-08-17 21:31:31 -07:00
Nick Gerleman 298d297e54 Null out ListMetricsAggregator contentLength on orientation change (#38937)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38937

It is a logic error to query for list metrics when a valid list content length has not yet been observed.

An invariant is meant to catch this if it happens, but a non-default orientation causes us to zero out a value which is null, and our null check does not catch this, and we return a nonsense value.

Change from zeroing out the field to nulling it out instead.

Changelog: [Internal]

Reviewed By: lenaic

Differential Revision: D48251645

fbshipit-source-id: 5c7e16f694db4ddb0d37b6092de2e75ee8d6c7d3
2023-08-17 21:31:31 -07:00
Nick Gerleman be2bb51c70 ListMetricAggregator UTs - cell offset APIs (#38732)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38732

Unit tests for cell offset APIs. Mostly the same as previous UTs for VirtualizedList a layer higher.

Changelog: [Internal]

Reviewed By: rozele

Differential Revision: D47978633

fbshipit-source-id: 8cb8a2e8125bc7370eabf9f01a3f7529043171c2
2023-08-15 21:56:25 -07:00
Nick Gerleman 966f97acdc ListMetricAggregator UTs - cached measurement adjustment after layout changes (#38936)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38936

Unit tests that validate we continue to return correct measurement results through the lifetime of the list when cells are unmounted, or layout of the list shifts.

Changelog: [Internal]

Reviewed By: lenaic

Differential Revision: D47978632

fbshipit-source-id: b700fafb699e8820de6825f0ead1ef02c6d8f168
2023-08-15 21:56:25 -07:00
Luna Wei 824c1c6d07 Math.floor the top, bottom values of an item in a VirtualizedList (#38962)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38962

Changelog: [General][Changed] Math.floor the top and bottom dimensions of a cell item when determining viewability.

Reviewed By: NickGerleman

Differential Revision: D48212402

fbshipit-source-id: 0ba7d5c218477c257a4504391940d916e4832f91
2023-08-11 13:57:21 -07:00
Nick Gerleman 0e69050612 Use right edge of ScrollView viewport for scrollMetrics.offset in RTL
Summary:
The offset we record should be the one closest to the reference zero-point in the coordinate space. This makes scroller offset reference match the cell reference we keep in D47978631.

Changelog:
[General][Fixed] - Use right edge of ScrollView viewport for `scrollMetrics.offset` in RTL

bypass-github-export-checks

Reviewed By: lenaic

Differential Revision: D48132236

fbshipit-source-id: 3307081e5e859f1b4afbc15a84c5be1b33915206
2023-08-07 19:36:07 -07:00
Nick Gerleman 9eec875a17 ListMetricAggregator UTs - cell measurement (#38735)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38735

UTs around cell measurement results through `getCellMetrics` and `getCellMetricsApprox`. For each orientation, validate basic scenarios for approximation, cached measurement, or measurement by user-provided `getItemLayout`.

Changelog: [Internal]

Reviewed By: rozele

Differential Revision: D47978630

fbshipit-source-id: 4c9ba6a60599848034f4f23cde3497dd4c2b8788
2023-08-04 10:37:57 -07:00
Nick Gerleman f3b2d2c759 ListMetricAggregator UTs - statistics collection (#38740)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38740

UTs around ListMetricsAggregator APIs which calculate statistics (average length, max measured index).

Changelog: [Internal]

Reviewed By: rozele

Differential Revision: D47978636

fbshipit-source-id: fe83a1fed939fca3d59ecca32945825d3c112739
2023-08-04 10:37:57 -07:00
Nick Gerleman ace0a80dea Fixup contentLength invalidation logic (#38733)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38733

I was working under the assumption that Fabric fired layout events bottom up, but it actually fires them top-down, in constrast to Paper.

Previous invalidation logic wasn't quite correct when fired bottom-up. This corrects the logic by:
1. Deriving direction based on initial event ordering
2. Use last cached contentLength if we are on Fabric (top-down)
3. Use future contentLength if we are on Paper (bottom-up)

Changelog:
[General][Fixed] - Fixup contentLength invalidation logic

Reviewed By: rozele

Differential Revision: D47978638

fbshipit-source-id: 3446d08aa34397b4e6bd9924dad0eba36a12a115
2023-08-04 10:37:57 -07:00
Nick Gerleman 5596f1c25b Right align scrollToIndex in RTL (#38737)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38737

This fixes up behavior on Android so that `scrollToIndex` aligns the right edge of the cell of the given index to the right edge of the scrollview viewport. We do not incorporate RTL on iOS which inverts x/y coordinates from scroller (but not layout).

Changelog:
[General][Fixed] - Right align scrollToIndex in RTL

Reviewed By: lenaic

Differential Revision: D47978637

fbshipit-source-id: 7786b5d97efaced318018409e2c7577a3d8f7402
2023-08-04 10:37:57 -07:00
Nick Gerleman 7319a28d22 Return right edge in RTL Cell metrics (#38734)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38734

Returned measurements from the measurements cache in RTL calculate offset as distance from the left edge of the cell to the right edge of the content, when it should instead be the distance from the right edge of the cell (the logical beginning).

Changelog:
[General][Fixed] - Return right edge in RTL cell metrics

Reviewed By: lenaic

Differential Revision: D47978631

fbshipit-source-id: b0db4e9aff676c5bee81d4491f901a6bbc38e4bf
2023-08-04 10:37:57 -07:00
Nick Gerleman 33d6da01ea Cache ScrollView content length before calling scrollToIndex (#38736)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38736

`scrollToIndex` relies on cached layout information, so we should cache the results from `onContentSizeChange` before attempting the scroll. Otherwise we will fail to scroll in RTL.

Changelog:
[General][Fixed] Cache ScrollView content length before calling `scrollToIndex`

Reviewed By: lenaic

Differential Revision: D47978635

fbshipit-source-id: 27f2a4702650e8a73e8812128821ca03f36216dd
2023-08-02 21:13:43 -07:00
Nick Gerleman 3eccc53629 Remove default 50ms Scroll Event Throttling in VirtualizedList (#38648)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38648

https://github.com/facebook/react-native/pull/38475 made this code no longer no-op on Android, which caused regressions documented in https://github.com/facebook/react-native/issues/38470#issuecomment-1639620459 due to VirtualizedList having more out-of-date information.

We are already coalescing scroll events on both Android and iOS, which will ensure we are not flooded with events. VirtualizedList also already inserts an artificial 50ms delay to new renders by default when high priority work is not happening (see `updateCellsBatchingPeriod`). This limits the heavy work done by VirtualizedList (no new renders or expensive math on scroll events), while letting the list still have the most recent events.

We can eventually remove this once VirtualizedList is able to use OffScreen universally.

Changelog:
[General][Changed] - Remove default 50ms Scroll Event Throttling in VirtualizedList

Reviewed By: ryancat

Differential Revision: D47823772

fbshipit-source-id: 55d22a1074235ccc1b2cf167f6b1758640c79edb
2023-07-31 13:58:56 -07:00
Nick Gerleman c168a4f88b Fix invariant violation when maintainVisibleContentPosition adjustment moves window before list start (#38655)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38655

https://github.com/facebook/react-native/pull/35993 added logic in VirtualizedList to support `maintainVisibleContentPosition`. This logic makes sure that a previously visible cell being used as an anchor remains rendered after new content is added.

The strategy here is to calculate the difference in previous and new positions of the anchor, and move the render window to its new location during item change. `minIndexForVisible` is used as this anchor.

When an item change moves the anchor to a position below `minIndexForVisible`, shifting the render window may result in a window which starts before zero. This fixes up `_constrainToItemCount()` to handle this.

Changelog:
[General][Fixed] - Fix invariant violation when `maintainVisibleContentPosition` adjustment moves window before list start

Reviewed By: yungsters

Differential Revision: D47846165

fbshipit-source-id: 8a36f66fdad321acb255745dad85618d28c54dba
2023-07-28 12:55:17 -07:00
Hanno J. Gödecke 3dd816c6b7 Add workaround fix for #35350 (#38073)
Summary:
This PR is a result of this PR, which got merged but then reverted:

- https://github.com/facebook/react-native/pull/37913

We are trying to implement a workaround for https://github.com/facebook/react-native/issues/35350, so react-native users on android API 33+ can use `<FlatList inverted={true} />` without running into ANRs.

As explained in the issue, starting from android API 33 there are severe performance issues when using scaleY: -1 on a view, and its child view, which is what we are doing when inverting the ScrollView component (e.g. in FlatList).

This PR adds a workaround. The workaround is to also scale on the X-Axis which causes a different transform matrix to be created, that doesn't cause the ANR (see the issue for details).
However, when doing that the vertical scroll bar will be on the wrong side, thus we switch the position in the native code once we detect that the list is inverted, using the newly added `isInvertedVirtualizedList` prop.

This is a follow up PR to:

- https://github.com/facebook/react-native/pull/38071

⚠️ **Note:** [38071](https://github.com/facebook/react-native/pull/38071) needs to be merged and shipped first! Only then we can merge this PR.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[ANDROID] [FIXED] - ANR when having an inverted FlatList on android API 33+

Pull Request resolved: https://github.com/facebook/react-native/pull/38073

Test Plan:
- Check the RN tester app and see that scrollview is still working as expected
- Add the `internalAndroidApplyInvertedFix` prop as test to a scrollview and see how the scrollbar will change position.

Reviewed By: cortinico

Differential Revision: D47848063

Pulled By: NickGerleman

fbshipit-source-id: 4a6948a8b89f0b39f01b7a2d44dba740c53fabb3
2023-07-28 10:55:52 -07:00
Nick Gerleman 8b39bfadbb Fix virtualization logic with horizontal RTL lists (#38529)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38529

VirtualizedList internally represents metrics along the scrolling axis using `offset` (x, y), and `length` (width, height). This one dimensional representation allows the list to be generic to being horizontal or vertical.

Right now offset conversion directly takes the `x` or `y` axis value, but  this is not correct when we are using a horizontal FlatList in RTL.

This change converts most VirtualizedList code handling  `offset,length` coordinates consistently flow from start to end, including in horizontal RTL and in inverted FlatList.

This is paired with a fix to Android native code in D47627115. With these together, basic Horizontal FlatList scenarios should work correctly when laid out in RTL.

Changelog:
[General][Fixed] - Fix virtualization logic with horizontal RTL lists

Reviewed By: vincentriemer

Differential Revision: D46586420

fbshipit-source-id: 79594e197d21871bb493399999e91fc0d6c7b050
2023-07-27 16:26:18 -07:00
Nick Gerleman a41c086e00 Move Default Prop Helpers to VirtualizedListProps.js (#38329)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38329

Some quick cleanup

Changelog: [Internal]

Reviewed By: lunaleaps

Differential Revision: D46659045

fbshipit-source-id: c57315fe7294af012c3db42e40533c49295e31bd
2023-07-13 17:28:40 -07:00
Arushi Kesarwani 2eb25cbdbe Update Node.js to v18 in all RN packages (#37791)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/37791

## Bump minimum Node JS version to 18 via `react-native/package.json#engines`

In https://github.com/facebook/react-native/pull/35443 we bumped up the node version from 16 to 18.

Node 16 [ends maintenance releases on 2023-09-11](https://nodejs.org/en/blog/announcements/nodejs16-eol/), and bumping this minimum will allow other associated Node JS tools (CLI, Metro, Jest) to reduce their support burden.

This follows up by formally making Node 18 the minimum supported version.

**Docs PR:**
https://github.com/facebook/react-native-website/pull/3748

**Changelog:**
[General][Breaking] Bump minimum Node JS version to 18

Reviewed By: cortinico, NickGerleman

Differential Revision: D46583997

fbshipit-source-id: 1f31e2f205ac8b09494c2a7d3b73b9f36eff221b
2023-06-22 09:53:21 -07:00
Nick Gerleman f544376f7c Revert D46871197: Add workaround for android API 33 ANR when inverting ScrollView
Differential Revision:
D46871197

Original commit changeset: 872a2ce5313f

Original Phabricator Diff: D46871197

fbshipit-source-id: d07e9e536d578f0612126bae07a83a02b5e6b792
2023-06-22 01:34:56 -07:00
Hanno J. Gödecke 90186cd9b7 Add workaround for android API 33 ANR when inverting ScrollView (#37913)
Summary:
As explained in this issue:

- https://github.com/facebook/react-native/issues/35350

starting from android API 33 there are severe performance issues when using `scaleY: -1` on a view, and its child view, which is what we are doing when inverting the `ScrollView` component (e.g. in `FlatList`).

This PR adds a workaround. The workaround is to also scale on the X-Axis which causes a different transform matrix to be created, that doesn't cause the ANR (see the issue for details).
However, when doing that the vertical scroll bar will be on the wrong side, thus we switch the position in the native code once we detect that the list is inverted.

The goal of this PR is that react-native users can just use `<FlatList inverted={true} />` without running into any ANRs or the need to apply manual hot fixes 😄

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID] [FIXED] - ANR when having an inverted `FlatList` on android API 33+

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[ANDROID] [FIXED] - ANR when having an inverted `FlatList` on android API 33+

Pull Request resolved: https://github.com/facebook/react-native/pull/37913

Test Plan:
- The change is minimal, and only affects android.
- Run the RNTesterApp for android and confirm that in the flatlist example the inverted list is still working as expected.

Reviewed By: rozele

Differential Revision: D46871197

Pulled By: NickGerleman

fbshipit-source-id: 872a2ce5313f16998f0e4d2804d61e4d8dca7bfd
2023-06-21 19:57:19 -07:00
Tommy Nguyen 7a2a3278d0 fix(virtualized-lists): react-test-renderer is not a runtime dependency (#37955)
Summary:
Installing `react-native` 0.72.x causes a warning about `react-test-renderer` because `react-native/virtualized-lists` has declared a peer dependency on it. As far as I know, it is not used for anything but tests.

```
➤ YN0002: │ react-native@npm:0.72.0-rc.6 [292eb] doesn't provide react-test-renderer (p5a2fb), requested by react-native/virtualized-lists
```

Note that while many package managers default to warnings in this case, there are still a number of users out there for which this is an error.

## Changelog:

[GENERAL] [FIXED] - `react-native/virtualized-lists` does not need `react-test-renderer` at runtime

Pull Request resolved: https://github.com/facebook/react-native/pull/37955

Test Plan: n/a

Reviewed By: rshest

Differential Revision: D46871536

Pulled By: NickGerleman

fbshipit-source-id: 1e5e15608ab394bc43cd4e6ac727a74734874642
2023-06-21 04:14:25 -07:00
Ruslan Shestopalyuk 92525ef3ae Untangle dependency loop in virtualized-lists (#37912)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/37912

# Changelog:
[Internal] -

I've noticed bundler spamming with cyclical dependency warnings when working on FB4A.

This gets rid of the unneeded two-way dependency between `ListMetricsAggregator <-> VirtualizeUtils`.

Reviewed By: NickGerleman

Differential Revision: D46761080

fbshipit-source-id: a25f7eebbd177e24c39dad9af7ac574f3aa30f35
2023-06-16 00:44:20 -07:00
Pieter Vanderwerff dc2037c0f1 Switch xplat prettier config to hermes plugin (#37915)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/37915

Enable the `prettier-plugin-hermes-parser` in xplat. This plugin enables the use of `hermes-parser` which is significantly faster than the current flow parser prettier bundles (improves formatting time by ~50%) and also brings support for the latest Prettier 3.0.0 printing logic for JS. This upgrade is required in order to enable upcoming Flow features that add new syntax.

Changelog: [Internal]

Reviewed By: SamChou19815

Differential Revision: D46748891

fbshipit-source-id: 3775ef9afa7c04e565fa4fcf8ca5b410f49d35a1
2023-06-15 08:23:06 -07:00
Nick Gerleman 8f7f0bf2a3 Extract logic to CellMetricsAggregator (#37777)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/37777

This extracts the state and logic VirtualizedList uses to query information related to cell metrics. We will need to modify this (and other places) when fixing up RTL support for horizontal FlatList.

Changelog: [Internal]

Reviewed By: javache

Differential Revision: D46427052

fbshipit-source-id: 0a23f6c726447de0f20c583b4d507003efd6a754
2023-06-08 15:53:10 -07:00
Nick Gerleman 96225cec2b Rename FrameMetrics to CellMetrics (#37778)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/37778

These functions describe how to get the metrics of a specific cell. "Frame" here seems non-descriptive and redundant to the "Metrics" part. Renaming this before a larger refactor.

Changelog: [Internal]

Reviewed By: philIip

Differential Revision: D46427058

fbshipit-source-id: e9ad9cf15e1adfd07604eb11526de0ed8cf99000
2023-06-08 15:53:10 -07:00
Pranav Yadav 14316bd635 Add missing READMEs & Update package.json in all RN packages (#37090)
Summary:
This diff adds _missing_ README files for all public RN packages.

#### Changes:
For all public RN packages:
- Add _Missing_ READMEs

Update package.json in all RN packages to add:
- Issues, Bugs urls
- Keywords and Homepage urls to respective pkgs

## Changelog:

[GENERAL][ADDED] - Add missing README files for all public RN packages.
[GENERAL][CHANGED] - Update package.json in all RN packages to add required fields.

Pull Request resolved: https://github.com/facebook/react-native/pull/37090

Test Plan: - `yarn lint && yarn flow && yarn test-ci` --> _should be green_

Reviewed By: cortinico

Differential Revision: D45390861

Pulled By: hoxyq

fbshipit-source-id: 524a92de56a7cb553573d9f54ccf40a998dfd35f
2023-04-28 04:26:20 -07:00
Janic Duplessis 69b22c9799 Fix VirtualizedList with maintainVisibleContentPosition (#35993)
Summary:
`maintainVisibleContentPosition` is broken when using virtualization and the new content pushes visible content outside its "window". This can be reproduced in the example from this diff. When using a large page size it will always push visible content outside of the list "window" which will cause currently visible views to be unmounted so the implementation of `maintainVisibleContentPosition` can't adjust the content inset since the visible views no longer exist.

The first illustration shows the working case, when the new content doesn't push visible content outside the window. The red box represents the window, all views outside the box are not mounted, which means the native implementation of `maintainVisibleContentPosition`  has no way to know it exists. In that case the first visible view is https://github.com/facebook/react-native/issues/2, after new content is added https://github.com/facebook/react-native/issues/2 is still inside the window so there's not problem adjusting content offset to maintain position. As you can see Step 1 and 3 result in the same position for all initial views.

The second illustation shows the broken case, when new content is added and pushes the first visible view outside the window. As you can see in step 2 the view https://github.com/facebook/react-native/issues/2 is no longer rendered so there's no way to maintain its position.

#### Illustration 1

![image](https://user-images.githubusercontent.com/2677334/163263472-eaf7342a-9b94-4c49-9a34-17bf8ef4ffb9.png)

#### Illustration 2

![image](https://user-images.githubusercontent.com/2677334/163263528-a8172341-137e-417e-a0c7-929d1e4e6791.png)

To fix `maintainVisibleContentPosition` when using `VirtualizedList` we need to make sure the visible items stay rendered when new items are added at the start of the list.

In order to do that we need to do the following:

- Detect new items that will cause content to be adjusted
- Add cells to render mask so that previously visible cells stay rendered
- Ignore certain updates while scroll metrics are invalid

### Detect new items that will cause content to be adjusted

The goal here is to know that scroll position will be updated natively by the `maintainVisibleContentPosition` implementation. The problem is that the native code uses layout heuristics which are not easily available to JS to do so. In order to approximate the native heuristic we can assume that if new items are added at the start of the list, it will cause `maintainVisibleContentPosition` to be triggered. This simplifies JS logic a lot as we don't have to track visible items. In the worst case if for some reason our JS heuristic is wrong, it will cause extra cells to be rendered until the next scroll event, or content position will not be maintained (what happens all the time currently). I think this is a good compromise between complexity and accuracy.

We need to find how many items have been added before the first one. To do that we save the key of the first item in state `firstItemKey`. When data changes we can find the index of `firstItemKey` in the new data and that will be the amount we need to adjust the window state by.

Note that this means that keys need to be stable, and using index won't work.

### Add cells to render mask so that previously visible cells stay rendered

Once we have the adjusted number we can save this in a new state value `maintainVisibleContentPositionAdjustment` and add the adjusted cells to the render mask.

This state is then cleared when we receive updated scroll metrics, once the native implementation is done adding the new items and adjusting the content offset.

This value is also only set when `maintainVisibleContentPosition` is set so this makes sure this maintains the currently behavior when that prop is not set.

### Ignore certain updates while scroll metrics are invalid

While the `maintainVisibleContentPositionAdjustment` state is set we know that the current scroll metrics are invalid since they will be updated in the native `ScrollView` implementation. In that case we want to prevent certain code from running.

One example is `onStartReached` that will be called incorrectly while we are waiting for updated scroll metrics.

## Changelog

[General] [Fixed] - Fix VirtualizedList with maintainVisibleContentPosition

Pull Request resolved: https://github.com/facebook/react-native/pull/35993

Test Plan:
Added bidirectional paging to RN tester FlatList example. Note that for this to work RN tester need to be run using old architecture on iOS, to use new architecture it requires https://github.com/facebook/react-native/pull/35319

Using debug mode we can see that virtualization is still working properly, and content position is being maintained.

https://user-images.githubusercontent.com/2677334/163294404-e2eeae5b-e079-4dba-8664-ad280c171ae6.mov

Reviewed By: yungsters

Differential Revision: D45294060

Pulled By: NickGerleman

fbshipit-source-id: 8e5228318886aa75da6ae397f74d1801d40295e8
2023-04-27 15:31:35 -07:00
Pranav Yadav a58dea1e9e Update Node.js to v16 in all RN packages (#37073)
Summary:
**NOTE**: This is a **BREAKING** change.
TLDR; Enforce minimum Node.js v16 in all RN packages.

This diff **Updates Node.js to v16** across all RN packages.

#### Context:

- For RN development and new project created; bump to node 16 was in https://github.com/facebook/react-native/pull/36217
- Recently `react-native-windows` also; updated node to v16, https://github.com/microsoft/react-native-windows/pull/11500

#### Changes:

- [BREAKING] Update Node.js to v16 across all RN packages under 'packages/' dir

## Changelog:

[GENERAL][BREAKING] - Update Node.js to v16 in all RN packages

Pull Request resolved: https://github.com/facebook/react-native/pull/37073

Test Plan: - `yarn lint && yarn flow && yarn test-ci` --> _should be green_

Reviewed By: cipolleschi

Differential Revision: D45306108

Pulled By: jacdebug

fbshipit-source-id: e3ba7d0151b86a6a0a3d63fb29c2bd887e1ac1e7
2023-04-26 08:24:06 -07:00
George Zahariev e2116d277d Codemod $Shape to Partial in xplat, suppressing errors [4] (#36960)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36960

We're deprecating the unsafe `$Shape` and moving to the safe `Partial`: https://fb.workplace.com/groups/flowlang/posts/1251655088773485

I have previously codemodded all locations that do not cause errors. Now start on the remaining ones: codemod and suppress.

Changelog: [Internal]

Reviewed By: SamChou19815

Differential Revision: D45076273

fbshipit-source-id: 27ebf33370143e19751dbdcfcc1876cf3c586e14
2023-04-18 13:49:07 -07:00
Oskar Kwaśniewski eb30a80c81 fix: make sure initialScrollIndex is bigger than 0 (#36844)
Summary:
Hey,

`adjustCellsAroundViewport` function was checking if `props.initialScrollIndex` is truthy and -1 was returning true. This caused bugs with rendering for tvOS: https://github.com/react-native-tvos/react-native-tvos/pull/485 There are warnings in the code about `initalScrollIndex` being smaller than 0 but this if statement would still allow that.

## Changelog:

[General] [Fixed] - Make sure initialScrollToIndex is bigger than 0 when adjusting cells

Pull Request resolved: https://github.com/facebook/react-native/pull/36844

Test Plan: Pass -1 as initialScrollToIndex. Check that this code is executed.

Reviewed By: cipolleschi

Differential Revision: D44856266

Pulled By: NickGerleman

fbshipit-source-id: 781a1c0efeae93f00766eede4a42559dcd066d7d
2023-04-18 09:51:51 -07:00
Broda Noel 4264ddb059 Fix typo (#36956)
Summary:
Fixing a simple typo

## Changelog:
Not applicable

Pull Request resolved: https://github.com/facebook/react-native/pull/36956

Test Plan: Not applicable

Reviewed By: NickGerleman

Differential Revision: D45085878

Pulled By: cortinico

fbshipit-source-id: 2b338c7a68cdbc38da85d74acaae3854df561cfe
2023-04-18 09:45:55 -07:00
leedom 234f1999a1 chore: update all website links about react (#36638)
Summary:
New website of React is online, so some links about react should be also updated in README.md.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[General] [Fixed] - Fix some links about react in readme

Pull Request resolved: https://github.com/facebook/react-native/pull/36638

Test Plan: The PR just changes some links in readme, don't need to test.

Reviewed By: jacdebug

Differential Revision: D44959199

Pulled By: blakef

fbshipit-source-id: 14a66a2d2b086fbe16f715c621c59a6d8d6ee698
2023-04-14 06:23:52 -07:00
Ruslan Lesiutin 8a18b53bcd bump packages for 0.73 (#36556)
Summary:
Changelog: [Internal]

Publishing to check CI if bumping and aligning in the same commit will work, since these new versions are not available on npm yet, but maybe our new monorepo setup will resolve this

**Adding back `react-native/virtualized-lists` as a workspace to `xplat/js` so that it won't be resolved from npm**

#publish-packages-to-npm

Pull Request resolved: https://github.com/facebook/react-native/pull/36556

Reviewed By: cipolleschi

Differential Revision: D44255353

Pulled By: hoxyq

fbshipit-source-id: 21372487d6e9c0b2382b7cd9af835beed46b8ce1
2023-03-30 15:55:58 -07:00
Nick Gerleman 776fe7a292 Bail on realizing region around last focused cell if we don't know where it is (#36541)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36541

We only know where the last focused cell lives after it is rendered. Apart from dead refs handled in D43835135, this means items added or removed before the focused cell will lead to a stale last index for the next state update.

We don't have a good way to correlate cells after data change. This effects the implementation for [`maintainVisibleContentPosition`](https://github.com/facebook/react-native/pull/35993) as well, but needs some thought on the right way to handle it. For now, we bail when we don't know where the last focused cell lives.

Changelog:
[General][Fixed] - Bail on realizing region around last focused cell if we don't know where it is

Reviewed By: yungsters

Differential Revision: D44221162

fbshipit-source-id: 8fc7e726fe13c62b98870600563857bb89290280
2023-03-24 02:30:20 -07:00
Nick Gerleman 2d41e6642e Fix types + documentation for CellRendererComponent
Summary:
CellRendererComponent can be given a more useful description, and more constrained type, to ensure it is used more correctly.

Changelog:
[General][Fixed] - Fix types + documentation for CellRendererComponent

Reviewed By: yungsters

Differential Revision: D43925572

fbshipit-source-id: 26aae6a2df989993c97709ffbf1544df7cbae036
2023-03-14 15:32:05 -07:00
Nick Gerleman d595fbcc5a Allow out-of-range initialScrollIndex after first scroll
Summary:
The contract here is that `initialScrollIndex` only applies once, right after the components mount. There is other code still relying on live `initialScrollIndex`, which is allowed to become stale. E.g. after removing items.

I looked at a larger change of only ever using `initialScrollIndex` in the start, so we have a consistent value. We also ideally should fix up the logic relying on it for the scroll to top optimization.

That series of changes is more involved than I want to spend time on, so this just avoids the check once we have triggered a scroll, where the rest of the code is UT'd to be permissive if it drifts out of allowed.

Changelog:
[General][Fixed] - Allow out-of-range initialScrollIndex after first scroll

Reviewed By: yungsters

Differential Revision: D43926656

fbshipit-source-id: bd09bd9a9aa6b3b5f07209dac8652c9374a762c4
2023-03-14 09:47:56 -07:00
Nick Gerleman c376e78224 Delete refs to unmounted CellRenderers
Summary:
VirtualizedList today will keep refs to cells around, long after they have been unmounted. This leaks memory, and is not needed.

Changelog:
[General][Fixed] - Delete refs to unmounted CellRenderers

Reviewed By: yungsters

Differential Revision: D43835135

fbshipit-source-id: 2104cae977a4e2e9e1a2738e1523ac1796293b4f
2023-03-08 18:59:13 -08:00
Nick Gerleman 7858a2147f Enforce compatibility with exactOptionalPropertyTypes (#36345)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36345

`exactOptionalPropertyTypes` is a TypeScript 4.4+ option set by users which changes behavior of optional properties, to disable accepting explicit `undefined`.

This is not enabled when using `--strict`, and is stricter than Flow, leading to most of the typings having an `| undefined` unique to TypeScript (added with https://github.com/DefinitelyTyped/DefinitelyTyped/commit/694c663a9486dbe7794d5eb894a691ee9ded318a).

We have not always followed this (I have myself previously assumed the two are equivalent). We can enforce that the convention is followed with a plugin `eslint-plugin-redundant-undefined`. This forces us to declare that every optional property accepts an explicit undefined (which Flow would allow). Alternatively, if we do not want to support this, we can enable the existing dtslint rule `no-redundant-undefined`.

Changelog:
[General][Fixed] - Enforce compatibility with `exactOptionalPropertyTypes`

Reviewed By: lunaleaps

Differential Revision: D43700862

fbshipit-source-id: 996094762b28918177521a9471d868ba87f0f263
2023-03-08 00:14:56 -08:00
Nick Gerleman aab9df3710 Gracefully handle out-of-bounds initialScrollIndex
Summary:
Changelog:
[General][Fixed] - Gracefully handle out-of-bounds initialScrollIndex

Reviewed By: rshest

Differential Revision: D43672964

fbshipit-source-id: dbd9007c538015fc586e573d268135b7557dc908
2023-03-02 16:13:04 -08:00
Nicola Corti b0863e1239 Bumping all the changes we have on main (#36355)
Summary:
This will publish several changes we have pending on main, specifically changes to React Native Gradle Plugin which are needed to unblock nightlies.

#publish-packages-to-npm

## Changelog

[INTERNAL] - Bumping all the changes we have on main

Pull Request resolved: https://github.com/facebook/react-native/pull/36355

Test Plan: n/a

Reviewed By: hoxyq

Differential Revision: D43733634

Pulled By: cortinico

fbshipit-source-id: 9c041f7557cd8e494dfc942ae89e13e55353bb48
2023-03-02 07:11:43 -08:00
Nick Gerleman 62a0640e4a Avoid VirtualizedList viewability updates during state updates
Summary:
VirtualizedList refactoring moved [a call of `_updateViewableItems`](https://www.internalfb.com/code/fbsource/[a9d4ad3cf149][history][blame]/xplat/js/react-native-github/Libraries/Lists/VirtualizedList.js?lines=1431-1447) to the inside of a state update.

This call may trigger an `onViewableItemsChanged`, which is normally not an issue (minus changing timing), but creates problems if the users callback then calls imperative methods on the VirtualizedList, since the batched state update may be in the process of changing the props/state the imperative method is reading. See https://github.com/facebook/react-native/issues/36329 for what I suspect is an example of this.

This moves the `_updateViewableItems` call to before the state update, like the previous version of VirtualizedList.

Changelog:
[General][Fixed] -  Avoid VirtualizedList viewability updates during state updates

Reviewed By: javache

Differential Revision: D43665606

fbshipit-source-id: 9398273c5209e371e69aafb02bac173c69874273
2023-03-01 06:44:33 -08:00
Nicola Corti e42dd1593d bump package versions (#36184)
Summary:
We do have a lot of changes on `main` to ship to nightlies. This change bump all the packages with pending changes.

## Changelog

[INTERNAL] [CHANGED] - [ci][monorepo] bump package versions

Pull Request resolved: https://github.com/facebook/react-native/pull/36184

Test Plan: Will rely on CI run.

Reviewed By: hoxyq

Differential Revision: D43363981

Pulled By: cortinico

fbshipit-source-id: eba5152dbe007eb3fad43f9088d145b3741fd94e
2023-02-17 06:19:37 -08:00
Ruslan Lesiutin 31ac1f8aec fix[virtualized-lists]: do not ship tests in npm package (#36143)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36143

Changelog: [Internal]

Currently, some `__tests__` are included in `react-native/virtualized-lists`, added them to ignore list, so that we will remove it in the next publish

Reviewed By: christophpurrer

Differential Revision: D43159337

fbshipit-source-id: 27ed0adf85387a2e8ac902da57888f0b188b0b91
2023-02-14 04:12:39 -08:00
Skander Ellouze ba7f9b40a6 fix a typo in the initialNumToRenderOrDefault description's comment (#36110)
Summary:
Fix typo in the initialNumToRenderOrDefault description's comment : function's parameter should be this.props.initialNumToRender instead of this.props.initialNumToRenderOrDefault

## Changelog
[GENERAL] [FIXED] - Fixed typo in the initialNumToRenderOrDefault description's comment

Pull Request resolved: https://github.com/facebook/react-native/pull/36110

Test Plan: Typo in a comment - no testing required

Reviewed By: christophpurrer

Differential Revision: D43160548

Pulled By: cortinico

fbshipit-source-id: 0555c7752102f431fb327b920434faaf4de4ff81
2023-02-10 02:10:55 -08:00
Nick Gerleman cab865be79 Fix VirtualizedList usage of potentially stale state on cell focus
Summary:
State updates can be batched together idependent of `this.state`, so we should do any calculation deriving state from state within a `setState()` callback. This fixes a bug where we were relying on potentially stale state, a RenderMask derived from `this.state` instead of the `state` callback parameter, when triggering updates from focus.

Note that this is not exercised on Android/iOS, but it on desktop/web. I noticed this a while back while making another change, but that change got abandoned, so this is the independent fix.

Changelog:
[General][Fixed] - Calculate VirtualizedList render mask for focused cell during batched state updates

Reviewed By: javache

Differential Revision: D43073415

fbshipit-source-id: dee4197ec925a6d8d427b63fb063aa4e3b58c595
2023-02-07 18:25:28 -08:00