Summary:
This is *another* attempt to solve a failed state update problem.
Unfortunately, some applications are inherently not compatible with the "let's recommit the update on the application side in case it failed" approach. The problem is that if we call `updateState` on the application side, we miss the original event window. E.g. if we need to deliver some state update with AsyncBatched priority and if the update fails, we lose the opportunity to commit it on time. These issues can be critical for some complex use-cases as ComponentKit interop.
This diff adds implementation for `updateState` that does the work a bit differently. For all failed state updates it tres to recommit them asap using `ShadowTree::commit` and calling lambda on every attempt. With this approach the update might fail in two cases:
The node disappeared from the tree, so there is no way to update it.
The lambda returned `nullptr` indicating that the update is no longer needed.
We need this for the ComponentKit interoperability layer that is very sensitive for missing state updates.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D23603958
fbshipit-source-id: a3b8c09fb2f1c8302583aa5880b48fc0840224e3
Summary:
We need it to stop repeating to commit new shadow tree in `ShadowTree::commit` when a transaction cancels the commit.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross, sammy-SC
Differential Revision: D23603959
fbshipit-source-id: d279fb3bf4190e860740a6450595d6f2fc3117f7
Summary:
Move the directory creation logic to the CLI. This needs to be processed by RNCodegen.js eventually.
Changelog: [Internal]
Reviewed By: hramos
Differential Revision: D23618550
fbshipit-source-id: e7dc4f5fc37e5ad12b5c6ae8ea31fc9ef16d74b9
Summary:
The TurboModule system requires a lookup function to map the spec name (name used in JS) to the C++ TurboModule subclass. This is a pure C function. For now, generate one lookup function per set of modules found in the codegen schema.
Changelog: [Internal]
Reviewed By: hramos
Differential Revision: D23618281
fbshipit-source-id: 889e07bdd4f2e5e93c4d14e60225f5b0c6683917
Summary:
Remove `enableFabricStartSurfaceWithLayoutMetrics` and treat as `true` always from now on.
Changelog: [Internal]
Differential Revision: D23633198
fbshipit-source-id: 5b7455b87e578ffa97d80746fa901cd2b50d3ea9
Summary:
Currently any component using `Pressability` is quickly tapped and released (i.e. responder grant followed only by reponder release), we fire `onPressIn` and `onPressOut` immediately to ensure that any visual feedback for the interactive element is momentarily visible.
Currently, we were mistakenly skipping this logic if `onPress` was null.
This fixes it so that we fire `onPressIn` and `onPressOut` consistently, even if `onPress` is null.
Changelog:
[General][Fixed] - Pressability now consistently fires `onPressIn` and `onPressOut`, even without an `onPress`.
Reviewed By: lunaleaps
Differential Revision: D23613254
fbshipit-source-id: e316707cbb2a4814262dea4eb1ddf6e3780268d1
Summary:
Adds a new `unstable_pressDelay` prop to `Pressable`.
This is intended to be used to experiment with adding a delay for when `onPressIn` fires. The intended purpose of this delay is to prevent `Pressable` elements from activating (i.e. `onPressIn` firing) if a touch gesture is immediately canceled by an enclosing native component (e.g. `ScrollView`).
Changelog:
[General][Added] - Added `unstable_pressDelay` prop to `Pressable`.
Reviewed By: lunaleaps
Differential Revision: D23604581
fbshipit-source-id: 5ce5d15a996c8a7aabaa465aa02fa8bad1044227
Summary:
Removes the default press delay from `Pressability`, which was introduced in 0.63 and affected `Pressable`. Fixes#29376.
In a subsequent commit, I will bring it back as an `unstable_pressDelay` prop.
Changelog:
[General][Changed] - Removed default 130ms delay from Pressability and Pressable.
Reviewed By: lunaleaps
Differential Revision: D23604582
fbshipit-source-id: c21c72bf8b59fed028f5905ca4f805bb3fa79399
Summary:
This pull request adds a call to `std::move` on the lambda capture in `Element::stateData`.
On Windows/Visual Studio 2017, this fixes a failure in the test `LayoutableShadowNodeTest.contentOriginOffset` where the error `std::bad_function_call` was being thrown. This was narrowed down to the callback being empty when called in `Element::stateData`.
https://github.com/facebook/react-native/blob/7e899348c74238a4a042380f86a8fe0d7e05511b/ReactCommon/react/renderer/element/Element.h#L98
Making sure the callback survives with `std::move` allows that test to pass under Windows.
## Changelog
Changelog: [Internal][Changed] - Fabric: Use std::move on callback in Element::stateData
Pull Request resolved: https://github.com/facebook/react-native/pull/29897
Test Plan: The Fabric test suite passes on Windows after this change is made. I also tested it under macOS and Linux built with Clang and they both pass with this change made.
Reviewed By: sammy-SC
Differential Revision: D23591969
Pulled By: shergin
fbshipit-source-id: e5c88bb0e94641e5128c4d49dd2f9dbfa49e9cfa
Summary:
In `StateReconciliationTest`, the way initializer lists are used to create null `ShadowNode`s causes this error on Visual Studio 2017 on Windows:
```cpp
auto result = (ShadowNode const *){nullptr};
```
---
```
StateReconciliationTest.cpp(35): error C4576: a parenthesized type followed by an init
ializer list is a non-standard explicit type conversion syntax
```
This change allows this test to compile in Visual Studio 2017, and the effected tests successfully compile and pass on Windows. They also compile and pass on Linux and macOS (both built with Clang)
## Changelog
Changelog: [Internal][Changed] - Fabric Tests: Change null ShadowNode creation in StateReconciliationTest
Pull Request resolved: https://github.com/facebook/react-native/pull/29899
Test Plan: The Fabric test suite passes on Windows after this change is made. I also tested it under macOS and Linux built with Clang and they both pass with this change made.
Reviewed By: sammy-SC
Differential Revision: D23592007
Pulled By: shergin
fbshipit-source-id: 7c6131736d478a0bf29d6c9475ef9149b7602dd6
Summary:
This pull request changes the default constructor in the `TestProps` class in `react/renderer/core/tests/TestComponent.h`
`using ViewProps::ViewProps;` was causing problems in Visual Studio 2017 - changing it to ` TestProps() = default;` allowed dependent CPP files to compile on MSVC and caused no regressions in the Fabric test suite.
## Changelog
Changelog: [Internal] [Changed] - Fabric Tests: Change default constructor in TestComponents' TestProps
Pull Request resolved: https://github.com/facebook/react-native/pull/29898
Test Plan: The Fabric test suite passes on Windows after this change is made. I also tested it under macOS and Linux built with Clang and they both pass with this change made.
Reviewed By: sammy-SC
Differential Revision: D23591986
Pulled By: shergin
fbshipit-source-id: 132e1c2e38fa74aa4f2c8746054d6152f30035e9
Summary:
RCTTurboModuleManager can be deallocated on a non-JS thread. This could create a race, when `__turboModuleProxy` is called from JS after cleanup has started. It's safer to just let `__turboModuleProxy` retain the TurboModuleManager.
In the future, we should clarify the ownership model for TurboModuleManager, so that we can explicitly delete RCTTurboModuleManager by nullifying a singular reference to it. For now, I'd like to land this change to potentially fix/narrow down the cause of T74825789.
Right now, a few objects can own RCTTurboModuleManager:
- FBReactModule, as the creator of RCTTurboModuleManager
- RCTCxxBridge, as an `id<RCTTurboModuleRegistry>` to supplement NativeModule lookup
- NativeModules that need to look up other NativeModules
- **New:** `__turboModuleProxy`
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D23608187
fbshipit-source-id: c14c8157e7670a8e32b7cd0d4c1ad5e2ab6d8abe
Summary:
This log was necessary to get to the bottom of the TurboModule Fb4a eager init crash. It's no longer necessary, plus it's okay for TurboModules to be null, so we'll remove it for now.
Changelog: [Internal]
Differential Revision: D23607208
fbshipit-source-id: 083b00abe6bdefc5986f842cd6f9438f47cce1ce
Summary:
Right now you can scroll a horizontal ScrollView with TalkBack even if you've disabled scrolling in JS, because our HorizontalScrollView component doesn't prevent the accessibility scroll event (this doesn't seem to happen with vertical ScrollViews for some reason...) This diff adds an accessibility delegate to both of the Android ScrollView components to make sure they're not scrollable if scrolling has been disabled in JS.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D23582689
fbshipit-source-id: b670bdb462ab9c963c7125597d60ca97c7d88a9c
Summary:
This pull request adds an include statement including the header `<algorithm>` to `react/renderer/graphics/Rect.h`.
It is needed for building with MSVC because of the use of the `std::max` function. It seems that this header is included as a side effect of some other header that's being used when building with Clang, and for some reason it is not included when being built with MSVC.
## Changelog
Changelog: [Internal][Added] - Fabric: Explicitly include algorithm in Rect.h
Pull Request resolved: https://github.com/facebook/react-native/pull/29884
Test Plan: Files that include this header now successfully compile on MSVC.
Reviewed By: sammy-SC
Differential Revision: D23591910
Pulled By: shergin
fbshipit-source-id: d8367bbdd94bc66c05d8fa308ed46c7aca24a68a
Summary:
With the upgrade to React Native 0.63, we started running into nullability warnings that were breaking our build. This PR fixes those nullability warnings as well as a few other warnings in React-Core.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[iOS] [Fixed] - Fix xcodebuild warnings in React-Core
Pull Request resolved: https://github.com/facebook/react-native/pull/29622
Test Plan:
- Nullability annotations should only affect compilation, but even though RNTester compiles, I'm not fully convinced that this won't break projects downstream. It would be good to get another opinion on this.
- The change in `RCTAllocateRootViewTag` is the only real logic change in this PR. We throw an exception if the root view tag is not in the correct format, so this change seems safe after some basic manual testing in RNTester.
Reviewed By: shergin
Differential Revision: D23386678
Pulled By: appden
fbshipit-source-id: a74875195a4614c3248e8f968aa98602e3ee2de0
Summary:
Right now, when two threads require two NativeModules, both threads fight for the same `std::mutex`. Why? Because NativeModule require can read from/write to the shared `std::unordered_map<std::string, TurboModuleHolder*>`.
**A Few Thoughts:**
- All threads should be able to read from the TurboModule cache concurrently, without issue. Only writes into the cache require exclusive access. *With our current setup, both reads and writes acquire exclusive access.*
- During the lifetime of an application, there will only be tens of NativeModule create events (i.e: writes into the TurboModule cache). However, there may be hundreds, if not thousands of TurboModule cache lookups. *We don't need to serialize those hundreds/thousands of TurboModule cache reads.*
This is a potential optimization opportunity for the TurboModule infra.
## Changes
This diff introduces a `std::shared_mutex` inside `RCTTurboModuleManager`. We use either the regular `std::mutex` or the `std::shared_mutex`, depending on whether `RCTTurboModuleSharedMutexInitEnabled()` is `YES`.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D23413118
fbshipit-source-id: 0880413c691b141db98a2715648f0c3e05983307
Summary:
Fix a issue that RCTModalHostView can't be dismissed while being presented
Steps To Reproduce
A native modal presented view controller is being dismissed before the first screen shows.
A RCTModalHostView is presented when the first screen shows.
The RCTModalHostView will be dismissed after an asynchronous callback.
If the callback was called before the completion of the presenting animation, the RCTModalHostView will not be dismissed.
## Changelog
[iOS] [Fixed] - Fix that RCTModalHostView can't be dismissed while being presented
Pull Request resolved: https://github.com/facebook/react-native/pull/29745
Reviewed By: shergin
Differential Revision: D23566487
Pulled By: sammy-SC
fbshipit-source-id: bd95f200b79fa75e2387e402091d58c0f538759c
Summary:
We need this to make crash logs more actionable. For now we know that those asserts fire so we should make them actionable.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D23580103
fbshipit-source-id: 8532e33b016e738a4f128c9b547c33d204610bcc
Summary:
Introduces support for `onPressIn` and `onPressOut` on the `TextInput` component.
This makes it possible to add visual feedback when users touch interact with `TextInput` components.
Changelog:
[General][Added] - TextInput now supports `onPressIn` and `onPressOut`.
Reviewed By: TheSavior
Differential Revision: D23514333
fbshipit-source-id: 1790e977b78f1c293d5476aef8613547f27d6731
Summary:
When we catch an Objective-C exception and convert it to NSError we need to somehow represent the call stack from NSException instance in NSError instance. For now, we just attach the stack trace to `message` field.
The next step would be to figure out how to pass the Objective-C stack trace to error reporting infra to help it to display the stack trace nicely in the web interface.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D23557600
fbshipit-source-id: a080c2e186e719e42dcfc01bb12f5811e3c5b2e6
Summary:
Fixes iOS builds failing with the following error in certain setups:
```
/~/Pods/React-RCTBlob/Libraries/Blob/RCTBlobPlugins.mm:14:9: fatal error: 'RCTBlobPlugins.h' file not found
#import "RCTBlobPlugins.h"
^~~~~~~~~~~~~~~~~~
1 error generated.
```
## 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
-->
[iOS] [Fixed] - "'RCTBlobPlugins.h' file not found" when building iOS
Pull Request resolved: https://github.com/facebook/react-native/pull/29684
Test Plan:
- Existing builds should not fail
- Tested this change internally where we are seeing this build failure
cc hramos
Reviewed By: fkgozali
Differential Revision: D23581090
Pulled By: appden
fbshipit-source-id: 89d2088c362bfbd22df5cd098b110a43d76e784f
Summary:
Changelog: [Internal]
In https://fburl.com/diffusion/3705cj0i we assert that view which is about to be recycled, has no superview.
This is a problem in Legacy interop layer which is nested within another interop layer. This originally wasn't considered.
Removing views in `finalizeUpdates` like it has been done until now is not enough because when a component is deleted, `finaliseUpdates` isn't called.
Reviewed By: shergin
Differential Revision: D23572999
fbshipit-source-id: f007dfe293b7d27d56253656c02529163304f83c
Summary:
Changelog: [internal]
[NSString UTF8String] is nullable. Therefore, we need to check if it isn't nil before passing it to std::string constructor which crashes if it's nil.
Reviewed By: shergin
Differential Revision: D23572652
fbshipit-source-id: 59e7f5e918b2e5c69333bfb687371f856555d8e0
Summary:
The standard says that zIndex should only be defined for non-`static` positioned views. This diff implements it.
For now, it actually enables zIndex for all views in RN because there is no way to specify `position: static` but we will give that ability by changing Flow definitions in future diffs in a couple of weeks (to ensure OTA safety).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D23559447
fbshipit-source-id: 20ea10c9349de2c5b1adea5735324a8f57150695
Summary:
Fabric Core does not support exception safety, so technically exceptions are unrecoverable and binaries should be built without exception support.
While it's not the case, some exceptions might bubble to JavaScript realm and be caught as JavaScript exceptions. In this case, we lose the stack trace. To prevent that and to investigate one particular error we have we add this try-catch block in this diff.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D23529233
fbshipit-source-id: 7ac7fb26ac08ad26af8790172de471ac178c3a37
Summary:
This argument for the `onResponderGrant` event callback on `Text` is extraneous.
Changelog:
[General][Fixed] - Remove extraneous argument for `onResponderGrant` Flow type on `Text`.
Reviewed By: TheSavior
Differential Revision: D23513190
fbshipit-source-id: c4057cf534f4cdf73967e4324db64acc8cf323d0
Summary:
The nested `.gitattributes` file in `packages/react-native-codegen/android/` caused some confusion on Linux and macOS, causing Git to show `packages/react-native-codegen/android/gradlew.bat` as modified (CRLF removed, LF added).
Instead of relying on repo-local `.gitattributes` files to convert endings in the working directory, the files should be committed to source control with the correct line endings in the first place. There is no reason to convert LF endings in .sh and many other file to CRLF on Windows (maybe this was an issue a long time ago, but unless Notepad is used this won't be a problem for practically all modern editors).
Also fixed the line endings of `scripts/launchPackager.bat` which was incorrectly committed as LF.
## Changelog
[Internal] [Fixed] - Line endings and .gitattributes
Pull Request resolved: https://github.com/facebook/react-native/pull/29792
Test Plan: Clone repo on Linux, macOS, and Windows, and make sure no modified files show up.
Reviewed By: fkgozali
Differential Revision: D23546135
Pulled By: mdvacca
fbshipit-source-id: 1572fcb959212f212b137066f1aa66f0bb6e86c3
Summary:
It looks like this config in gradle.properties caused the :packages:rn-tester:android:app:dependencies task to fail. This was reported here: https://github.com/gradle/gradle/issues/9645#issuecomment-530746758 -- it's likely related to using Kotlin build.gradle.kts?
For now comment these out.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23521954
fbshipit-source-id: 9997f4950d89e5fc57036b05db5518b39d47737b
Summary:
Replaces the `listeners()` method on `EventEmitter` with a `listenerCount()` method.
Changelog:
[General][Removed] - Removed `listeners()` from `DeviceEventEmitter` and `NativeEventEmitter`.
[General][Added] - Added `listenerCount()` to `DeviceEventEmitter` and `NativeEventEmitter`.
Reviewed By: cpojer
Differential Revision: D22204156
fbshipit-source-id: 15029525aeef55de9934a4f319910e666ecbe1d8
Summary:
The Circle CI iOS jobs are failing when installing Watchman:
```
#!/bin/bash --login -eo pipefail
HOMEBREW_NO_AUTO_UPDATE=1 brew install watchman >/dev/null
Error: The `brew link` step did not complete successfully
Exited with code exit status 1
CircleCI received exit code 1
```
The commit associated with the job where this issue first appeared did not make any changes to Watchman, but it did change the VM used by Circle CI for these jobs. Flushing the homebrew cache might help.
## 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
-->
[INTERNAL]
Pull Request resolved: https://github.com/facebook/react-native/pull/29843
Test Plan: Run on Circle CI. Watchman install step succeeded: https://circleci.com/gh/facebook/react-native/166160?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link
Reviewed By: fkgozali
Differential Revision: D23495409
Pulled By: hramos
fbshipit-source-id: 56a68bba3afc1350d4fc409d65f9229e3f38f467
Summary: Changelog: [Changed] Used scoped performance logger to mark the beginning and end of the call to the renderer, instead of using `GlobalPerformanceLogger`.
Reviewed By: lunaleaps
Differential Revision: D23240784
fbshipit-source-id: 7099f1a4b5da3f29596fcee289a715d295ae0a65
Summary:
This type makes more sense as an interface, given a class would be a common implementation (and object types aren't supported in that case).
It also adds the names of the parameters so it's easier to understand for implementers.
Changelog:
[General][Changed] - Changed type definition of IPerformanceLogger from object to interface
Reviewed By: lunaleaps
Differential Revision: D23449816
fbshipit-source-id: be872748827b123587f3f397da20f5545b0aae07
Summary:
When we render a text input that already has a text value (<TextInput value="123" />), its selection (cursor) is automatically set to the end of the text. However, when you swipe to focus the text input with TalkBack, Android decides it needs to clear the selection, which moves the cursor back to the beginning of the text input. This is probably not what you want if you're editing some text that's already there. Ideally we would just keep the selection at the end, but I don't know how to prevent this from happening - it seems to be part of how TextView handles the accessibility focus event? So instead I'm just explicitly setting the selection to the end of the text in our handler for accessibility click.
Changelog: [Android][Fixed] Move selection to the end of the text input on accessibility click
Reviewed By: mdvacca
Differential Revision: D23441077
fbshipit-source-id: 16964f5b106637e55a98c6b0ef0f0041e8e6215d
Summary:
SoLoader fixed crash on Android 4.1, and includes many improvements and fixes https://github.com/facebook/SoLoader/releases/tag/v0.9.0. Also Fresco 2.3.0 depends on it, and will create a PR soon to bump Fresco.
## Changelog
[Android] [Changed] - Bump SoLoader to 0.9.0
Pull Request resolved: https://github.com/facebook/react-native/pull/29821
Test Plan: CI is green
Reviewed By: fkgozali
Differential Revision: D23477538
Pulled By: mdvacca
fbshipit-source-id: d2d982d5c5c84fc173dc66dfe069713ca90711a8
Summary:
After monitoring scuba for a few days, previous fixes(D23301714 D23331828 (https://github.com/facebook/react-native/commit/07a597ad185c8c31ac38bdd4d022b0b880d02859)) don't work as expected.
I managed to test this issue on a Xiaomi device, the crash didn't happen but the there was a popup "Frequetly used email" on top of email edit text:
{F317216473}
Getting rid of the popup probably be the right fix.
For more context see https://github.com/facebook/react-native/issues/27204
Changelog: [Android] - Set caretHidden to true to fix the Xiaomi crash
Reviewed By: mdvacca
Differential Revision: D23451929
fbshipit-source-id: 521931422f3a46a056a9faa4b10fe93cf4732db0
Summary:
This PR tries to remove the code duplication between `RNTesterApp.ios.js` and `RNTesterApp.android.js`. The common code has been moved to `RNTesterAppShared.js`.
This PR also refactors the state architecture and simplifies it to use just one single `useReducer` hook to manage the app's state.
It also adds the visibility hack to make the screen change a little bit faster and removed the unneeded test files.
Pull Request resolved: https://github.com/facebook/react-native/pull/29777
Reviewed By: cpojer
Differential Revision: D23349302
Pulled By: rickhanlonii
fbshipit-source-id: ce0d7b03e6ef65b1655bcb744817b73c5144438b
Valentin Shergin
Ankit Tiwari
chirag-singhal
Kevin Gozali
Joshua Gross
Tim Yung
empyrical
Ramanpreet Nara
Emily Janzer
Jayme Deffenbaugh
Mi-ZAZ
Tommy Nguyen
Samuel Susla
Frieder Bluemle
Nat Mote
Héctor Ramos
Rubén Norte
Héctor Ramos
Dulmandakh
Rick Hanlon
Lulu Wu
