Summary:
Current creation of the errorProcessor lambda does a full copy of folly::dynamic object, which for large objects can cause 1000's of memory allocations, and thus increasing app's memory footprint and speed.
Changelog:
[General][Fixed] - Avoid full copy of large folly::dynamic objects in JSIExecutor#defaultTimeoutInvoker
Reviewed By: sammy-SC
Differential Revision: D38368392
fbshipit-source-id: 88579a7069891828cf6dae130c4964db6b494565
Summary:
Make the interface to enable/disable debugging symmetrical; both enabling and disabling are done by passing in a reference to the RuntimeAdapter.
Doing so requires moving ownership of the RuntimeAdapter, so each caller (java2js, arfx engine, React Native, react-native-github, and venice) has been updated to own their adapter.
Additionally, the two implementations of the Inspector connection (react-native-github and arfx engine) have been updated to expect and use the new argument.
`Connection::getRuntime` could be removed and replaced with calls to `Connection::getRuntimeAdapter::getRuntime`. I've left that choice to a followup diff, as this one is getting messy enough as it is.
Changelog: [Internal]
Reviewed By: jpporto
Differential Revision: D38242964
fbshipit-source-id: f2a3354f9d424b203a76d2c15122a6515a0926f2
Summary:
Changelog: [Internal]
Debugging is enabled via `enableDebugging` with a `RuntimeAdapter` that provides a `HermesRuntime`. This same `HermesRuntime` should be passed to `disableDebugging`, not a decorated version. Change the argument type of `disableDebugging` to steer callers in the right direction.
Reviewed By: jpporto
Differential Revision: D38051662
fbshipit-source-id: ae436a4af12fa01143bfbcdb97e65e4fca90154d
Summary:
Changelog: [Internal]
Provide a `HermesRuntime` from the `RuntimeAdapter` (instead of a decorated Hermes runtime).
As the Inspector can now directly access the `HermesRuntime` (which exposes access to the debugger), `RuntimeAdapter::getDebugger` is no longer necessary; remove it.
Reviewed By: jpporto
Differential Revision: D38050821
fbshipit-source-id: fa83165a9348bdff0dce1b04ec1afb81d2b1c3e2
Summary:
Changelog: [Internal]
Make runtime use more consistent, always using the same undecorated runtime. This prevents the previous mixing of decorated and undecorated runtimes.
Reviewed By: jpporto
Differential Revision: D38035166
fbshipit-source-id: 81929e42ccc4de6d5c5c09ee7ee31f055af82735
Summary: Changelog: [Internal] Add logic for `isPrimary` for PointerEvent object and set other properties to their default value
Reviewed By: javache
Differential Revision: D38363163
fbshipit-source-id: 5ec9de69fb5b34295f1da6daedd5c67e3bd3727e
Summary:
Changelog: [Internal] - Instead of using toolType which is a property per pointer in the MotionEvent, let's use [getSource](https://developer.android.com/reference/android/view/MotionEvent#getSource()) which is the source for the entire event (all pointers).
This aligns with what we've seen on Android when we have a mouse and touch input, there is only one active source input device.
And removes the need for checking a flag we set here: D36958947
Reviewed By: NickGerleman
Differential Revision: D37702090
fbshipit-source-id: ba2a4f0c28e1aff2b8b04314fe6f737b66ed0be3
Summary:
Expo Go was using `nativeInjectHMRUpdate` for [snack](https://snack.expo.io/) to provide stack traces to OTA errors. `nativeInjectHMRUpdate` was replaced with `globalEvalWithSourceUrl` [here](71c84cf6be). The issue with `globalEvalWithSourceUrl` is that it is stripped in RELEASE builds, and Expo Go is installed via the App Store, making it a release build.
I propose we keep the method, `eval` is still exposed in production so there's no increase in security risk, simply provides a better DX for multipurpose development clients.
## 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] [Added] - Expose `globalEvalWithSourceUrl` in production builds.
[iOS] [Added] - Expose `globalEvalWithSourceUrl` in production builds.
Pull Request resolved: https://github.com/facebook/react-native/pull/34319
Test Plan: - Function should be available as `global.globalEvalWithSourceUrl` in a production build.
Reviewed By: christophpurrer
Differential Revision: D38312111
Pulled By: motiz88
fbshipit-source-id: adcd83ad1103c8fd4c6d5d7aec765f27881e0432
Summary:
Resolves https://github.com/facebook/react-native/issues/34116.
In a nutshell, the problem was a missing `.exe` extension on the `hermesc` binary path when running on Windows OS. The missing extension causes the method `.exists()` of the File instance to always return false, so none of the conditions ever met and an error was thrown whenever a release build with Hermes enabled was run on Windows. More details can be found in the comments on the above issues.
## 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
-->
[Android] [Fixed] - Fix error of release builds with Hermes enabled for Windows users
Pull Request resolved: https://github.com/facebook/react-native/pull/34151
Test Plan:
### Reproduce
Changes on Gradle scrips are better tested on an actual application. To reproduce the issue you can:
1. Create or reuse a React Native application with version `v0.69.1` on a Windows machine
2. Enable Hermes on Android following the steps on the [documentation](https://reactnative.dev/docs/hermes#enabling-hermes)
3. Clean the build folder: `cd android && ./gradlew clean`
4. Bundle the JS and assets for a release version: `./gradlew bundleReleaseJsAndAssets`
5. The build fails with the following error:
```shell
Execution failed for task ':app:bundleReleaseJsAndAssets'.
> java.lang.Exception: Couldn't determine Hermesc location. Please set `project.ext.react.hermesCommand` to the path of the hermesc binary file. node_modules/react-native/sdks/hermesc/%OS-BIN%/hermesc
```
### Test the changes
Follow the same steps above using the fix on this PR and the error should disappear 🙂
Reviewed By: NickGerleman
Differential Revision: D37755468
Pulled By: cortinico
fbshipit-source-id: 2ad0ced583555b907259df116f64a45da6d153f3
Summary:
This Pull Request aims at removing the making of reactnativeutilsjni as it is built from the same sources as reactnativejni. It also replaces references to reactnativeutilsjni with reactnativejni.
This should get rid of `reactnativeutilsjni.so` while reusing `reactnativejni.so` in it's place. This should give us some size improvements in the finally built apk.
## 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] [Changed] - Replaced reactnativeutilsjni with reactnativejni in the build process to reduce size
Pull Request resolved: https://github.com/facebook/react-native/pull/34339
Test Plan:
1. Ran the CMakelist.txt file using CMake and I could see that reactnativeutilsjni.dir is no longer generated with my changes.
2. Built the aar from this branch in Android Studio and build happened successfully.
I am not sure if we could run any more tests. Please let me know in case anymore testing is required and I can do accordingly
Reviewed By: cortinico
Differential Revision: D38400481
Pulled By: genkikondo
fbshipit-source-id: 592736e56441328389ae89135667c336ff8018e6
Summary:
This PR fixes an edge case where `prepare_hermes_workspace` job was using a commit to build hermes but `build_hermes_macos` was using a different one. This resulted in cache poisoning where subsequent jobs thoughts to be using a version of Hermes while the restored cache was loading a different one.
<img width="1440" alt="Screenshot 2022-08-03 at 06 26 14" src="https://user-images.githubusercontent.com/11162307/182570809-5c6d9323-c3fb-4834-952f-7d07b99c4880.png">
This PR simplifies the flow, creating a single `.hermesversion` file in the `prepare_hermes_workspace` workspace and using that file as key for all the caches.
## Changelog
[iOS] [Changed] - upload test result as artifact
Pull Request resolved: https://github.com/facebook/react-native/pull/34329
Test Plan:
CircleCI is now green and all the caches are using the same file to create the checksum.
We can verify that by looking at the `Save cache`/`Restore cache` commands related to Hermes. (In the workflow, their hash is always `B1NEL0P0OKhQYtk8DE150bXSoGrdWUweedHKmqNqnjo`)
Also, we removed completely the code that could create a version misalignment.
Reviewed By: cortinico
Differential Revision: D38382895
Pulled By: cipolleschi
fbshipit-source-id: 5f5501a7ef313eb56abda336716b24b486a34a1f
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:
Changelog:
[Android][Fixed] - Fix regression when setting shadow node properties.
Also simplified the corresponding macros to avoid using lambdas altogether, as they are not required.
Note that this **does not** modify any constexpr-related semantics of the existing code, as the main constexpr macro, `CONSTEXPR_RAW_PROPS_KEY_HASH` evaluation result is still contstexpr value, and the other ones already involved non-const parts (see my comments).
Reviewed By: NickGerleman
Differential Revision: D38356411
fbshipit-source-id: 22c330d3425c8aed36693f4652f1b257d2dc96be
Summary:
Some custom logic is applied to workaround a platform bug where velocity may be incorrect on Android P. [The bug in question](https://issuetracker.google.com/issues/112385925) appears to have been fixed before Android `Q` was released, so we shouldn't *need* to apply the workaround on other versions.
As described in https://github.com/facebook/react-native/issues/34226 the workaround can adversely affect certain scroll behaviors, which can easily be reproduced when you briefly scroll one direction then quickly fling the opposite direction (see the video in the linked ticket).
This PR changes the workaround to *only* be applied on Android P, in order to avoid causing weird scroll behavior on versions that are not actually affected by the bug the workaround is working around.
## Changelog
```
[Android] [Fixed] - Fix occasionally incorrect ScrollView fling behavior
```
Pull Request resolved: https://github.com/facebook/react-native/pull/34233
Test Plan:
- Repro the strange fling behavior in the current version (See video attached in https://github.com/facebook/react-native/issues/34226)
- Verify that the string fling behavior is fixed with this patch
- Verify that fling behavior still works as expected on Android versions affected by the [original bug](https://issuetracker.google.com/issues/112385925), and those immediately following it (to verify that the bug being worked around was, in fact, fixed as expected).
Reviewed By: javache
Differential Revision: D38287277
Pulled By: ryancat
fbshipit-source-id: 2c786872c4d41655b3849bb92e02f1f16c663b41
Summary:
This PR runs eslint in PR using Danger.
## Changelog
[General] [Changed] - Run ESLint in CI
Pull Request resolved: https://github.com/facebook/react-native/pull/34305
Test Plan:
1. Add a JS lint error in the package/react-native-codegen package.
2. Observe the CI task post a message with the errors
3. Fix the errors
4. Observe the CI task report completion with no errors
Reviewed By: cortinico
Differential Revision: D38315268
Pulled By: cipolleschi
fbshipit-source-id: 9984402ee427dd62d47dd716f73c030e6d0f7b5e
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 reverting D38280674 (a561df0e96)
D38280674 (a561df0e96) has been identified to be causing the following test or build failures:
Tests affected:
- https://www.internalfb.com/intern/test/562950020230233/
Here's the Multisect link:
https://www.internalfb.com/intern/testinfra/multisect/1102425
Here are the tasks that are relevant to this breakage:
T93118125: [7 CRITICAL TEST FAILURES] 12 tests started failing for oncall marketplace_feed_experience in the last 2 weeks
We're generating a revert to back out the changes in this diff, please note the backout may land if someone accepts it.
---
Changelog:
[General][Changed] - Reverting "[react-native] Exit early if currentShadowNode cannot be casted" due to test failures
Differential Revision: D38338773
fbshipit-source-id: 0e0c11b1f5e27d98fdcfab34847a2fd0750ff5af
Summary:
Fix macro errors for Windows. Current syntax breaks the build of the React Common project on Windows because the ({...}) syntax is not supported; must be replaced with lambda expressions.
Resolves https://github.com/facebook/react-native/issues/34090
## 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
-->
[General] [Fixed] - Fix macro errors for Windows.
lyahdav JoshuaGross
Pull Request resolved: https://github.com/facebook/react-native/pull/34299
Test Plan: Build on react-native-windows repo. Tested in RNW app.
Reviewed By: javache
Differential Revision: D38272966
Pulled By: NickGerleman
fbshipit-source-id: e76eac11cde173ef49465d01d793c593017f2ab7
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:
This diff fixes the bug T127619309 by exit early during calculateTransformedFrames if currentShadowNode cannot be casted
This is a bug that fired in fb4a but we didn't have a way to reproduce locally.
We are going to release this and enable feature flag with a MC
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D38280674
fbshipit-source-id: 1c42c17678d8473564e4075a78d3c688efed1a23
Summary:
While doing some testing with cipolleschi on React Native 0.70.0 we realized that custom Java Package is not properly propagated when invoking codegen for external libraries.
This PR fixes it.
## Changelog
[Internal] - Introduce findPackageJsonFile to address issue with GenerateCodegenArtifactsTaskTest
Pull Request resolved: https://github.com/facebook/react-native/pull/34321
Test Plan: Added JUnit tests + Tested locally with cipolleschi
Reviewed By: cipolleschi
Differential Revision: D38314770
Pulled By: cortinico
fbshipit-source-id: ec2de1ba59ffc8fb0644f422521ced642b38d2c7
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/34307
There are unreleased changes on eslint-config-react-native-community, I'm bumping the version for this package so we can publish it.
Changelog:
[General] [Changed] - Bump eslint-config-react-native-community
Reviewed By: dmitryrykun
Differential Revision: D38279512
fbshipit-source-id: 46492f7e99d31c0a5917d41726ecc20fada1582f
Summary:
Calculation of TransformedFrames was a method introduced by D37994809 (64528e5faa) to fix rendering of inverted flat lists.
We found that this operation is crashing in fb4a causing the UBN T127619309
This diff creates a feature flag to disable the calculation of TransformedFrames in Layoutable ShadowNodes. **The goal of this diff is to revert the behavior introduced by D37994809 (64528e5faa)**
The featureFlag is disabled in fb4a and enabled in react AR (because ReactAr apps relies on the calculation of TransformedFrames and these apps are not affected)
The root cause of the bug will be fixed by D38280674 (which still requires more testing and investigation)
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D38286857
fbshipit-source-id: 721cd0554ae6a6b369b3f8dbb584160a270d0f18
Summary:
This change adds the following methods to jsi::BigInt
* [static] fromInt64(value): creates a jsi::BigInt from a signed 64-bit value
* [static] fromUint64(value): creates a jsi::BigInt from an unsigned 64-bit value
* [static] strictEquals(a, b): return a === b, a and b are BigInts
* asInt64(): truncates the BigInt to a single signed 64-bit integer; throws a JSIException if the truncation is lossy.
* getInt64(): truncates the BigInt to a single signed 64-bit integer
* isInt64(): returns true if the BigInt can be truncated losslessly to an int64_t
* asUint64(): truncates the BigInt to a single unsigned 64-bit integer; throws a JSIException if the truncation is lossy.
* getUint64(): truncates the BigInt to a single unsigned 64-bit integer
* isUint64(): returns true if the BigInt can be truncated losslessly to an uint64_t
* toString(radix): converts the BigInt to a string representing the number in the given radix.
A lossy truncation is one that yields a result from which the BigInt cannot be reconstructed from, i.e.,
* BigInt::fromInt64(b.toInt64()) !== b
* BigInt::fromUint64(b.toUint64()) !== b
Changelog: [Internal]
Reviewed By: kodafb
Differential Revision: D37909139
fbshipit-source-id: 172848024f8367aed73cc602f38cde22f03cac8f
Summary:
The `codegenConfig` unified configuration for New Architecture on Android relies on the Gradle plugin finding the package.json correctly. Currently we use the `root` folder to resolve the package.json. This works fine when invoking the codegen for the app module, but it doesn't work well when invoking the codegen for modules.
This extends the algo to make sure we first, look for a `package.json` in `..` and fallback to the one in the root if not found.
## Changelog
[Internal] - Improve package.json search mechanism for codegenConfig support
Pull Request resolved: https://github.com/facebook/react-native/pull/34298
Test Plan: It's hard to write a unit test for this as it's inside a lambda, I'll look into doing this though. I've tested this on RNNewArchitectureApp and it works fine.
Reviewed By: cipolleschi
Differential Revision: D38249663
Pulled By: cortinico
fbshipit-source-id: 3cfd6a31e9f75d7b19b15f77bbd5131af42be9d3
Summary:
While I was working on rewriting `react-native-slider` to Fabric I found a weird bug that prevented the slider to be set as disabled (to be exact: call the method `slider.setEnabled(false)`. As it turned out the `accessibilityState` (with value: `accessibilityState={{disabled: true}}` prop occurred after the `enabled={false}` prop that I was passing to the slider, which lead to both of this props overwrite each other.
Handling of `accessibilityState` props inside view leads to always overwriting the enabled prop to true (even if we explicitly set it to `{disabled: false}`.
Workaround for this was to reorder the props, so that the `accesibilityState` occur before `disabled`, but I think it's better to not set `view.setEnabled(true)` if we are passing a disabled property.
## Changelog
[Android] [Fixed] - Fix accessibilityState overwriting view's disabled state on Android
Pull Request resolved: https://github.com/facebook/react-native/pull/34287
Test Plan:
Change order of props inside native component implementation (that `disabled` occurs before `accesibilityState`). For example: `Libraries/Components/Slider/Slider.js`
<details>
<summary>Video showing the bug in RNTester (using Switch component)</summary>
https://user-images.githubusercontent.com/52801365/181287547-964f50e2-55dc-450f-b413-0d1c14d4bb83.mp4
</details>
Reviewed By: NickGerleman
Differential Revision: D38209232
Pulled By: dmitryrykun
fbshipit-source-id: 93d423716f89b45251be9d5aefcf01f7bd776f2c
Summary: Changelog: [General][Changed] - `eslint-plugin-specs` package has prepack hook that changes `PACKAGE_USAGE` variable of `react-native-modules.js` to `true`. This changed file is can then be published to NPM, but should not be committed to the repo. This diff adds postpack hook that reverts the change, so we do not have to do it manually.
Reviewed By: cipolleschi
Differential Revision: D38244367
fbshipit-source-id: 818dbdea82e7e4b89094b3e27ae2d63b9e736659
Summary:
I've realized that the gradle plugin is currently looking at `.js` files for task re-execution. This means that, while the *.ts would still be considered when the codegen is invoked, an edit on one of those file, won't retrigger the codegen on Android.
This change fixes it so that we consider both `*.ts` and `*.js` files.
## Changelog
[Android] [Fixed] - Make sure *.ts files are considered for task avoidance in the Gradle Plugin
Pull Request resolved: https://github.com/facebook/react-native/pull/34296
Test Plan: Tests are attached.
Reviewed By: cipolleschi
Differential Revision: D38246125
Pulled By: cortinico
fbshipit-source-id: 80efcc9ef747c598ca040d65b25d270593c8aed2
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:
Fixes https://github.com/facebook/react-native/issues/34120
The new React Native architecture doesn't check `needsCustomLayoutForChildren` so it wrongly positions native views on Android. In https://github.com/facebook/react-native/issues/34120 there are videos comparing the positioning of a native action view in the old and the new architecture.
This PR passes the parent tag to the `updateLayout` method of the `SurfaceMountingManager`. The `SurfaceMountingManager` calls `needsCustomLayoutForChildren` on the parent view manager (copied the code from the `NativeViewHierarchyManager` in the old architecture).
**NOTE** - I wasn't sure where to get the parent shadow view from so I've put in my best guesses where I could and left it as `{}` otherwise.
## Changelog
[Android] [Fixed] - Migrate `needsCustomLayoutForChildren` check to the new architecture
Pull Request resolved: https://github.com/facebook/react-native/pull/34254
Test Plan:
I checked the fix in the repro from https://github.com/facebook/react-native/issues/34165. Here is a video of the action view closing using the native button that is now visible in the new architecture.
https://user-images.githubusercontent.com/1761227/180607896-35bf477f-4552-4b8a-8e09-9e8c49122c0c.mov
Reviewed By: cipolleschi
Differential Revision: D38153924
Pulled By: javache
fbshipit-source-id: e2c77fa70d725a33ce73fe4a615f6d884312580c
Christoph Purrer
Matt Blagden
Luna Wei
evanbacon
JoseLion
Sparsha Saha
Marshall Roch
Pieter Vanderwerff
Riccardo Cipolleschi
fabriziobertoglio1987
Antoine Doubovetzky
Ruslan Shestopalyuk
Daniel Leong
Nick Gerleman
Carmi Grushko
Dark Knight
Chiara Mooney
David Vacca
Michał Pierzchała
Nicola Corti
LeoTM
John Porto
Alex Hunt
Oskar Kwaśniewski
Dmitry Rykun
Graham Mendick
