Summary:
Changelog: [internal]
# Problem
Default padding for TextEdit is top: 26, bottom: 29, left: 10, right: 10 however the default values for LayoutMetrics.contentInsets is {0, 0, 0, 0}.
If you try to construct TextEdit with padding 0, Fabric will drop padding update because padding is already 0, 0, 0, 0.
# Fix
To fix this, I added a special case to `Binding::createUpdatePaddingMountItem`, if the mutation is insert, proceed with updating padding.
Reviewed By: JoshuaGross
Differential Revision: D23731498
fbshipit-source-id: 294ab053e562c05aadf6e743fb6bf12285d50307
Summary:
An experiment where we use `LayoutMetrics::overflowInset` to optimize performance of hit-testing algorithm.
In my measurements enabling this feature drastically (2x) reduced amount of calls to `hitTest:withEvent:` method.
Reviewed By: sammy-SC
Differential Revision: D23669091
fbshipit-source-id: ac6dadc90b6a2fbb45e41e0d4a113bd5ae8f1218
Summary:
Right now nested Text components are not accessible on Android. This is because we only create a native ReactTextView for the parent component; the styling and touch handling for the child component are handled using spans. In order for TalkBack to announce the link, we need to linkify the text using a ClickableSpan.
This diff adds ReactClickableSpan, which TextLayoutManager uses to linkify a span of text when its corresponding React component has `accessibilityRole="link"`. For example:
<Text>
A paragraph with some
<Text accessible={true} accessibilityRole="link" onPress={onPress} onClick={onClick}>links</Text>
surrounded by other text.
</Text>
With this diff, the child Text component will be announced by TalkBack ('links available') and exposed as an option in the context menu. Clicking on the link in the context menu fires the Text component's onClick, which we're explicitly forwarding to onPress in Text.js (for now - ideally this would probably use a separate event, but that would involve wiring it up in the renderer as well).
ReactClickableSpan also applies text color from React if it exists; this is to override the default Android link styling (teal + underline).
Changelog: [Android][Fixed] Make nested Text components accessible as links
Reviewed By: yungsters, mdvacca
Differential Revision: D23553222
fbshipit-source-id: a962b2833d73ec81047e86cfb41846513c486d87
Summary:
Since Prettier 2.0, the default for the `arrowParens` option has changed from `avoid` to `always`.
In preparation for upgrading Prettier in Xplat to 2.1, add `arrowParens: 'avoid'` explicitly to configurations that were missing it so that we do not have a change of behaviour when upgrading.
Changelog: [Internal]
Reviewed By: bolinfest
Differential Revision: D23666397
fbshipit-source-id: 665f20d1419d0d664a6ef5cddc46f441b20eeb51
Summary:
Disabling Fast Refresh won't be persisted when restarting the app.
I believe this regressed with 824e171117. In this commit `HMRClient.setup` will always be called with `enabled` being set true. This PR loads the _enabled_ state from the user defaults (as it was done before).
## Changelog
[iOS] [Fixed] - Enable Fast Refresh gets persisted across app launches
Pull Request resolved: https://github.com/facebook/react-native/pull/29880
Test Plan:
Without this change Fast Refresh is enabled on every app restart.
Tested with this change:
- Disabling Fast Refresh
- Restarting App -> Fast Refresh still disabled
- Enabled Fast Refresh again
- Restarting App -> Fast Refresh still enabled again
Reviewed By: sammy-SC
Differential Revision: D23686435
Pulled By: PeteTheHeat
fbshipit-source-id: 42c31b22060d3dc4b1d4cb8f41792b303fc7fce8
Summary:
Fix a typo in the invariant message presented when ActionSheetManager does not exist (typo was found in 2 places)
## Changelog
[General] [Fixed] - Fix typo in invariant message
Pull Request resolved: https://github.com/facebook/react-native/pull/29913
Test Plan: Run ActionSheetIOS.showActionSheetWithOptions(...) on an Android emulator to get this invariant message to appear.
Reviewed By: hramos
Differential Revision: D23691048
Pulled By: appden
fbshipit-source-id: 469db65a42bf547c7818b45729befa214b5791f2
Summary:
It's more correct and we will rely on this in the future.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D23681508
fbshipit-source-id: 5fef0ea164e8a400e6ca9a67947252b47ce6d44e
Summary:
This diff allows specifying the preemptive compilation threshold via CompileFlags.
Users get a toggle in CLI and RuntimeConfig that allows choosing between 1. fully eager, 2. fully lazy, 3. use default thresholds ("smart"). The default is #3.
However, since we use `-O` by default, it overrides any lazy compilation when
using the `hermes` binary.
ChangeLog: [Internal] Inspector updated to use new RuntimeConfig laziness control
Reviewed By: tmikov
Differential Revision: D23356463
fbshipit-source-id: 508b7b2e6a218346c69acfec97e7891e388f0e9b
Summary:
This pull request tweaks the build script for `jsiruntime` to ensure that `RN_FABRIC_ENABLED` is defined when building for Android with Fabric enabled, and using the JavascriptCore JS engine. Without it, the `createWeakObject` and `lockWeakObject` methods in `JSCRuntime.cpp` will throw an exception.
## Changelog
[Internal] [Changed] - Android: Ensure RN_FABRIC_ENABLED is defined when building with Fabric
Pull Request resolved: https://github.com/facebook/react-native/pull/29939
Test Plan: RNTester with JSC now builds and runs on Android with Fabric enabled.
Reviewed By: hramos
Differential Revision: D23671070
Pulled By: fkgozali
fbshipit-source-id: 27e69364a9f709615543be7c20b72b5536443cd8
Summary:
This fixes TextInput measurement caching. Previously we were not setting the correct Spannable in the cache; we needed to additional Spans to it to indicate font size and other attributes.
This brings Fabric closer to how non-Fabric was measuring Spannables for TextInputs (see ReactTextInputShadowNode.java).
This should fix a few crashes and will be most noticeable with dynamically-sized multiline textinputs where the number of lines changes over time.
This also allows us to transmit less data from C++ to Java in the majority of cases.
Changelog: [Internal]
Differential Revision: D23670779
fbshipit-source-id: cf9b8c848b9e0c2619e01766b72b074248466825
Summary:
Every time we measure a TextInput we allocate a JNI local array and weren't cleaning it up, leading to JNI table exhaustion.
Changelog: [Internal]
Differential Revision: D23670780
fbshipit-source-id: 2ecf9770c8593eeadd70a248be58037fefdca61e
Summary:
Changelog: [Internal]
There was a key mismatch between what Java and C++.
cacheIdMap was incorrectly initialised.
Differential Revision: D23652342
fbshipit-source-id: 66f54dc887a011afeead9420bda093e9a0c9a8ca
Summary:
One of the operations we do in `Scheduler::stopSurface()` is committing an empty tree to free up `ShadowNode` objects and "disable" `EventEmitter`s associated with them. Before this change, we had a gap in time between a moment when we commit an empty tree and remove the tree from the registry. During this time gap, JavaScript (or native, actually) can commit another tree and mount another new state on the screen. To prevent this, we remove the tree from the registry first and only then commit an empty tree to a uniquely owned tree.
Note that the deleted comment says that we actually have to have a tree in the registry for committing an empty tree, I don't think it's true now.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D23667882
fbshipit-source-id: 387052e9f3e78e7d4446f36baed50f9caa831133
Summary:
When `TouchableHighlight` was migrated to use `Pressability`, a bug was introduced due to `onLongPress` being unconditionally supplied as a callback. This bug leads to `onPress` not firing if the element is pressed for longer than 500ms, even when `onLongPress` is not supplied.
Closes#29040.
Changelog:
[General][Fixed] - TouchableHighlight fires `onPress` when pressed for >500ms, when `onLongPress` is not supplied.
Reviewed By: TheSavior
Differential Revision: D23664365
fbshipit-source-id: 3a0e92e276871eedd303888346a13433be15ac47
Summary:
Changelog: [Internal] Log image-requested QPL point for Fabric images
* See D23450649 for the full lifecycle
Reviewed By: fkgozali
Differential Revision: D23448179
fbshipit-source-id: 0a78dae2d4f1e6322bbeee3574b10abe1efb30ef
Summary: Changelog: [Internal] Add IGviewpoint to get image visibility callbacks for when an UIImageView is in or out of view
Reviewed By: fkgozali
Differential Revision: D23428528
fbshipit-source-id: 87e4cee8fbe3c6b7da5153f87bbb530b2f990d96
Summary:
* Create `ImageTelemetry` as a property of ImageRequest to pass image metadata from the ImageShadowNode into RCTImageComponentView
* ImageRequest is a object in the ImageShadowNode::ConcreteState
Reviewed By: fkgozali
Differential Revision: D23239664
fbshipit-source-id: db0f08bb889af8ce63fbe1abe88744edddb86150
Summary:
This function replaces the usage of an Objective-C `#import` statement with `#include` in `react/renderer/scheduler/SynchronousEventBeat.cpp`.
Clang seems to have no problems with `#import` being used in CPP files, but it was causing errors in MSVC. ([it has a different purpose](https://docs.microsoft.com/en-us/cpp/preprocessor/hash-import-directive-cpp?view=vs-2019) on Windows)
## Changelog
Changelog: [Internal][Changed] - Fabric: Replace #import statement in SynchronousEventBeat
Pull Request resolved: https://github.com/facebook/react-native/pull/29885
Test Plan: This CPP file now compiles in MSVC
Reviewed By: sammy-SC
Differential Revision: D23591933
Pulled By: shergin
fbshipit-source-id: ee2cb8152f37e5ee09500af621a57b59d5a18400
Summary:
Changelog: [internal]
# Problem
onTextLayout was called even if there was no change to text layout. This can cause an infinite loop with onTextLayout triggering commit which triggers onTextLayout.
# Fix
Do not call onTextLayout if there is nothing to layout.
Reviewed By: yungsters
Differential Revision: D23648430
fbshipit-source-id: 055dc34a9aca0edf2c78a5812b35b80df32c9e3e
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`.
7e899348c7/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
Samuel Susla
Valentin Shergin
Emily Janzer
George Zahariev
Ullrich Schäfer
S. Weatherall
Will Holen
empyrical
Joshua Gross
Tim Yung
Paige Sun
Ankit Tiwari
chirag-singhal
Kevin Gozali
Ramanpreet Nara
Jayme Deffenbaugh
