Summary:
## Changelog:
[Internal] -
I was looking at the logs, troubleshooting a non-dev build issue, and noticed a line:
```
I0429 15:56:42.107558 1874554880: Running "MyApplication
```
In `__DEV__` mode this usually continues with `" with ...`, but in release mode the closing quote was missing, which made me think there may be something going on garbling the log messages.
Which ultimately was a red herring, and it's just a bad formatting in the message in release mode, which this change fixes.
Reviewed By: zeyap
Differential Revision: D56704170
fbshipit-source-id: a28604fffec6be74733c8759f59ee52a67a81746
Summary:
Changelog: [Internal]
Fixes a crash that may happen on Android when the `inspectorEnableModernCDPRegistry` feature flag is true, and clarifies the documentation of `HostTarget::create()` to avoid similar issues in future integrations.
## Context
The executor provided to `HostTarget::create()` ("the inspector executor") is used throughout the CDP backend to schedule work on the inspector thread. (See also D53356953.) To facilitate this, the executor is a copyable `std::function`.
On Android, the executor is backed by a Java object, to which we hold a reference from C++ using JNI. This reference is expressed as a `facebook::jni::global_ref` which gets copied as part of the executor. (`global_ref` is a RAII wrapper around the `NewGlobalRef` / `DeleteGlobalRef` JNI functions.)
## The bug
All the *calls* to the inspector executor from C++ happen on threads that are already properly [attached to the JVM](https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/invocation.html#attaching_to_the_vm) ( = the UI thread / the JS thread) and are therefore allowed to make JNI calls. However, there are cases where a copy of the inspector executor may have its *destructor* run on an unexpected thread, namely the (Hermes) JS GC thread. (This happens when the executor itself is captured in a lambda that's stored in a `jsi::HostObject` or `jsi::HostFunction`, which is in turn [destroyed on the JS GC thread](https://github.com/facebook/react-native/blob/5a0ae6e2d9d7f2357f9ea6c5dc1d573233075326/packages/react-native/ReactCommon/jsi/jsi/jsi.h#L118-L126).) In such cases, the `DeleteGlobalRef` call mentioned above will crash, because the JS GC thread is not attached to the JVM.
## The fix
First, we document that copies of the inspector executor provided to `HostTarget::create` may indeed be destroyed on arbitrary threads. This is an unavoidable consequence of the existing design. Second, we adapt the Android integration to this requirement.
`fbjni` provides the [`jni::ThreadScope`](https://github.com/facebookincubator/fbjni/blob/968e3815f92aeb0670f5d88ae975fbbd47a4b482/cxx/fbjni/detail/Environment.h#L93-L123) RAII helper to manage attaching C++ threads to the JVM. If we had any explicit control over the setup and teardown of the JS GC thread, we could create a single `jni::ThreadScope` to globally ensure the safety of JS-finalizer-to-JNI calls in React Native. However, neither JSI nor the Hermes API provides such control.
Instead, we essentially resort to creating a temporary `ThreadScope` around each `DeleteGlobalRef` call where we don't control the calling thread. We do this using a new kind of JNI reference wrapper class called `SafeReleaseJniRef`. Retaining a `SafeReleaseJniRef` instead of a plain `global_ref` is all that's needed to make a particular reference safe to destroy on any thread.
Reviewed By: huntie
Differential Revision: D56620131
fbshipit-source-id: 0b6f32a7bd6477d0384af19c42e21d9242ce623d
Summary:
This gives Frameworks more control in selecting specific tasks and integrating the return types data in their UI. For example piping `stdout` to the user or using packages like [Listr2](https://www.npmjs.com/package/listr2) to run tasks in parallel and show progress.
The ordering is suggestive (but also enforced by some assertions). Frameworks are free to do what they want.
The order was implicit in the previous data structure with lists of Tasks, but made it difficult to tap into each async task.
I've also had to rework how we transpile the code if directly executed from the monorepo. This keeps our:
- flow types valid,
- allows the core-cli-utils package to be built (to generate TypeScript types and a valid npm module), and
- allows direct transpiled execution as a yarn script.
Changelog: [Internal]
Reviewed By: cipolleschi
Differential Revision: D56242487
fbshipit-source-id: a1a18f14a4aef53a98770462c8ebdef4111f0ab4
Summary:
Changelog: [Internal]
In the new CDP backend, calling any `console` method a second time involves a call to a moved-from `std::function`. This shouldn't work, and indeed results in an exception on some platforms, but isn't strictly an error according to the C++ standard: moving from a `std::function` leaves it in an [unspecified state](https://en.cppreference.com/w/cpp/utility/functional/function/function#:~:text=the%20call%20too.-,other,is%20in%20a%20valid%20but%20unspecified%20state%20right%20after%20the%20call.,-5), not necessarily an empty state, so (in particular) it's perfectly legal for the implementation to perform a copy instead of a move and leave the original variable intact.
(See [Compiler Explorer](https://godbolt.org/z/qoo5Mnd68) for proof that libc++ and libstdc++ differ on this - the former performs a copy, while the latter actually performs a move, resulting in a `std::bad_function_call` exception later.)
In the code in question, we're right to want to avoid a copy of the `body` function into the argument of `delegateExecutorSync` - only one copy of this function needs to exist at a time. But the correct way to avoid this copy is to capture `body` by reference, as we can do that repeatedly with no ill effects. (`delegateExecutorSync` is, as its name suggests, synchronous, so there are no lifetime issues.) Doing this also allows us to remove the use of `mutable` so the capturing is by *const* reference.
Reviewed By: sammy-SC
Differential Revision: D56673529
fbshipit-source-id: b235977b2fbc889462c4c78adfe41ae6f509e349
Summary:
changelog: [internal]
force_static doesn't need to be in here, let's remove it.
I change one module per diff. It makes it easier to land it and pinpoint where build failures are coming from.
Reviewed By: christophpurrer
Differential Revision: D56678530
fbshipit-source-id: c602e065d77fdd649c66ce2d26eee83428ef5ba8
Summary:
Resolved Pull request: https://github.com/facebook/react-native/pull/44296
changelog: [internal]
force_static doesn't need to be in here, let's remove it.
I change one module per diff. It makes it easier to land it and pinpoint where build failures are coming from.
Reviewed By: javache
Differential Revision: D56632286
fbshipit-source-id: e942603d3c69d9eebf4d3b64e2f73ee6a5df6de4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44295
changelog: [internal]
mapbuffer is only used on Android. Let's remove the option to have it compile on iOS.
Reviewed By: NickGerleman
Differential Revision: D56635289
fbshipit-source-id: 1a57c271d21b8aef81179d96b1a6832e7615dd27
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44283
changelog: [internal]
force_static doesn't need to be in here, let's remove it.
I change one module per diff. It makes it easier to land it and pinpoint where build failures are coming from.
Reviewed By: christophpurrer
Differential Revision: D56630625
fbshipit-source-id: 069587893dbb8866d1a08de256b4612d60bcc3b8
Summary:
changelog: [Android][Fixed] - fix a crash in Modal component
Instance variable `propertyRequiresNewDialog` in `ReactModalHostView` controls if new dialog will be created on next `showOrUpdate` or not. It must be kept in sync with `dialog` ivar.
if `dismiss` is ever called from anywhere but `showOrUpdate`, the class gets into a state where the next `showOrUpdate` call will throw an error because dialog is set to null but `propertyRequiresNewDialog` stays false.
`dismiss` is called from three places: `showOrUpdate` (this is ok), `onDropInstance()` and `onDetachedFromWindow`.
The fix in this diff is to make sure propertyRequiresNewDialog is set to true when dialog is dismissed.
Reviewed By: alanleedev
Differential Revision: D56627522
fbshipit-source-id: e7a16cd022401a7a4a0fbf8fc71a2312d05fcb8e
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44140
Why?
Previously we didn't support using percentages like:
```
style={{
width=100,
height=100,
borderRadius='100%',
}}
```
These percentages refer to the corresponding dimension of the border box.
What?
Change the unit type for `BorderRadii` values to `ValueUnit`. This type allows us to have an object containing a `float`, and a `UnitType` properties. With this we conditionally calculate the corresponding point (dp) value for a given percentage (considering size). Ex:
```
result = {raw_percentage_value} / 100 * (max(height, width))
```
We know the maximum border radius for our current implementation is half the dp of the shorter side of our view, hence why we consider half our maximum view side as equivalent to 100%.
Note: We still don't support vertical/horizontal border radii
## Changelog:
[iOS][Added] - Added support for using percentages when defining border radius related properties.
Reviewed By: NickGerleman
Differential Revision: D56198302
fbshipit-source-id: 6cd510b1c7164dcb82ca5ad8a9861c5ce5c8b15b
Summary:
Non-uniform edge insets caused issues on iOS 10. The code nowadays interferes with the rendering for large borderRadii so this diff removes it.
Since we didn't support this behavior before there are some bugs/missing features that happen:
T186810893 Incorrect border rendering with large radii
T186812303 View discoloring with overlapping border radii
T186812736 Add support for vertical and horizontal border radii
## Changelog:
[iOS][Fixed] - Removed Legacy iOS 10 code messing with border radius
Reviewed By: NickGerleman
Differential Revision: D56333637
fbshipit-source-id: 92b1bb1459d1e95476b3d768db725dfbbc1e55ae
Summary:
`flow-api-translator` can't handle `module.exports`. Shift this to ESM style exports like the other built packages.
Changelog: [Internal] - Fixing an internal build script broken by D56243647
Reviewed By: cipolleschi
Differential Revision: D56638506
fbshipit-source-id: f5a4c7bea06b7f95300388e3d37cf0d377bc3b17
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44220
This diff is part of RFC0759
https://github.com/react-native-community/discussions-and-proposals/pull/759
Here we're looking into splitting the autolinking into a component that will live inside core (specifically inside the React Native Gradle Plugin - RNGP) and another component that will live inside the Community CLI.
Here I start by adding 2 fields to RNGP extension, that frameworks and templates can use to provide their autolinking config.
Changelog:
[Internal] [Changed] - RNGP - Add autolinking fields to ReactExtensions
Reviewed By: cipolleschi
Differential Revision: D55475597
fbshipit-source-id: 316d1919a113a94c57426710f487f334c6128345
Summary:
We ran CodeQL in react-native-windows and it found a comparison of narrow type with wide type in loop condition in ReactCommon/react/renderer/core/RawPropsKeyMap.cpp
microsoft/react-native-windows#12701
## Changelog:
[INTERNAL] [SECURITY] - Fix comparison of narrow type with wide type in loop condition in RawPropsKeyMap.cpp
Pull Request resolved: https://github.com/facebook/react-native/pull/44262
Test Plan: Tested on windows.
Reviewed By: cipolleschi
Differential Revision: D56628137
Pulled By: javache
fbshipit-source-id: 9ff3bd3cbcfd084efc1e01180ff01529d1be02eb
Summary:
In addition to memoizing `mergedStyle` in `createAnimatedComponent`, this avoids unnecessary object allocations by:
* Not allocating `passthroughProps`, created via a rest spread operator. It is unnecessary because we always override `style` in the JSX.
* Not allocating a new object if either `style` or `passthroughStyle` are null or undefined. Also, create an array of the two style objects instead of spreading them, which is needless.
Changelog:
[General][Changed] - Improved performance of `Animated` components
Reviewed By: sammy-SC
Differential Revision: D56621191
fbshipit-source-id: ac863661c60d87c681284ce5ef5d6774b9c50653
Summary:
Changes two important aspects of `StyleSheet.compose`:
- Extract it from `StyleSheet` so that it can be imported from other internal modules without incurring circular dependencies. (Surprisingly, `StyleSheet` has a lot of dependencies.)
- Avoid a redundant `style1 != null` check.
Changelog:
[General][Changed] - Optimized performance of `StyleSheet.compose`
Reviewed By: sammy-SC
Differential Revision: D56621407
fbshipit-source-id: f899b50d9f13f1514485371c8513a85be78eae24
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44239
A simple CLI to build our iOS `helloworld` application. This isn't intended for an audience other that the release team.
Some Framwork authors might be interested in it as an example of how to use our react-native/core-cli-utils to build a React Native application.
Changelog: [Internal]
I'm not going to export because it's not that interesting to folks outside / in need of scrutiny.
Reviewed By: cipolleschi
Differential Revision: D56243647
fbshipit-source-id: a5f1b6d1046bda165aa7c6848938e05f0cca2dc8
Summary:
On the new architecture on Android on the new arch, `textAlign` style was ignored (`Layout.Alignment.ALIGN_NORMAL` was always used) during the measurement of text. During this phase, the positions of attachments are also calculated, which results in inline views being always positioned as if alignment to the left was set. This PR updates the measurement logic to also take `textAlign` into account during measurement.
Fixes https://github.com/facebook/react-native/issues/41008
## Changelog:
[ANDROID] [FIXED] - Fixed `textAlign` not being taken into account when positioning views inlined in text
Pull Request resolved: https://github.com/facebook/react-native/pull/44146
Test Plan:
<details>
<summary>I've been testing on the following code</summary>
```jsx
import { SafeAreaView, Text, View } from "react-native";
function InlineView(props) {
return (<View style={{margin: 10}} >
<Text style={{ textAlign: props.textAlign, backgroundColor: 'cyan' }}>
Parent Text
<Text style={{ fontWeight: 'bold' }}>Child Text</Text>
<View style={{width: 50, height: 50, backgroundColor: 'red'}} />
<Text style={{ fontWeight: 'bold' }}>Child Text</Text>
{props.long && <Text style={{ fontWeight: 'bold' }}>aaaa a aaaa aaaaaa aaa a a a aaaaa sdsds dsdSAD asd ASDasd ASDas</Text>}
</Text>
</View>)
}
export default function Test() {
return (
<SafeAreaView style={{ flex: 1 }}>
<Text style={{textAlign: 'center', fontSize: 20}}>BoringLayout</Text>
<InlineView textAlign="left" />
<InlineView textAlign="center" />
<InlineView textAlign="right" />
<InlineView textAlign="justify" />
<Text style={{textAlign: 'center', fontSize: 20}}>StaticLayout</Text>
<InlineView textAlign="left" long />
<InlineView textAlign="center" long />
<InlineView textAlign="right" long />
<InlineView textAlign="justify" long/>
</SafeAreaView>
);
}
```
</details>
| Old architecture | New architecture |
|------------------|------------------|
| <img width="447" alt="Screenshot 2024-04-18 at 17 08 59" src="https://github.com/facebook/react-native/assets/21055725/b21848ff-3939-4dde-9f78-03ce50c9429a"> | <img width="447" alt="Screenshot 2024-04-18 at 17 04 46" src="https://github.com/facebook/react-native/assets/21055725/fb57a3c4-09e8-4db7-abc3-79747314529b"> |
Reviewed By: NickGerleman, cipolleschi
Differential Revision: D56361169
Pulled By: cortinico
fbshipit-source-id: c3002f65541774e376e315c3076a6157aa330f8d
Summary:
Based on a more recent 14.x.x release of Listr.
Changelog: [Internal]
These are direct copies from `xplat/js/flow/{listr,rxjs_v6.x.x}.js`
Reviewed By: huntie
Differential Revision: D56576985
fbshipit-source-id: c850c89891bf8eb57586a5e2a50f0204fd885f65
Summary:
Any component wrapped via `createAnimatedComponent()` will always re-render, because it creates a new `style` object. It's impossible to memoize.
Adding `useMemo()` here ensures that the `style` object passed to the underlying object is stable: if no `style` is passed to the wrapped component, then memoization can work.
Allowing memoization to function when the `style` object is passed in will require a deeper fix. See https://fb.workplace.com/groups/rn.support/permalink/26084643474490921/
Before:
{F1496803038}
After:
{F1496805410}
## Changelog:
[General] [Fixed] - Fixed memoization for components wrapped with createAnimatedComponent
Differential Revision: D56618868
fbshipit-source-id: a0af8b1a02c34b5cf6e6d7e9f0381fb323b232cc
Summary:
While the class and constructor are referenced from native code, the constructor is only accidentally retained on the Java side without proper keep rules. Adding explicit DoNotStrip in the code here.
Changelog: [Internal]
Reviewed By: beicy
Differential Revision: D56529943
fbshipit-source-id: 5459b7d32ada5eeb1fabff1dfc796c2f81d3bb96
Summary:
One way we register cxx turbo modules with React Native is via cxxreactpackages.
This diff allows the application to pass in cxxreactpackages into the default react host, which allows the application to, in turn, register cxx modules with react native!
Changelog: [Android][Added] - Allow bridgeless apps to register cxx modules via cxxreactpackages
Reviewed By: cortinico
Differential Revision: D56547493
fbshipit-source-id: 4e8f02f0546c4b647a915fc65ea9687aa1592190
Summary:
We might want to publish some new versions of React Native with experimental feature to allow some partners to test whether those versions fixes some reported issues, before creating a proper stable version for the whole ecosystem.
The infra is mostly [setup for this](https://www.internalfb.com/code/fbsource/[496a64d180faab501b8598aa0ec26d47454fb961]/xplat/js/react-native-github/scripts/releases/utils/version-utils.js?lines=149), already. The only detail we need to take care of is not to move the `next` tag.
## Changelog:
[Internal]
Reviewed By: cortinico, huntie
Differential Revision: D56578456
fbshipit-source-id: 8dcc674aab5f85077c1b3e6580c5aeb99226eff8
Summary:
There are a couple scenarios where flattening the child of a ScrollView can cause problems.
1. `maintainVisibleContentPosition` on both Android and iOS rely on reading live positions in the view tree
2. `snapToAlignment` on Android uses live view tree, for items to snap to. iOS seems to have very different behavior, and aligns assuming that children are scroll view height, or that a snap interval has been set.
This change adds a prop `collapsableChildren` which can be used to disable children of scroll content view from being collapsed.
Differentiator is... complicated... but we can mostly just adapt the code dealing with existing traits at the surface level.
Changelog:
[General][Fixed] - Automatically disable flattening of scroll content view children when needed
[General][Added] - Add `collapsableChildren` prop
Reviewed By: javache
Differential Revision: D56226241
fbshipit-source-id: ed81f7fff5a15eac424708f763afc9b844aefa9c
Summary:
Flow shouldn't consider definitions inside this folder. This speeds up working in the OSS checkout if you happen to have Hermes built.
Changelog: [Internal]
Reviewed By: huntie
Differential Revision: D56575537
fbshipit-source-id: 8e5cdd0436712322a4a7298a24c721d9659d98af
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44256
## Changelog:
[iOS] [Fixed] - Preserve content offset in ScrollView when the component is suspended
# The problem
On iOS, components are recycled. For ScrollView, its content offset has to be reset back to original position. When we call `[UIScrollView setContentOffset:]`, it triggers all of its delegate methods and triggers `scrollViewDidScroll` where we set native state.
So when user scrolls to position 100 and scroll view suspends. it is removed from view hierarchy and recycled. Once the suspense boundary is resolved, scroll view will be inserted back into view hierarchy. But when it was recycled, we set back its original content offset (the default is 0, 0) but this was accidentally propagated through to shadow tree.
# Solution
To avoid this, we simply need to invalidate `_state` before calling `[UIScrollView setContentOffset:]`.
Reviewed By: cipolleschi
Differential Revision: D56573370
fbshipit-source-id: c03d7d2d403af2e1649b4cf189072baeb4c286c8
Summary:
Set the proper build flags for debugging in Bridgeless mode.
This fixes [#44240](https://github.com/facebook/react-native/issues/44240)
## Changelog:
[iOS][Fixed] - Add `HERMES_ENABLE_DEBUGGER=1` flag to React-RuntimeApple
Reviewed By: cortinico
Differential Revision: D56575647
fbshipit-source-id: a0613a5d46caeb1d3e636e54ecd43428fbaf46e8
Summary:
Defines module for `React-jsinspector` that for swift modules to integrate with.
to fix https://github.com/expo/expo/issues/28209, any podspec depends on HermesExecutorFactory should use ` add_dependency(s, "React-jsinspector", :framework_name => 'jsinspector_modern')` to add dependency. otherwise it will encounter the header not found issue because use_frameworks will change "jsinspector-modern" to "jsinspector_modern".
to depend on React-jsinspector from expo-modules-core, we need it to define as a module.
otherwise, it will have the error
```
The Swift pod `ExpoModulesCore` depends upon `React-jsinspector`, which does not define modules. To opt into those targets generating module maps (which is necessary to import them from Swift when building as static libraries), you may set `use_modular_headers!` globally in your Podfile, or specify `:modular_headers => true` for particular dependencies.
```
## Changelog:
[IOS] [CHANGED] - Add `DEFINES_MODULE` for React-jsinspector.podspec
Pull Request resolved: https://github.com/facebook/react-native/pull/44252
Test Plan: ci passed
Reviewed By: cortinico
Differential Revision: D56575102
Pulled By: cipolleschi
fbshipit-source-id: 9b7b4568a3e499f0a741a79a846263118ff2d112
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44231
changelog: [internal]
The flag is not used and is statically set to false, let's delete it.
Reviewed By: NickGerleman
Differential Revision: D56473851
fbshipit-source-id: fe1076d20a765ffed2437f080764f2b5fe060bb6
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44232
changelog: [internal]
surfaceId parameter is not needed `schedulerDidRequestPreliminaryViewAllocation` as it can be derived from shadow node.
Additionally, conversion to ShadowView can happen on the lower layers.
Reviewed By: NickGerleman
Differential Revision: D56350599
fbshipit-source-id: 9c38cc0df36911bbd6927fe0a0d5e64c248d87c4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44167
We received an issue for OSS where, when the main window is inactive and the system tries to present a dialog, the dialog is not presented in the right position on the screen.
This change introduce a fallback to the first inactive window (which is still visible on screen) and it fixes the issues.
## Changelog:
[iOS][Changed] - Fallback to the first `foregroundInactive` window when there are no `foregroundActive` windows in RCTKeyWindow
Reviewed By: dmytrorykun
Differential Revision: D56354741
fbshipit-source-id: fa23131ecd40f6d91c705879a72890506ee21486
Summary:
Inside [Re.Pack](https://github.com/callstack/repack) we consume command's options, to reduce the amount of assumptions that 3rd party tools need to make - we can move assigning default value to config command level, so default values will be aligned across tools.
For default `start` command this change doesn't change any behaviour.
## Changelog:
[INTERNAL] [CHANGED] - Add `localhost` as default host in `start` command config
Pull Request resolved: https://github.com/facebook/react-native/pull/44244
Test Plan: `start` command should work the same way as before.
Reviewed By: huntie
Differential Revision: D56567793
Pulled By: blakef
fbshipit-source-id: fe8f3686ae39a3d2996de11930a0d03364692adc
Summary:
Web props work (somewhere around D41230978 and D39268920) made it so that numeric font weights can be set instead of just strings. This is implemented by converting number to string before passing to native component within the `Text` component.
We have crash with:
```
2024-04-19 09:38:21.360 16963 17190 E ViewManager: Error while updating prop fontWeight
2024-04-19 09:38:21.360 16963 17190 E ViewManager: java.lang.IllegalArgumentException: method com.facebook.react.views.text.ReactBaseTextShadowNode.setFontWeight argument 1 has type java.lang.String, got java.lang.Double
2024-04-19 09:38:21.360 16963 17190 E ViewManager: at java.lang.reflect.Method.invoke(Native Method)
```
`TextStyleProps` can also be passed to `TextInput`, which passes to underlying native component, without going through this logic. And the types for Native props directly derive from JS props, so type system does not catch passing incorrect number type to underlying native component.
This does a quick and dirty replication of the exact logic in `Text.js` to `TextInput.js`. I'd love to potentially fix this up for Fabric in a different way when we rethink CSS parsing.
Changelog:
[General][Fixed] - Handle `fontWeight` normalization for TextInput component
Reviewed By: arushikesarwani94
Differential Revision: D56539571
fbshipit-source-id: 8975886c117d814a624f817bffe408841bb03b88
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44236
## Changelog:
[Android] [Fixed] - fix a case when view preallocation or props forwarding on Android lead to dropped props update
# What does this fix
This fixes a bug where prop change is not delivered to Android mounting layer if the prop change was initiated from state update inside of `useLayoutEffect`, `componentDidMount` or `componentDidUpdate`.
This affects android only and when batched rendering is enabled.
There are two root causes of this problem:
1. View preallocation on Android: https://fburl.com/code/r62p3vot
2. Prop forwarding on Android: https://fburl.com/code/644f1ppk
Minimal repro :
```
import React, {useLayoutEffect, useState} from 'react';
import {Button, SafeAreaView, View} from 'react-native';
function Foo() {
const [bgColor, setBgColor] = React.useState('red');
useLayoutEffect(() => {
console.log('useLayoutEffect');
setBgColor('blue');
}, []);
return (
<View
style={{
backgroundColor: bgColor,
width: '100%',
height: '100%',
}}
/>
);
}
function RNTesterApp() {
const [show, setShow] = useState(false);
return (
<SafeAreaView>
<Button title="Toggle" onPress={() => setShow(!show)} />
{show && <Foo />}
</SafeAreaView>
);
}
export default RNTesterApp;
```
# The underlaying problem
The problem is combination of view preallocation and batched rendering updates.
Here is a step by step what happens in the repro above:
1. React issues asks Fabric to create new shadow node A with background colour **red**.
2. Fabric asks Android to allocate a view for shadow node A with background colour **red**.
3. React commits tree **T1** and calls layout effects. Meanwhile Fabric waits, without trying to mount the tree **T1**, to prevent painting state that is about to be updated and prevent flickering.
4. React clones node A, changing the background colour to **blue** and commits the new tree **T2**.
5. Fabric, will now go ahead and mount the latest tree **T2**. While creating mount instructions, it will drop prop updates because it believes prop updates where delivered already as part of step 2.
# The fix
The fix is to change two things:
1. Ignore view preallocation for shadow nodes which were cloned with new props.
2. Set hasBeenMounted flag on ShadowNode later in the Fabric pipeline to fix it.
Both of these are hidden behind a single feature flag: `fixMountedFlagAndFixPreallocationClone`
## Performance implication:
I estimate that this will impact around 3% of views.
Reviewed By: rubennorte
Differential Revision: D56353589
fbshipit-source-id: 651d3cd2d0f78bfbbe9c05aa1ae1b1690c15e4ea
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44188
The current approach used for `batchRenderingUpdatesInEventLoop` is not compatible with Android due to limitations in its props processing model. The raw props changeset is passed through to Android, and must be available for the Android mounting layer to correctly apply changes.
We have some logic to merge these payloads when multiple ShadowNode clones take place but were previously assuming that a ShadowTree commit was a safe state to synchronize.
In the current implementation this means that two commits driven from layout effects (triggering states A → B → C) may cause Android to observe only the B → C props change, and miss out on any props changed in A → B.
Changelog: [Android][Fixed] Cascading renders were not mounting correctly when `batchRenderingUpdatesInEventLoop` is enabled.
Reviewed By: rubennorte
Differential Revision: D56414689
fbshipit-source-id: 7c74d81620db0f8b7bd67e640168afc795c7a1d7
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44233
The package was added to our build scripts, but shouldn't have been. We're not exporting this package or making it public.
Changelog: [Internal]
This should unblock our OSS CI.
Reviewed By: cipolleschi
Differential Revision: D56513694
fbshipit-source-id: f37c75871253b2570fb933175165d8f0a9593a16
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44197
Changelog: [General][Breaking] Remove executeAsynchronously and executeSynchronously_CAN_DEADLOCK
these are not used anywhere nor in OSS, we can delete this safely
Reviewed By: javache
Differential Revision: D56447959
fbshipit-source-id: d66c10f676946422385750c1b8825ead2d5d0ed8
Summary:
This is a copy of the current packages/react-native/template that we exclusively use internally for testing.
Changelog: [Internal]
NOTE: Best contribution would be to scan the file list and ensure there isn't anything that shouldn't be in there.
bypass-github-export-checks
Reviewed By: cortinico, cipolleschi
Differential Revision: D56242484
fbshipit-source-id: 0913ff7acff9b0314b49f48e986674b77dbb908e
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/44209
## Changelog:
[Android] [Fixed] - When React Native is destroyed, unmount all Fabric surfaces.
Previously, Fabric surfaces would not be torn down. Meaning that passive and layout effects wouldn't be unmounted and surface infra wouldn't be cleaned up.
For example:
```
useEffect(() => {
return () => {
console.log('unmounted');
}
)};
```
When calling `ReactNativeHost.clear()` on Android in native code, the above effect should be unmounted.
This is a requirement for Fabric.
Reviewed By: javache
Differential Revision: D56238947
fbshipit-source-id: 5dbf5cdef520f34c78953c2b8f2d42349549e893
