Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36177
react_native_assert calls C `assert()`, where XCode does not have a built-in breakpoint navigator to hook to assertion failures (though you can add a symbolic breakpoint to "abort()" to get the effect). This changes the Apple implemented of `react_native_assert()` to use `NSCAssert` under the hood. This is safe to use in C functions, but will be trapped by the default XCode exceptions breakpoint navigator.
Changelog:
[iOS][Fixed] - Use NSCAssert() in react_native_assert instead of C assert()
Reviewed By: cipolleschi
Differential Revision: D43275024
fbshipit-source-id: 43c4e4f1ae6b99f32634d4b1880bce712c3ae8f6
Summary:
[Changelog][Internal]
This adds a method, `emitDeviceEvent` to the C++ API of TurboModules, which allows to make calls to JS's `RCTDeviceEventEmitter.emit` from a C++ TurboModules.
This is a very common pattern, specifically for the VR apps, but not only for them - e.g. Desktop fork also has a [custom implementation for this](https://www.internalfb.com/code/fbsource/third-party/microsoft-fork-of-react-native/react-native-utils/RCTEventEmitter.cpp).
Note that my original intent was to actually backport the latter, however there are some complications with wiring things in a robust way, without exposing too much stuff and relying on singletons or folly::dynamic.
So I ended up adding it to the TurboModule API itself and use the scheduler/JSI facilities instead.
This approach is arguably well self-contained, uses high level APIs, and shouldn't be abusable much.
Since I was trying to avoid usage of folly::dynamic in this case, I used a kind of "value factory" pattern instead in order to send the arguments to the JS thread in a thread safe way (see [the discussion here](https://fb.workplace.com/groups/rn.fabric/permalink/1398711453593610/)).
Reviewed By: christophpurrer
Differential Revision: D43466326
fbshipit-source-id: a3cb8359d08a46421559edd0f854772863cb5c39
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36210
One of the circular dependencies we have in OSS was between React-Codegen and React-Fabric.
React-Codegen generates component which has to depends on React-Fabric because they need to use the files contained in the `react/renderer/view` folder.
React-Fabric contains some components that depends on RNCore, which was generated inside the React-Codegen folder.
This change generates the RNCore components inside the `ReactCommon/react/renderer/components/rncore` folder, breaking the dependency as `rncore` folder is now contained by React-Fabric itself.
**Fun Fact:** That's how it always should have been. There was already a line in the `.gitignore` to exclude the content of `ReactCommon/react/renderer/components/rncore` folder. I guess that with some of the refactoring/previous projects on Codegen, this requirements has slipped.
## Changelog:
[iOS][Breaking] - generates RNCore components inside the ReactCommon folder and create a new pod for platform-specific ImageManager classes
Reviewed By: sammy-SC, dmytrorykun
Differential Revision: D43304641
fbshipit-source-id: ebb5033ce73dbcd03f880c3e204511fdce04b816
Summary:
Update podspecs with the right search paths to include the required framework by every module.
## Changelog:
[iOS][Changed] - Update search paths to support `use_frameworks!` with Fabric
Reviewed By: sammy-SC, dmytrorykun
Differential Revision: D43089372
fbshipit-source-id: 4bbfc4b98bd289d66ce4015429d581856d9c05b3
Summary:
changelog: [internal]
Making a copy of shared_ptr is order of magnitude more expensive than moving it. This diff avoids two redundant copies in commit phase.
Reviewed By: rubennorte, cipolleschi
Differential Revision: D43306245
fbshipit-source-id: cfa942cc67b1e5c91be47803b80f7c8cda2e32d8
Summary:
changelog: [internal]
C++17 has implementation of shared_mutex in standard library. Let's use it instead of folly.
Reviewed By: cipolleschi
Differential Revision: D43275493
fbshipit-source-id: d766251226aa230110011aca94b4e697fe0d31a1
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36164
Changelog:
[General][Fixed] - Invalid prop values no longer trigger assertion failures in Fabric
## Context
Fabric has historically been very strict about prop parsing. It originally `abort()`ed on any prop value that failed to parse according to the expected type; this was replaced with dev-only assertions in D27540903 (https://github.com/facebook/react-native/commit/cb37562f8355b624e271c5b74416c257b0e62a00). We've received feedback that C++ assertions (and other similar mechanisms in the legacy renderer) are still too aggressive and disruptive as a diagnostic for developers working on JS code.
We are changing React Native to behave more like a browser in this regard, reflecting a new principle that **bad style values are not runtime errors**. (See e.g. D43159284 (https://github.com/facebook/react-native/commit/d6e9891577c81503407adaa85db8f5bf97557db0).) The recommended way for developers to ensure they are passing correct style values is to use a typechecker (TypeScript or Flow) in conjunction with E2E tests and manual spot checks.
More broadly, values passed from JS product code should not be able to crash the app, which is why we're not strictly limiting this change to style props. From now on, if a JS developer can trigger an internal assertion in React Native simply by writing normal application code, that is a bug.
## This diff
This diff introduces a new macro called `react_native_expect` which serves as a drop-in replacement for `react_native_assert`, but logs (to glog / logcat / stdout) instead of asserting. This way we don't need to fully delete the existing call sites. This will be helpful if we decide that we want to repurpose these checks for a new, more visible diagnostic.
I'm *intentionally* opting for the simplest possible improvement here, which is to silence the assertions - not to print them to the JS console, not to convert them to LogBox warnings, etc. The hypothesis is that this is already strictly an improvement over the previous behaviour, will help us get to feature parity between renderers faster, and allow us to design improved diagnostics that are consistent and helpful.
## Next steps
1. There are still places where Fabric can hit an unguarded assertion in prop conversion code (e.g. unchecked casts from `RawValue` with no fallback code path). I will fix those in a separate diff.
2. Paper on iOS needs a similar treatment as it calls `RCTLogError` liberally during prop parsing (resulting in a native redbox experience that is nearly as bad as an outright crash). I will fix that in a separate diff.
3. I'll add some manual test cases to RNTester to cover these scenarios.
4. We will eventually need to take a clear stance on PropTypes, but since they provide reasonable, non-breaking diagnostics (recoverable JS LogBox + component stack) it is less urgent to do so.
Reviewed By: sammy-SC
Differential Revision: D43184380
fbshipit-source-id: 0c921efef297d935a2ae5acc57ff23171356014b
Summary:
`setComponentDescriptorProviderRequest` is only used on iOS for dynamic registration of additional components. This variable is not default initialized, so will point to invalid memory when a component is missing and thus crash.
Changelog: [Internal] Fix Android crash when component is missing
Reviewed By: rubennorte
Differential Revision: D43305656
fbshipit-source-id: 70dd2973d50abbfcc19e051bd24c1deb90883941
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36158
To properly setup the Header Search Paths for `use_frameworks!`, we need to move the files that are in the `.../textlayoutmanager/platform/ios` folder into the `.../textlayoutmanager/platform/ios/react/renderer/textlayoutmanager` folder.
This mimic the same folder structure we have also `android`
## Changelog
[iOS][Changed] Moved the files from `.../textlayoutmanager/platform/ios` to `.../textlayoutmanager/platform/ios/react/renderer/textlayoutmanager`
Reviewed By: cortinico
Differential Revision: D43088586
fbshipit-source-id: 9589fe62f36fbff2744fdfbf3475e95954424232
Summary:
To properly setup the Header Search Paths for `use_frameworks!`, we need to move the files that are in the `.../imagemanager/platform/ios` folder into the `.../imagemanager/platform/ios/react/renderer/imagemanager` folder.
This mimic the same folder structure we have also `android`
## Changelog
[iOS][Changed] Moved the files from `.../imagemanager/platform/ios` to `.../imagemanager/platform/ios/react/renderer/imagemanager`
Reviewed By: cortinico
Differential Revision: D43088421
fbshipit-source-id: c3b86a95e67cc1ab9531997bb1bfbf011b7f730f
Summary:
To properly setup the Header Search Paths for `use_frameworks!`, we need to move the files that are in the `.../textinput/iostextinput` folder into the `.../textinput/iostextinput/react/renderer/components/iostextinput` folder.
This mimic the same folder structure we have also `android`
## Changelog
[iOS][Changed] Moved the files from `.../textinput/iostextinput` to `.../textinput/iostextinput/react/renderer/components/iostextinput`
Reviewed By: cortinico
Differential Revision: D43084639
fbshipit-source-id: fe0f55d9f372bda0fbd59c6c567ac747ca308a69
Summary:
To properly setup the Header Search Paths for `use_frameworks!`, we need to move the files that are in the `.../nativemodule/xxx/platform/ios` folder into the `.../nativemodule/xxx/platform/ios/ReactCommon` folder.
We have two folders to migrate: `core` and `sample`.
As a cleanup, `sample` should be extracted from the codebase and moved to RNTester or a library as it can be used as an example to add more examples in the codebase that we ship to our users.
This mimic the same folder structure we have also in `cxx` and `android`
## Changelog
[iOS][Changed] Moved the files from `.../nativemodule/xxx/platform/ios` to `.../nativemodule/xxx/platform/ios/ReactCommon`
Reviewed By: cortinico
Differential Revision: D43082851
fbshipit-source-id: 414426274d9a5ab20cc0e76cdada0c2977264b5f
Summary:
To Properly setup the Header Search Paths for `use_frameworks!`, we need to move the files that are in the `react/renderer/graphics/platform/ios` folder into the `react/renderer/graphics/platform/ios/react/renderer/graphics` folder.
This mimic the same folder structure we have also in `cxx` and `android`
## Changelog:
[iOS][Changed] - Moved the files from `.../platform/ios` to `.../platform/ios/react/renderer/graphics`
Reviewed By: sshic
Differential Revision: D43082032
fbshipit-source-id: 519d2e12eb7edd50b5f71bac5b1c618c6bf89919
Summary:
This removes the null-check on Yoga config added (we think we root-caused the issue), and adds an assertion to the public API accepting a config that it is non-null.
There are more changes to config setting that will come later.
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D43273456
fbshipit-source-id: cba498352d114a3fa2694f3a144f5f01a83d3190
Summary:
See code comment. D42282358 (https://github.com/facebook/react-native/commit/e55277c3310d8f6b712a57f80589cfe91ed35553) added usage of `YGConfigIsExperimentalFeatureEnabled` during layout, in a place where we sometimes encounter a Yoga node from RN which has an unexpectedly null config.
This is a hack to stop the bleed while we add logging to figure out where the null config is coming from in RN.
Changelog: [Internal]
Reviewed By: rozele
Differential Revision: D43203521
fbshipit-source-id: 2a21143a45c712ca00d16172f734fb116d165926
Summary:
changelog: [internal]
There was a bug in prop parsing where `accessibilityRole` and `accessibilityTraits` had wrong default value. Instead of falling back to sourceProps, they would set role to empty and trait to none. The fix is to fall back to sourceProps.
The issue surfaces with Animated, which clones props.
Reviewed By: cipolleschi
Differential Revision: D43190775
fbshipit-source-id: ec6b48c082c1dcc5b6e81dbff4f92b3a58a41da2
Summary:
changelog: [internal]
`useLayoutEffect` has two guarantees which React Native breaks:
- Layout metrics are ready.
- Updates triggered inside `useLayoutEffect` are applied before paint. State between first commit and update is not shown on the screen.
React Native breaks the first guarantee because it uses Background Executor. Background executor moves Yoga layout to another thread. If user core reads layout metrics in `useLayoutEffect` hook, it is a race. The information might be there, or it might not. They can even read partially update information. This diff does not affect this. We already have a way to turn off Background Executor. We haven't done this because it introduces regressions on one screen but we have a solution for that.
React Native breaks the second guarantee. After Fabric's commit phase, Fabric moves to mounting the changes right away. In this diff, we queue the mounting phase and give React a chance to change what is committed to the screen. To do that, we schedule a task with user blocking priority in `RuntimeScheduler`. React, if there is an update in `useLayoutEffect`, schedules a task in the scheduler with higher priority and stops the mounting phase.
We are not delaying mounting, this just gives React a chance to interrupt it.
Fabric commit phase may be triggered by different mechanisms. C++ state update, surface tear down, template update (not used atm), setNativeProps, to name a few. Fabric only needs to block paint if commit originates from React. Otherwise the scheduling is wrong and we will get into undefined behaviour land.
Rollout:
This change is gated behind `react_fabric:block_paint_for_use_layout_effect` and will be rolled out incrementally.
Reviewed By: javache
Differential Revision: D43083051
fbshipit-source-id: bb494cf56a11763e38dce7ba0093c4dafdd8bf43
Summary:
changelog: [internal]
Passing MountingCoordinator argument by value instead of reference. Using reference does not make sense since we eventually take ownership of shared_ptr anyway. This better communicates the intent.
Reviewed By: christophpurrer
Differential Revision: D43082955
fbshipit-source-id: 29e20abb9824c10a5f0d5e0ba1049ff6d67cee98
Summary:
Changelog: [Internal]
Caller needs to explicitly set commit options. This is for readability and making sure caller is aware of what are the options of the commit. This will be important in subsequent diff where we will add another commit option.
Reviewed By: christophpurrer
Differential Revision: D43082837
fbshipit-source-id: 1417205299c19430f902453c2b6d9bb9ca31707d
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36051
[Changelog][Internal]
This has been on my backlog for some time, submitting the diff to get it out of the way.
It makes the macro-based code in the "iterator-based property parsing" branch somewhat less horrible and less error prone (by removing duplication vs the default values in the class declaration).
Reviewed By: sammy-SC
Differential Revision: D42990595
fbshipit-source-id: e4b91160c6e09d3d1eab2ba70a58d390243bb335
Summary:
This is a prototype to add circular dependencies detection on CMake for ReactCommon and ReactAndroid.
It can be enabled per module and works as follows:
```
set(ALLOWED_HEADER_IMPORT_PATHS
react/renderer/graphics
react/debug)
check_for_circular_dependencies("${ALLOWED_HEADER_IMPORT_PATHS}")
```
The allowed header import path must be manually specified as libraries are exposing wrong header search paths (so can't be reused).
The CI will be red till the circular dependency on `graphics` is resolved.
Changelog:
[Internal] [Changed] - Add macro to detect circular dependencies on Cmake
Reviewed By: cipolleschi
Differential Revision: D42927036
fbshipit-source-id: b1393dfd43fd042e2ebf1d5b46b24bd9f5e20d58
Summary:
I discovered that 0.69 could run React Native as Dynamic framework with Hermes and starting from 0.70 that's not possible anymore.
This diff restore that possibility.
Notice that now Hermes provisdes JSI and Dynamic Frameworks requires that all the dependencies are explicitly defined, therefore, whenever we have a pod that depended on `React-jsi`, now it also has to explicitly depends on `hermes-engine`
## Changelog
[iOS][Fixed] - Add Back dynamic framework support for the Old Architecture with Hermes
Reviewed By: cortinico
Differential Revision: D42829728
fbshipit-source-id: a660e3b1e346ec6cf3ceb8771dd8bceb0dbcb13a
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/35953
DimensionValue is a reserved prop type that can be a number or string (such as '50%'). On Java, it will get converted to a YogaValue (converter added to this diff); on C++ it will get converted to a YGValue (converter already exists as it's used in Fabric).
Changelog:
[Internal][Added] - Add codegen support for DimensionValue for components
Reviewed By: cipolleschi
Differential Revision: D42650799
fbshipit-source-id: 1d2bc30bbd93837dedbbb4c74f814963c8140957
Summary:
changelog: Enable Layout Animations on iOS
[LayoutAnimations](https://reactnative.dev/docs/next/layoutanimation) in New Architecture have been disabled in OSS on iOS because of unresolved crash. This crash only happens rarely. Turning on LayoutAnimations in OSS should be safe and brings New Architecture to parity with old.
Reviewed By: fkgozali
Differential Revision: D42708774
fbshipit-source-id: b0f7febee3aa4f0ddac25556644198ebe79378c1
Summary:
Constructing a `JSError` currently catches all exceptions that occur,
regardless of their type, and embeds the information from them in the
newly created `JSError`. This means that the type of exceptions can be
silently changed.
This results in a bug if Hermes is built with
`HERMESVM_EXCEPTION_ON_OOM` enabled. The OOM exception will get
converted to a `JSError`, which in turn can get converted into a
regular JS exception. The regular JS exception can then be caught by JS
and ignored. This leads to the VM continuing to execute in a bad
state.
I considered three ways to fix this:
1. Add a new type of exception to JSI that is intentionally meant to be
ignored, and the Hermes OOM exception then subclasses it.
2. Propagate all exceptions that happen when constructing `JSError`.
3. Propagate all exceptions except `JSIException`s when constructing
`JSError`.
The first is technically the most surgical, but it adds complexity to
JSI, and will require some extra machinery to implement since we
wouldn't be able to throw it directly from inside the VM. I'm also not
comfortable with the idea of JSI suppressing exceptions that are
completely unrelated to JSI.
The second is the simplest, and seems to match what JS would do if
`throw new Error()` itself threw, but there is some possibility that it
is a change in behaviour for existing code.
So the third approach, which is implemented in this diff, tries to
compromise between the two. Hermes can only ever throw `JSIException`s
from regular JS operations, so there should be no change for existing
code (except code that uses exceptions on OOM, which is what we're
trying to fix). Any other exception gets passed through to the caller.
Changelog:
[Internal][Fixed] - Fixed handling of Hermes OOM exceptions in JSI.
Reviewed By: jpporto
Differential Revision: D41831616
fbshipit-source-id: 42e0dde1c4acc016ab19533941c58fc1797ba1c2
Summary:
This removes some unused flags which will cause Yoga to layout every tree twice, then diffing the tree, reporting whether the whole tree is different. This is too expensive to run outside of local experimentation, but we have more nuanced ways to implement the `YGNodeLayoutAffectedByQuirk` I am wanting to add.
Changelog: [Internal]
Reviewed By: lunaleaps
Differential Revision: D42406917
fbshipit-source-id: b415ed02768f6b59de3a6fa90c60c750d56fd4b0
Summary:
X-link: https://github.com/facebook/yoga/pull/1217
This updates the CMake build present for being able to share options, fixing up flags, etc. A GTest build is added as well, along with a script and VSCode debug target so that OSS contributors can very easily run and debug tests on any OS.
Note that this isn't completely done (need to revise Windows, Mac, documentation), but should be finished enough otherwise for review.
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D42406686
fbshipit-source-id: 95e7ba5e4751c496a171785490e85cf0097fa839
Summary:
On Fabric, `onContentSizeChange` of `TextInput` component was never fired on `iOS`, since the logic dispatching it was implemented in `RCTBaseTextInputShadowView` on Paper: https://github.com/facebook/react-native/blob/0f8dc067ac079f7b14696cfcafa37e3ec19a0409/Libraries/Text/TextInput/RCTBaseTextInputShadowView.m#L105. This class is not used on Fabric, therefore the event was never dispatched. On Paper, it was dispatched in `dirtyLayout` method, so I added dispatching of this event based on the change of content size in `layoutSubviews` method, since this method seems the closest one on Fabric. I am not sure if it is the best place for it though.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[IOS] [ADDED] - dispatch `onContentSizeChange` event on Fabric.
Pull Request resolved: https://github.com/facebook/react-native/pull/35816
Test Plan:
Try to use `onContentSizeChange` callback in `TextInput` component:
```tsx
import React from 'react';
import {TextInput, SafeAreaView} from 'react-native';
const App = () => {
return (
<SafeAreaView style={{flex: 1, backgroundColor: 'red'}}>
<TextInput
multiline={true}
placeholder="type here"
onContentSizeChange={e => console.log(e)}
/>
</SafeAreaView>
);
};
export default App;
```
Reviewed By: christophpurrer
Differential Revision: D42499974
Pulled By: sammy-SC
fbshipit-source-id: 3e010ff096cf91cb3e7b87ed2753e9d0e7be9650
Summary:
This is always enabled internally, so we should just turn it on everywhere.
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D42406509
fbshipit-source-id: c9cdd4fcf907d66cd276e0aec608a2e7db7ca5fb
Jeffrey Beauchamp
Nick Gerleman
Ruslan Shestopalyuk
Riccardo Cipolleschi
Samuel Susla
Joshua Selbo
Moti Zilberman
Nicola Corti
Pieter De Baets
Christoph Purrer
Rubén Norte
Stephanie Nicholson
Deepak Jacob
Janic Duplessis
Eric Rozell
Jonathan Maurice
gabrieldonadel
Genki Kondo
Neil Dhar
Ruslan Lesiutin
Wojciech Lewicki