Summary:
X-link: https://github.com/facebook/yoga/pull/1384
Pull Request resolved: https://github.com/facebook/react-native/pull/39446
This adds logic to the enum generator to generate C++ style scoped enums.
This gives us a few nicities over C enums, even if both must exist:
1. We can add types and keep unsgined enums directly in bitfields
2. Style/readability
3. Avoiding implicit int conversion
Reviewed By: rozele
Differential Revision: D49267996
fbshipit-source-id: 1c41164c377b317c1fef97811c46cbc00b5a837e
Summary:
X-link: https://github.com/facebook/yoga/pull/1380
Pull Request resolved: https://github.com/facebook/react-native/pull/39433
Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified.
During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element.
For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is usually never noticed, because we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized.
There was at least one screenshot test where this issue showed up though, and I was able to add a slightly different repro where we may have free space without a definite dimension by enforcing a min dimension and not stretching.
{F1091401183}
The new reference is correct, and looking back at diffs, is what this seemed to originally look like when added three years ago. Seems like there may have been a potential regression, but I didn't spot anything suspicious when I looked around the code history.
`betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size.
Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_
See the original conversation on https://github.com/facebook/yoga/pull/1188
Reviewed By: javache
Differential Revision: D49260049
fbshipit-source-id: 218552c5ff938668b9f257df7a1493e13ded4d0d
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39458
## Changelog
[Android][Deprecated] - hasConstants in ReactModuleInfo is marked as deprecated
getting rid of `mHasConstants` ivar here and marking the getter as deprecated
Reviewed By: cortinico
Differential Revision: D49262577
fbshipit-source-id: 5394281bdc4a315ffc0d1debfaef1808dc6f4299
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39457
## Changelog
[Android][General] - hasConstants in ReactModuleInfo does nothing now
we can get rid of this. currently, the default value of hasConstants true, and the only library setting this value to false is `WebSocketModule`. this value is only read in bridge mode - it is an optimization that will not initialize the constants dictionary for native modules in bridge.
however, we have plenty of native modules that don't provide constants that have not set this flag, so this is only turned on for `WebSocketModule`, which is probably not moving anything significant.
i would recommend we get rid of this to simplify the ReactModuleInfo deprecation plan.
Reviewed By: cortinico
Differential Revision: D49210251
fbshipit-source-id: c82c997e01cd8012b2c1a5c3c36061f81799eb04
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39431
Changelog: [Android][Breaking] - Do not enable `excludeYogaFromRawProps` feature flag, if you need to pass layout props to Java view managers when using new architecture
[Internal]
Reviewed By: NickGerleman
Differential Revision: D49114771
fbshipit-source-id: 171dfceef61d9851094465be8ff4eb9a87a3ab8f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39342
Annotate HybridData with DoNotStrip
The stack of diffs aims to introduce new features and deprecate old ones in the React Native Android architecture. The main goal is to make the architecture more stable and easier to use for developers. The changes include marking ReactPackage as NullSafe, introducing the new getModule method, introducing the WillBeDeprecatedInNewArchitecture annotation, deprecating createNativeModules and marking getModule as UnstableReactNativeAPI, and fixing a lint warning in MainReactPackage.
# This diff
This diff is adding the annotation `DoNotStrip` to the `HybridData` field in the `NativeMethodCallInvokerHolderImpl` and `CallInvokerHolderImpl` classes. This is to prevent the Android ProGuard optimizer from stripping the `HybridData` field, which is necessary for the proper functioning of the TurboModule system in React Native. Additionally, the `BUCK` file for the `core` module is updated to include the `androidx:annotation` and `com.facebook.proguard.annotations:annotations` dependencies.
changelog: [intenral] internal# Context
Reviewed By: cortinico
Differential Revision: D49077065
fbshipit-source-id: cb36c0a11bdb8f5f0f4eaa7db1f0c9dcb3be2341
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39339
Deprecating createNativeModules method from ReactPackage interface recommending using getModule instead in the new architecture of React Native
changelog: [Android][Changed] Deprecating createNativeModules method from ReactPackage interface recommending using getModule instead in the new architecture of React Native
Reviewed By: cortinico
Differential Revision: D48992719
fbshipit-source-id: b20de9274ed4bd35a239c672b52bfac529a79bc2
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39351
In this diff I'm introducing the new WillBeDeprecatedInNewArchitecture annotation, the goal of this annotation is to describe that a class or method will be deprecated when the new architecture of react native is fully rolled out in
OSS.
changelog: [Android] Introduce WillBeDeprecatedInNewArchitecture annotation
Reviewed By: cortinico
Differential Revision: D49068234
fbshipit-source-id: 4c5ce25b3fbcc2d7545f2000d931be893a661332
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39336
We are introducing the new getModule method into the ReactPackage interface following the "Registration of Native Modules" design
This method defaults to true with the goal to avoid compatibility breakage
changelog: [Android][Changed] Introducing getModule method into ReactPackage interface, defaulting to null. This method will be used in the Stable API of React Native
Reviewed By: cortinico
Differential Revision: D48992720
fbshipit-source-id: 4ecc93bc84daa8903a4d9a1cf6ced60adb78f153
Summary:
X-link: https://github.com/facebook/metro/pull/1078
Pull Request resolved: https://github.com/facebook/react-native/pull/39159
Source is not required for `getPreset` of `react-native/babel-preset`. There is a codition that adds `react-native/babel-plugin-codegen` to the preset only if source is passing certain regex. This condition fails if source is null, but that's wrong because the plugin may still be requred for this transformation even though source is not provided.
This diff changes the condition so the regexp tests source only if it is not null, and `react-native/babel-plugin-codegen` automatically added to the preset otherwise.
Changelog: [Internal]
Reviewed By: robhogan
Differential Revision: D48684443
fbshipit-source-id: 7cea69c91c0175ac0374d72e91f58e20fe75fd53
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39407
changelog: [internal]
CoreFeatures::enableMapBuffer is not used and always set to false, let's delete it.
Reviewed By: christophpurrer
Differential Revision: D49144919
fbshipit-source-id: 7d6a2a96ade13f7cedc2d12f7672c9df9d247e0d
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39418
Changelog: [Internal]
Adds the `enableOpenDebuggerRedirect` experiment flag. The flag enables the handling of GET requests in the `/open-debugger` endpoint, in addition to POST requests. GET requests respond by redirecting to the debugger frontend, instead of opening it using the `BrowserLauncher` interface.
This can be useful when integrating `dev-middleware` in highly customised environments (e.g. VS Code remoting) where things like automatic port forwarding interact poorly with the `BrowserLauncher` abstraction.
WARNING: As this is gated by an experiment flag, the functionality may change or go away unannounced. In separate work, we will look into a stable solution for this use case.
Reviewed By: huntie
Differential Revision: D49144733
fbshipit-source-id: 5af6c8b2ddaa7b6e7d14c792e49fe3d0849c7a25
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39423
D49158227 made the CDP WebSocket URLs returned from `/json` proxy-safe by using the Host header (etc) from the HTTP request. This did *not* fully fix the related `/open-debugger` endpoint, because the original headers were being lost in the internal `fetch('/json')` call.
Here, we eliminate that `fetch` call, and instead give the `/open-debugger` handler direct access to the method on `InspectorProxy` that backs the `/json` endpoint. This allows us to pass in an appropriate base URL derived from the headers seen by `/open-debugger`.
Changelog: [Internal]
Reviewed By: huntie
Differential Revision: D49229545
fbshipit-source-id: 9036ab295721e0d1fd3cdb608d0a7cc07b8f2eeb
Summary:
Right now we're fetching latests versions of drivers which isn't the best, we should always download the same versions.
## Changelog:
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[INTERNAL] - Add versions when installing Appium drivers.
Pull Request resolved: https://github.com/facebook/react-native/pull/39275
Test Plan: CI Green - (side note: if jobs `test_e2e_ios` and `test_e2e_android` are green it doesn't mean that they passed.)
Reviewed By: cipolleschi
Differential Revision: D49248778
Pulled By: NickGerleman
fbshipit-source-id: 5b114b7dc1172993afc4b02e9d3380afa9f03c40
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39367
Updates React Native to use the new CDP handler provided by the Hermes engine instead of the legacy one (`Connection.cpp`) built into React Native. The new Hermes CDP handler has a simpler & safer design, new features (e.g. `console.log` buffering) and is under active development by the Hermes team.
NOTE: Both the legacy and modern handlers are Hermes-specific. In future work, React Native will *wrap* the modern Hermes handler in an engine-agnostic CDP layer implementing additional functionality and managing the lifecycle of debugging sessions more correctly. This diff is the first step of this larger migration.
Changelog: [General][Changed] Use new Hermes CDP handler implementation for debugging
Reviewed By: cipolleschi
Differential Revision: D48783980
fbshipit-source-id: 4d2ca3fa04e96e92a38d82c90737cb660ba56655
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39426
When opening the react-native repo in Android Studio the first time, users is missing 3rd party native dependencies,
which gets downloaded during the first build.
Here I'm hooking the Gradle sync step to the `preBuild` task that takes care of preparing the repo so that the whole build can succeeds (i.e. downloading and unzipping native deps, running codegen, preparing hermes/hermesc, etc.).
Changelog:
[Internal] [Changed] - Make sure first Gradle Sync is downloading 3rd party deps
Reviewed By: cipolleschi
Differential Revision: D49231058
fbshipit-source-id: 11a3d436550581f8a67a582f9fd325ad39486ddc
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39429
Using `require` after `const registerFn = require('babel/register')` but before `registerFn(config)` causes Babel to transform required code with the default configuration (ie, using a nearby `babel.config.js`, if available).
This was causing the Babel plugins loaded by `metro-babel-register` to be (unnecessarily) transformed according to `babel.config.js`, which actually fails if the plugins/presets referenced in `babel.config.js` themselves require transformation.
This ensures no code is loaded in between registering Babel as a side effect of requiring Babel register, and replacing that hook with something explicitly configured.
## React Native
Changelog: [Internal]
## Metro
```
* **[Fix]:** `metro-babel-register` prevent arbitrary transformation of Babel plugins during registration setup
```
Reviewed By: dmytrorykun
Differential Revision: D49238671
fbshipit-source-id: 52a55b1b5dbd127171558c056f16ab04e8fa8232
Summary:
This mirrors the clang-format config used by fbsource to Yoga.
They are pretty similar, except for an annoying habit where Yoga's previous forced small functions in headers to be a a single line, so you would get a combination of multiline and single line functions next to each other which are hard to read. That is what motivated this change.
It also enforces header ordering (yay). I don't think we have any side-effect causing headers, so this should be safe.
Reviewed By: yungsters
Differential Revision: D49248994
fbshipit-source-id: 66998395e7c0158ff9d9fb1bee44e8401bdd8f21
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39404
X-link: https://github.com/facebook/yoga/pull/1378
We thread a config through the root node, to every function, that we don't actually use (with the exception of `canUseCachedMeasurement` in the previous diff which was passing root `pointScaleFactor`).
Since the model is currently per-node config, this only creates redundancy/confusion.
I do think we might want to make some more global/layout-pass scoped configuration in the future, but likely not with this sort of drilling.
Reviewed By: yungsters
Differential Revision: D49180385
fbshipit-source-id: 91248042df7d3cea1fc316b47b8137fcb790b521
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39417
## Changelog
[Android][Breaking] - Deleting warnOnLegacyNativeModuleSystemUse
this has not fired in over a year, we should clean it up. there's an argument we want to keep this for consumers that are in the middle of upgrading, but personally i think that this needs to be built into the interop layer - currently we can only configure this in Fb specific infra.
Reviewed By: cortinico
Differential Revision: D49209540
fbshipit-source-id: 0e2bb293999ad356e564a0acf069211a7a205f21
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39415
Add a simple constructor for `JSError` which does not accept a
`jsi::Runtime` and cannot call back into JSI. This guarantees that the
constructor cannot recursively invoke itself, leading to stack
overflows.
Changelog: [Internal]
Reviewed By: avp
Differential Revision: D48796703
fbshipit-source-id: 1c134e8a59ff54be64a5da901e548436d512c21d
Summary:
X-link: https://github.com/facebook/yoga/pull/1379
Pull Request resolved: https://github.com/facebook/react-native/pull/39403
Right now we have a `pointScaleFactor` per-node, but only ever read the one off the root node. In most cases where config is global, these will be the same, but it is possible for these to differ.
This... doesn't make much sense from an API perspective, and there are edge cases where we may want to allow laying out a subtree with a different DPI then the rest of the tree (though I think there might be other solutions to that).
We should rethink some of what is currently on config being allowed per-node (do we really need each node to be able to have a separate logger?), but this makes the model consistent in the meantime.
This change is breaking to any users relying on setting `pointScaleFactor` on the config of the root node, but not other nodes.
Reviewed By: yungsters
Differential Revision: D49181131
fbshipit-source-id: f1363ca242094f04b995fd50c1e56834d5003425
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39289
"tickleJS" is a function in RN Hermes Debugger, we need to implement it for Bridgeless for feature correctness and keep parity with Bridge.
Changelog:
[Internal] - Implement "tickleJS" for Hermes in Bridgeless mode
### Summary of discussion:
**Goal of this diff**:
Implement tickleJS for Hermes debugger with Bridgeless, here is Bridge's implementation
https://www.internalfb.com/code/fbsource/[7058fb4dae4f]/xplat/ReactNative/react/jsi/HermesExecutorFactory.cpp?lines=45-57
**The key problem to solve:**
As you can see from Bridge's implementation, we need to dispatch the js call of "__tickleJs" to dedicated JS thread queue for thread safety, and it did cause a crash for me locally without dispatching to JS thread queue (see P818439922).
For Bridgeless passing the JS message queue directly works, but then we would naturally want ask if RuntimeExecutor/RuntimeScheduler is a better choice because: 1, It should also work 2, It's the common way to run js code in native with Bridgeless 3, Can avoid potential race condition caused by dispatching to JS message queue directly 4, We're planning to expose RuntimeExecutor/RuntimeScheduler or a similar abstraction anyway. So the argument here is which solution should we take?
**Why we decide to move forward with the JS msq solution?**
- Time constraint: we need to ship this before 0.73
- After discussion there hasn't been any major concern over dispatching to JS msq directly so far
- How to expose RuntimeExecutor/RuntimeScheduler is a bigger topic which need further research, we don't want to hold this diff for it
For detailed discussion see the post https://fb.workplace.com/groups/react.technologies.discussions/permalink/3615044702060575/
Reviewed By: javache
Differential Revision: D48952581
fbshipit-source-id: 3dfb05d35c422f3fd6a420e265e4efe8fa13f057
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39413
`yarn android` on main is currently broken due to the CLI attempting to search for the manifest inside the src/androidTest folder.
This fixes it by specifying the exact path of the Android Manifest.
The fix to the CLI is also pending to prevent the CLI from searching inside test folders.
Fix for the CLI is here:
- https://github.com/react-native-community/cli/pull/2075
Changelog:
[Internal] [Changed] - Unblock `yarn android` on main
Reviewed By: huntie
Differential Revision: D49190626
fbshipit-source-id: 99309f17cb08a33be2a565f5faa29130862686ea
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39422
Those scripts were used when Buck OSS was invoking codegen. As we don't do Buck OSS anymore, we can remove them.
Changelog:
[Internal] [Changed] - Remove Codegen buck-oss scripts as they're unused
Reviewed By: huntie
Differential Revision: D49229846
fbshipit-source-id: fad0526e609bab159c56a62b85c91690e4cdbbce
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39380
Changelog:
[Internal][Added] - Created a new event dispatching pipeline that immediately moves events over to the C++ queue, along with the onFrame that triggers the event beat ticking mechanism.
Reviewed By: sammy-SC
Differential Revision: D49012996
fbshipit-source-id: 0bc9067e5b019f308ec1f45ca8bd83fd195b37ce
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39420
changelog: [internal]
The intent of the code is to retain shared_ptr<Scheduler> but by using a reference, it didn't do that. Leading to a race condition.
bypass-github-export-checks
Reviewed By: rubennorte
Differential Revision: D49227147
fbshipit-source-id: 1a548f7174eacb95c14c89f72b51ccd263c5ab01
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39357
changelog: [internal]
# Problem
## When React clones the wrong node revision
Whenever React wants to commit a new change, it first needs to clone shadow nodes. React sometimes clones from the wrong revision. This has mostly been fine, Fabric does state reconciliation to pass newest state forward. State reconciliation is needed, as we need to keep native state in the shadow tree.
However, when React clones a node that has never been through layout step, it will clone a node without any layout information and its yoga node is dirtied. Even though there might be a subsequent revision of the node with layout information already calculated. As a result, Yoga needs to traverse bigger parts of the tree, even though layout has been calculated before. It is just cached on a different revision that was used as a source.
There are two main sources (there is more but they don't help to paint the picture) when this can happen. Background Executor and State Progression. Let's start with the simpler one but less severe: Background Executor.
Background Executor moves layout from JavaScript thread. React can start cloning nodes right away, even though they might not have layout information calculated yet. This is a race condition and depending on when the node is cloned, we can see different results. In this case, React eventually clones node from the correct revision with the layout cache. It will be in a correct state in the end. This case is not as bad as far as I can tell but I included it here because it better illustrates what is going on.
State Progression is where things get worse. In this scenario, React will never clone from the correct revision and will never recover from this. Anytime React clones node with a state that needs to be progressed, it will get cloned one more time during commit but React will hold the wrong revision. Depending on where this node is located in the view hierarchy, it may lead to expensive layout calculations.
Example:
Let's use notation A/r1 as node of family A revision 1.
- React calls create node. Node A/r1 is created and React holds reference to this. It will later use it to clone it.
Node A has native state that was updated. New revision A/r2 is created. Now React and RN do not observe the same node anymore (this is sometimes necessary).
- React now clones node A to create A/r3. This revision may have the wrong yoga cache. Now this might sound like one off but let's explore what happens next.
- During commit, Fabric must do state progression to give node A/r3 state from A/r2. This requires cloning and new revision A/r4 is created. React has again a wrong node that does not have Yoga cache and can't recover from this state.
The blast radius of this varies depending on where in the tree the node is.
# Solution
Clone-less state progression. In this algorithm, state progression never clones. It checks if a ShadowNode has ever been mounted via `ShadowNode::hasBeenMounted_` to check if a node can be safely mutated without the need to clone and checks if current state the node is holding is obsolete (like the previous algorithm).
The clone-less state progression depends on the fact that any shadow node cloned from React is still unsealed and is not exposed to a multithreaded environment. This makes it safe to mutate it in place, without the need to clone.
WARNING: there is important semantic difference with the approach. With the old algorithm, you couldn't go back to a shadow node with old state. New state was always enforced when state reconciliation was enabled. The clone-less algorithm does not support that, because it can't mutate such a node in place.
Reviewed By: javache
Differential Revision: D49012353
fbshipit-source-id: 2d322f0587042c13761c83f7e916d1623159773d
Summary:
Currently, when the `_currentColorScheme` is nil the default comes from `[UITraitCollection currentTraitCollection]` which provides the trait collection of the context it was called in. Generally, this will work but in some cases the context can be different and this will return the wrong value also if the `Appearance` property is set in the plist, then initially that value is returned, regardless of if you have overridden it. Seen as the `userInterfaceStyle` of the window is overridden when the value is changed, then the default should be whatever the current style of the window is and not dependent on the calling context.
<table>
<tr><th>Before</th><th>After</th></tr>
<tr>
<td>
<video src="https://github.com/facebook/react-native/assets/30924086/3bd2d217-00f2-41d5-9229-05c31da2ecf5"/>
</td>
<td>
<video src="https://github.com/facebook/react-native/assets/30924086/6ed67e4c-dd01-40ff-84fd-0d7ffdf6534c" />
</td>
</tr>
</table>
## Changelog:
[IOS] [FIXED] - Fix the default trait collection to always return the value of the window
Pull Request resolved: https://github.com/facebook/react-native/pull/38214
Test Plan: Tested on rn-tester and verified the current behaviour is unaffected and also in our own repo to make sure it addresses the problem. Video provided above
Reviewed By: dmytrorykun
Differential Revision: D49186069
Pulled By: cipolleschi
fbshipit-source-id: f069fea8918fe17c53429564ed2a52e316ce893b
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39388
This change prepare the infra to release and work properly in the dual release mode, making sure that the new architecture is turned on with some versions of react native.
It connects the diffs in the previous changes in the stack.
## Changelog:
[iOS][Changed] - Set the new arch flag based on the React Native version.
Reviewed By: dmytrorykun
Differential Revision: D49149212
fbshipit-source-id: 4d97cc2fbf64d217aec60c0d71de6ed1802ef793
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39389
The Podfile of the Template is using a function called `get_default_flags` to get the default flags.
Its behavior is duplicated in both the default value of the `use_react_native!` function and in the body of the same function, making that helper actually redundant.
In this change, we are deprecating it so we can remove it in 0.74 with no breakages.
## Changelog:
[iOS][Deprecated] - Deprecate `get_default_flags` in Ruby scripts
Reviewed By: dmytrorykun
Differential Revision: D49147290
fbshipit-source-id: 41a9f9aa4ba5d1a31d86953fe78778b45d28d9b2
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39390
This diff introduce an helper to read the content of the React Native package of json from ruby.
## Changelog:
[iOS][Added] - Add helper to read the package.json from the cocoapods script.
Reviewed By: dmytrorykun
Differential Revision: D49146946
fbshipit-source-id: cd25052d743a61fcfabbe197b701188ec824709e
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39387
This change creates an helper function and tests to set the right value for the new arch enabled flag based on the react native version
## Changelog:
[iOS][Added] - add helper to set New Arch enabled flag based on RN version.
Reviewed By: dmytrorykun
Differential Revision: D49145515
fbshipit-source-id: c5ba0016b3be24b4529ff467cf37518624859538