Summary:
Changelog:
[General][Changed] - Copied and refactored the current devtools highlighting code from Inspector into its own module and add to the top level `AppContainer`. The effect is that the highlight stills shows without Inspector opened.
This diff copies the current devtools highlighting logic from Inspector into a module and add to the top level `AppContainer`. The effect is without Inspector opened, the highlight will still show.
## Context
This is the first diff for "Component Tab Automatically Highlight Elements". The idea is to replicate the behavior on Web to RN.
## Behavior
on Web:
- highlight shows whenever an element in the component list is hovered
- Selecting an element doesn't keeps the highlight showing.
on RN (before this diff):
- when RN inspector opens: selecting an element keeps the highlight showing
- when RN inspector closes: stop showing highlihgintg.
on RN(this diff)
- selecting an element keeps the highlight showing
## TODO
- See if highlighting event can be sent on hover, instead of when an element is selected.
Reviewed By: lunaruan
Differential Revision: D38568135
fbshipit-source-id: d168874677d08a9c5526a7f896943579da804565
Summary:
This adds an option to the Error Reporting pipeline of React Native to attach custom extra data to Exceptions passed to the native ExceptionsManager. This allows for error logging abstractions such as Meta's FBLogger to attach extra fields to the reported error. This can help with more detailed error reports without having to stringify all data in the error message.
Note: The field (which is technically on ExtendedError) is keyed using a Symbol. This is to make sure that any use of this ability is extremely deliberate, as (accidentally) adding tons of data (or unserializable data) can cause issues we send down the data to the native ExceptionsManager implementation. Data sent using this method should be strictly controlled, hence opting in is a concious effort using the symbol in ExceptionsManager
Changelog:
[Internal] [Added] - Ability to add custom data to extraData field of exceptions passed to the ExceptionsManager
Reviewed By: yungsters
Differential Revision: D36099191
fbshipit-source-id: ce3f0dae52acd742de98b71868323c5878eaa677
Summary:
Changelog:
[General][Fixed] - Run ExceptionsManager unit tests in both __DEV__ and prod mode
This suite of unittests use a `__DEV__` check in various places to check that the correct data is sent to either LogBox (in __DEV__) or the Native ExceptionsManager (in PROD). However, we're not actually running both modes, so half of the assert branches in these tests aren't ever utilized.
This diff uses the same approach as ReactNativeVersionCheck-test.js to execute this test suite twice: Once in DEV and once in PROD mode.
Reviewed By: motiz88
Differential Revision: D36099190
fbshipit-source-id: 40b8ea26f1d9e093202f3c3f3b55111110a8d64c
Summary:
Currently both iOS and Android send over the list of transforms as an array. But there is an if statement that causes other platforms to get a matrix. This prevents other platforms from being able to use the fabric ViewProps class and the conversion functions as they exist in core, as those expect the transforms to be an array.
Stop special casing iOS and android. - Which will allow for example Windows to be able to share more fabric code.
## Changelog
[Internal] [Changed] - All platforms should get transform sent to native as an array of transform operations instead of a matrix
Pull Request resolved: https://github.com/facebook/react-native/pull/33579
Test Plan:
Similar change made in react-native-windows.
https://github.com/microsoft/react-native-windows/issues/9797
Reviewed By: NickGerleman
Differential Revision: D38615676
Pulled By: cortinico
fbshipit-source-id: 8861afe6bf34bebb09dd82f7365faf007dd79cbf
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/34384
This Diff aims to create a RCTAppDelegate library to offer a subclass which automates some operations required to set up the new architecture.
## Changelog
[iOS][Added] - Added the RCTAppDelegate library
Reviewed By: cortinico
Differential Revision: D38580424
fbshipit-source-id: 38f6c4b8ff2790a2ce9e23d385b36307701cffb7
Summary:
Sometime over the past few months (and with changes such as migrating to the `hermes-eslint` parser), a bunch of lint warnings crept into the codebase.
This does a pass to clean them all up, ignore generated files, and refactor some code to be... better.
There should be no observable behavior changes as a result of this.
Changelog:
[Internal]
Reviewed By: NickGerleman
Differential Revision: D38646643
fbshipit-source-id: a7b55d1e4cd5700340cc5c21f928baf3ea1d5a58
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:
needsOffscreenAlphaCompositing is supported on both iOS and Android, but was missing from the view config for Android.
https://reactnative.dev/docs/view#needsoffscreenalphacompositinghttps://fburl.com/code/hfxkrur1
Changelog:
[Internal][Fixed] - Add needsOffscreenAlphaCompositing to view config
Reviewed By: NickGerleman
Differential Revision: D38580371
fbshipit-source-id: 9b577079e575d73c94d7c0d0298ba880c1438099
Summary:
RN for Android fires `keyboardDidShow` and `keyboardDidHide` by observing changes to viewable window size. This isn't always reliable, such as when an activity has `awindowSoftInputMode` set to not have the system adjust the viewport when a keyboard is opened.
Android 11 added the direct ability to measure and check visibility of the soft keyboard via `WindowInsets`, which fixes these issues. This is exposed downlevel to API 23 via WindowInsetsComapt` with the same limitations as previously, but using it simplifies our calculations a lot.
Changelog:
[Android][Fixed] - Use WindowInsetsCompat for Keyboard Events
Reviewed By: javache
Differential Revision: D38500859
fbshipit-source-id: d4ad41d7e75e4b9c14a485539a5f9de19de74362
Summary:
BaseViewManager.setPointerEnter was never called on ReactTextViews, and thus text views would not receive hover events.
Changelog:
[Internal][Fixed] - Add pointer events to text view config
Reviewed By: lunaleaps, javache
Differential Revision: D38557546
fbshipit-source-id: cfc0e5442efbd7c76d1b47acf56496d10ef78cf5
Summary:
Previously published by [grgr-dkrk][2] as [https://github.com/facebook/react-native/issues/31296][3], fixes the following issues:
1) ImportantForAccessibility="no" does not work on Text, Button
2) ImportantForAccessibility="no-hide-descendants" does not work on Text, Button, or ImageBackground.
Note: On ImageBackground, focus is prevented on the imageBackground node itself, but focus is not prevented on its descendants.
Note: [Button component expected behavior for prop importantForAccessibility][4]
>Some components like Button seem like atomic units, but they end up rendering a hierarchy of components for example a wrapper View with a Text inside them. Allowing those descendants to become focusable, breaks the model of these being a single component to consider and forcing no-hide-descendants makes sense here.
>The other option is always to render any descendants of these elements with importantForAccessibility="no", so they can never be focusable on their own. This would have the same result, **BUT may potentially cause issues when the descendant content is supposed to automatically get moved up to the focusable ancestor to act as a label** (which is what Talkback does with unfocusable text elements by default).
Note: [importantForAccessibility="no" does not allow screenreader focus on nested Text Components with accessibilityRole="link" or inline Images][5]
fixes https://github.com/facebook/react-native/issues/30850 related https://github.com/facebook/react-native/pull/33690
## Changelog
[Android] [Fixed] - adding importantForAccessibility for Text, Button, ImageBackground
Pull Request resolved: https://github.com/facebook/react-native/pull/34245
Test Plan:
1) testing ImageBackground importantForAccessiblity ([link to test][1])
2) importantForAccessibility="no" does not allow screenreader focus on nested Text Components with accessibilityRole="link" or inline Images ([link to test][5])
3) testing ImageBackground importantForAccessiblity ([link to test][6])
4) Button with importantForAccessibility=no ([link to test][7])
[1]: https://github.com/facebook/react-native/pull/31296#issuecomment-1192341626 ""
[2]: https://github.com/grgr-dkrk "grgr-dkrk"
[3]: https://github.com/facebook/react-native/pull/31296 "https://github.com/facebook/react-native/issues/31296"
[4]: https://github.com/facebook/react-native/pull/31296#discussion_r616184584 "expected behaviour with prop importantForAccessibility in Button component"
[5]: https://github.com/facebook/react-native/issues/30850#issuecomment-1192286477 "importantForAccessibility=no does not allow screenreader focus on nested Text Components with accessibilityRole=link or inline Images"
[6]: https://github.com/facebook/react-native/pull/34245#issuecomment-1192446124 "testing ImageBackground importantForAccessiblity"
[7]: https://github.com/facebook/react-native/pull/34245#issuecomment-1192443589 "Button with importantForAccessibility=no"
Reviewed By: cipolleschi
Differential Revision: D38121992
Pulled By: dmitryrykun
fbshipit-source-id: 368b4dcb47d7940274820aa2e39ed5e2ca068821
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:
I've noticed that `BackHandler.removeEventListener()` performs two same `indexOf()` calls on an array that is not changing. By removing extra `indexOf` we can slightly improve time complexity of `BackHandler.removeEventListener()` from O(2n) to O(n)
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[Android] [Fixed] - Remove extra indexOf call in BackHandler.removeEventListener
Pull Request resolved: https://github.com/facebook/react-native/pull/34281
Test Plan:
1. Add the following code to any function component
```javascript
BackHandler.addEventListener('hardwareBackPress', () => true).remove();
```
2. Press on hardware back button
Expected result: Application closes
Reviewed By: dmitryrykun
Differential Revision: D38198510
Pulled By: javache
fbshipit-source-id: eab6a57689a536623138a4b3ebddbf9ba87d281f
Summary:
At the moment we are having a bug that whenever we hit a "Done" button in an input field, a value is not submitted back, so the users cannot complete their work. Here is an example:
https://pxl.cl/28xFq
To fix it, we call a textInputShouldSubmitOnReturn method on Accessory button click.
Changelog
[IOS][Fixed] - Fix keyboard accessory button not triggering onSubmitEditing
Reviewed By: dmitryrykun
Differential Revision: D38152974
fbshipit-source-id: fc026dffd641966cd3d342d95a56c43c71008036
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:
The `NSMutableAttributedString` was initialized with the same backing store as the cached attributed string on the shadow queue (background thread), but then passed to the main thread and ultimately the JS thread.
This explicitly copies the mutable attributed string into an immutable one on the shadow thread before passed off to other threads, which hopefully will address the `convertIdToFollyDynamic` crash that always includes `RCTBaseTextInputShadowView` touching an attributed string on the shadow queue in the trace.
Changelog:
[iOS][Fixed] - Possible fix for convertIdToFollyDynamic crash in RCTBaseTextInputView and RCTEventDispatcher
Reviewed By: sammy-SC
Differential Revision: D38133150
fbshipit-source-id: f371da5d17a32c3341287cd3e9730b31a98495f9
Summary:
Add capture-phase focus events to the type system, for use in the refactored VirtualizedList https://github.com/facebook/react-native/pull/32646/files
Tracking the last focused child is done via focus events. Focus events are bubbling (vs direct events like onLayout), and are given both a "capture" phase, and "bubbling phase", like DOM events on the web. https://stackoverflow.com/questions/4616694/what-is-event-bubbling-and-capturing
The VirtualizedList change wants to know if a child will receive focus. This is not possible to reliably capture in the bubbling phase, since a child may stop propagation.
See https://github.com/react-native-community/discussions-and-proposals/pull/335#discussion_r584851337 for some discussion with Scott Kyle about this issue back in the day
This is done by convention in React by adding a "capture" variant of the `onXXX` method. For all platforms I've seen with focus events, these map the `topFocus` native event to `onFocus` for bubbling phase, and `onFocusCapture` for capture phase. See https://reactjs.org/docs/events.html#supported-events
Changelog:
[General][Added] - Add types for onFocusCapture/onBlurCapture
Reviewed By: javache
Differential Revision: D38013861
fbshipit-source-id: 7bda22e1a4d5e36ac5e34e804abf6fb318a41baf
Summary:
We are already returning the `remove` function, and the `removeEventListener` method has been removed in this PR: https://github.com/facebook/react-native/issues/33580. So I believe this comment is now outdated and no longer relevant.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[Internal] [Removed] - Remove outdated TODO in AppState
Pull Request resolved: https://github.com/facebook/react-native/pull/34264
Reviewed By: dmitryrykun
Differential Revision: D38120344
Pulled By: GijsWeterings
fbshipit-source-id: c6c4d42f5b631a67bb6ad49c14df1a772f1cc71b
Summary:
A crash encountered in react-native-macOS is very similar to one fixed by https://github.com/microsoft/react-native-macos/pull/489#discussion_r451789471 (see discussion), and it's possible this `replacement` string also suffers from sharing the same backing store as the attributed string (maybe only when the range encompasses the entire string?) and therefore should be copied as well.
Changelog:
[iOS][Fixed] - Possible fix for convertIdToFollyDynamic crash in RCTBaseTextInputView and RCTEventDispatcher
Reviewed By: sammy-SC
Differential Revision: D38064551
fbshipit-source-id: 9c15f2a980155ab3cbb3fde79fcb93b24ee2091a
Summary:
This method assumes a semicolon existed before the closing bracket (`>`), but only does on iOS. This instead puts the content before the closing bracket, which is always there on both platforms.
Changelog:
[macOS][Fixed] - Fix exception thrown by [RCTTextView description] on macOS
Reviewed By: sammy-SC
Differential Revision: D38074642
fbshipit-source-id: f46d15c2bf2d966d1c1430568f083e4d501d4b40
Thuong Tran
Samuel Susla
Tianyu Yao
Luna Wei
Gijs Weterings
Andrew
Riccardo Cipolleschi
Tim Yung
Genki Kondo
Nick Gerleman
fabriziobertoglio1987
Christoph Purrer
Marshall Roch
Pieter Vanderwerff
Antoine Doubovetzky
Vitalii T
Oleh Malanchuk
Danilo Bürger
Michael Sokolnicki
