Summary:
I was working on `Animated.FlatList` and found some types missing as below
also `Animated.SectionList`
So I refactored type definition for `Animated.FlatList` and `Animated.SectionList` using `abstract class`.
## Changelog
[GENERAL] [FIXED] - add missing FlatList types for Animated FlatList
[GENERAL] [FIXED] - add missing SectionList types for Animated SectionList
<!-- 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
-->
Pull Request resolved: https://github.com/facebook/react-native/pull/36292
Test Plan: Ran `yarn test-typescript` and `yarn test-typescript-offline` with no errors.
Reviewed By: lunaleaps
Differential Revision: D43673884
Pulled By: NickGerleman
fbshipit-source-id: 7ccab5997fa2f22226fb0e106672cee98e568ba4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36237
This changes the data parameter type for `getItemLayout` from a mutable array (too lenient, even before), to `ArrayLike`, which is now the most constrained subset of data which may be passed to a FlatList.
We could do something more exact by adding another generic parameter to FlatList, but that would be likely be noticeably more breaking, since during testing I couldn't manage a pattern that both kept the same minimum number of generic arguments while keeping inference working.
Changelog:
[General][Breaking] - Constrain data type in `getItemLayout` callback
Reviewed By: javache
Differential Revision: D43466967
fbshipit-source-id: 7a1ce717e7d5cc96a58b8d3ad9def6cf6250871f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36236
D38198351 (https://github.com/facebook/react-native/commit/d574ea3526e713eae2c6e20c7a68fa66ff4ad7d2) addedd a guard to FlatList, to no-op if passed `data` that was not an array. This broke functionality where Realm had documented using `Realm.Results` with FlatList. `Real.Results` is an array-like JSI object, but not actually an array, and fails any `Array.isArray()` checks.
This change loosens the FlatList contract, to explicitly allow array-like non-array entities. The requirement align to Flow `ArrayLike`, which allows both arrays, and objects which provide a length and indexer. Flow `$ArrayLike` currently also requires an iterator, but this is seemingly a mistake in the type definition, and not enforced.
Though `Realm.Results` has all the methods of TS `ReadonlyArray`, RN has generally assumes its array inputs will pass `Array.isArray()`. This includes any array props still being checked [via prop-types](https://github.com/facebook/prop-types/blob/044efd7a108556c7660f6b62092756666e39d74b/factoryWithTypeCheckers.js#L548).
This change intentionally does not yet change the parameter type of `getItemLayout()`, which is already too loose (allowing mutable arrays). Changing this is a breaking change, that would be disruptive to backport, so we separate it into a different commit that will be landed as part of 0.72 (see next diff in the stack).
Changelog:
[General][Changed] - Make FlatList permissive of ArrayLike data
Reviewed By: yungsters
Differential Revision: D43465654
fbshipit-source-id: 3ed8c76c15da680560d7639b7cc43272f3e46ac3
Summary:
This change reverts D41745930 (https://github.com/facebook/react-native/commit/2e3dbe9c2fbff52448e2d5a7c1e4c96b1016cf25) as part of a stack to splice back source history which was lost (Git registered the file moves as additions).
It is expected this diff will individually fail. The entire stack should be applied at once.
Changelog: [Internal]
Reviewed By: hoxyq
Differential Revision: D43068113
fbshipit-source-id: c8398629fe5dcc1ca4bf02f550adc00c78a8487a
Summary:
Since currently the check was for null , and that too not === check. So added a check , only for item !== undefined, since null is an assigned value, and we can have null as values in the array for flatlist,
undefined is in absence of any data, hence if its only undefined we should assign falsy to frame variable, since null is an assigned value, sometimes null can be passed to data in the dataset
Hence added a check on top of Sam's previous commit to fix it
UPDATE:
Now after discussing with NickGerleman , removed the check for item with nullish/undefined.
Now directly frames value is being controlled by _keyExtractor function
This is already an issue [https://github.com/facebook/react-native/issues/35280](url)
Currently in my project, even [0,1] -> didnt trigger onViewableItemsChanged since, it was considered as falsy value,
went to check the node modules code for flatlist, debugged this.
When pulled latest main branch, saw it was partially fixed , but for null as values it wasnt fixed. So added that check .
## Changelog
[General] [Fixed] Fix VirtualizedList onViewableItemsChanged won't trigger if first item in data is null
```
const frame =
item !== undefined ? this._frames[this._keyExtractor(item, index, props)]
: undefined;
```
in place of existing which is
```
const frame =
item != null ? this._frames[this._keyExtractor(item, index, props)]
: undefined;
```
Update:
`const frame = this._frames[this._keyExtractor(item, index, props)]`
Finally this is the one used for getting frames value
<!-- 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
Pull Request resolved: https://github.com/facebook/react-native/pull/36009
Test Plan:
Checked out in my local , wrt changes , will share video if required
Update:
After the new changes too, checked in local, working fine
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
Reviewed By: NickGerleman
Differential Revision: D42934757
Pulled By: skinsshark
fbshipit-source-id: cb5622a79523bccbdfbc15470baf84422f635b33
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/35875
Fixes https://github.com/facebook/react-native/issues/35871
Nested VirtualizedLists register to their parents for updates, associated to a specfific cellKey set by VirtualizedListCellContextProvider. This cellKey is usually set when rendering a cell for a data item, but we can also render a nested VirtualizedList by putting one in a ListHeaderComponent/ListFooterComponent/ListEmptyComponent.
D6603342 (https://github.com/facebook/react-native/commit/a010a0cebd4afc0d88336c2c265a5d9dbb19918f) added cellKeys when we render from a header/footer, but not ListEmptyComponent, so that association would silently fail earlier.
D39466677 (https://github.com/facebook/react-native/commit/010da67bef0c22418d0d41b7c2eae664672a4a27) added extra invariants to child list handling, that are now triggered by this case, complaining because we are trying to unregister a child list we never successfully registered, due to a missing cellKey.
This fixes the issue by providing a cellKey for ListEmptyComponent as well.
Changelog:
[General][Fixed] - Fix invariant violation when nesting VirtualizedList inside ListEmptyComponent
Reviewed By: christophpurrer
Differential Revision: D42574462
fbshipit-source-id: f76fa795bf471cb8a929c2efdbd814ea51927663
Summary:
Add `onStartReached` and `onStartReachedThreshold` to `VirtualizedList`. This allows implementing bidirectional paging.
## Changelog
[General] [Added] - Add onStartReached and onStartReachedThreshold to VirtualizedList
Pull Request resolved: https://github.com/facebook/react-native/pull/35321
Test Plan: Tested in the new RN tester example that the callback is triggered when close to the start of the list.
Reviewed By: yungsters
Differential Revision: D41653054
Pulled By: NickGerleman
fbshipit-source-id: 368b357fa0d83a43afb52a3f8df84a2fbbedc132
Summary:
Adds some imperative VirtualizedList methods to the TS types which are documented on the website and are public enough. Also explicitly add `viewOffset` to `scrollToItem` since `scrollToItem` params are indefinite in flow and are passed to `scrollToIndex` (which supports the prop).
Changelog:
[General][Fixed] - Add missing VirtualizedList Imperative Types
Reviewed By: GijsWeterings
Differential Revision: D42047674
fbshipit-source-id: 60f7b35b049853d9fcb724918b3a0008a75ea573
Summary:
This was left over from an old codemod.
Changelog: [Internal]
Reviewed By: christophpurrer
Differential Revision: D41154548
fbshipit-source-id: 0b5fb3e78491b66ebaf13555f80e0265a25dc7d8
Summary:
Breaks the runtime dependency cycle introduced in D40259791 (https://github.com/facebook/react-native/commit/971599317b7bdf1152157206f9503a23ac8c4162) by converting a value import to a type import. (Unlike runtime dependency cycles, type-level cycles are OK as long as they are reasonably small.)
Changelog:
[General][Fixed] - Fix require cycle warning in VirtualizedList
Reviewed By: javache
Differential Revision: D40412019
fbshipit-source-id: 33bf3af12be64a1932549a0d11f2ce8b3c483218
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
Jerry
Nick Gerleman
hoxy
Ruslan Lesiutin
Gabriel Donadel Dall'Agnol
Julien Brayere
Gaurav ROy
Andrey Lunyov
Janic Duplessis
Sam Zhou
Ali Akbar Azizi
GabeDahl
Pieter Vanderwerff
Pieter De Baets
Sam Chan
Mike Vitousek
Moti Zilberman
Jiamin Zhu
Tim Yung
Luna Wei
Vic
Nick Gerleman
Rujin Cao
Rubén Norte
Anne Pham
Pieter Vanderwerff
