Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42144
D51895785 changed several CMake libraries from shared to static, including `jsinspector`. This happens to be semantically incorrect in the case of `jsinspector`, as the library contains singletons which can be inadvertently duplicated due to static linking. As a result, different parts of the code can end up accessing different instances of a supposed singleton, leading to bugs.
Here we revert the change to `jsinspector` (only) and add an explanatory comment to signpost this for future readers.
## More context & general principle
While nothing is broken today, allowing static libraries to contain global state is brittle and breaks in surprising ways:
* The upcoming diff D52231237 introduces a new dependency on `jsinspector` which builds cleanly, but causes debugging to stop working because of the duplicated singleton.
* The only reason debugging currently works in the CMake build of Bridgeless is by a happy accident: the shared library `hermesinstancejni` depends on `reactnativejni` through a chain of three other libraries unrelated to debugging, and as a result, can access `reactnativejni`'s copy of `jsinspector` (see graph).
{F1237835169}
It seems that the safest rule of thumb, given the way React Native is currently structured, is that **singletons should live in their own shared libraries** so no call site can cause them to be duplicated through static linking. (It's reasonable to revisit this guidance if we manage to consolidate React Native into one monolithic shared library, eliminating the footgun at the source.)
Changelog:
[Internal] [Changed] - Change jsinspector back to a shared library in the CMake build.
Reviewed By: cortinico, NickGerleman
Differential Revision: D52541488
fbshipit-source-id: 502210add0b734a9bbc470bdf38fb70a41e149a9
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42136
`Wpedantic` flags usage of variadic macros with zero arguments. This is widely supported by different compilers (including MSVC), but was previously forbidden by the standard.
C++ 20 explicitly allows them, so, theoretically Clang should know not to warn about these now. Let's try that.
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D52534129
fbshipit-source-id: e27a75081fac6b4196c6dbb5242812877b0bd679
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42133
## Changelog:
[General][Fixed] - TouchableBounce, TouchableHighlight and TouchableNativeFeedback dropping touches with React 18.
TouchableBounce, TouchableHighlight and TouchableNativeFeedback do not trigger onPress when used with React 18. This is because it resets its pressability configuration in `componentWillUnmount`. This is fine, we want to stop deliver events and restart all timers when component is unmounted.
```
componentWillUnmount(): void {
this.state.pressability.reset();
}
```
But TouchableBounce, TouchableHighlight and TouchableNativeFeedback were not restarting the pressability configuration when component was mounted again. It was restarting the configuration in `componentDidUpdate`, which is not called when component is unmounted and mounted again.
Reviewed By: fkgozali
Differential Revision: D52514643
fbshipit-source-id: 0d6ae4bb7c2a797cc443181459c5614da0ecfc7a
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42062
We have had relative as the default for a few big frameworks/apps right now (Fabric FB iOS, Fabric FB Android, OSS, Litho, CK) and have not run into issues. Seems it is safe to pull the trigger here and put everything on relative 🎉
This also fixes a test that relied on this default, changes the layout metrics default, and removes the gating plumbing that was in place earlier.
Lastly, a few animation tests start failing after this change. Seems that there is an animation bug with relative trees that would have existed already, so this is merely discovering that that bug exists, not causing any extra issues. Since that test is a set of random trees with random props it is very hard to debug and I am just adding skips to the failing ones.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D52137858
fbshipit-source-id: 6856bc608b8211c868c9ee81fc92e005ec3d2faa
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42020
I added position type in D51412428 (https://github.com/facebook/react-native/pull/41819). I didn't notice this == override which makes it so position type in layout metrics will not be updated if it changes.
To use this cpp 20 feature we needed to change a few buck files which is also done here
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D52339890
fbshipit-source-id: e77ee092477dbf786e4a72e6a33138ccbc450645
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42122
This diff introduces the `TypeUtils` directory where we can put platform-specific, context-independent type transformations.
Changelog: [Internal]
Reviewed By: cipolleschi
Differential Revision: D52291837
fbshipit-source-id: 561b9c494aab5bfee3b3c668d3346bbd320e5266
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42128
Scroll view was not emitting the `onScrollToTop` event when the user tapped on the status bar.
This change fixes this by adding the event to the C++ Event Emitter and by invoking it into the `RCTScrollViewComponentView`
## Changelog
[iOS][Added] - Add onScrollToTop event in Fabric
Reviewed By: sammy-SC
Differential Revision: D52509919
fbshipit-source-id: 7b72c927823fa971be99c4da4b0287d4e23a02b6
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42127
The current scrollViewExamples does not trigger the OnScrollToTop event.
This change for Paper adds that and refactors the codebase to isolate that example.
## Changelog:
[Internal] - Improve RNTester ScrollView example adding OnScrollView event.
Reviewed By: sammy-SC
Differential Revision: D52509669
fbshipit-source-id: 8fd0fcca7153ba41bf054832928e661ef7dff3fe
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42113
For easier testing/debugging, log something if bridgeless is enabled for the app. This log will show up only once.
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D52464640
fbshipit-source-id: 5019a1a6bf4f171a5f1dc4b3b2692db9e07ff43c
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42073
This moved various Meta-internal runtime setup off AppDelegate.mm to reduce the #if checks throughout the file.
Changelog: [Internal]
Reviewed By: cipolleschi
Differential Revision: D52424748
fbshipit-source-id: b53799c8bb1544dbbb429cea811861ae52125641
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42086
Changelog: [General][Changed] - Update monorepo dependency versions to remove ^
This change will remove the caret for now as we already perform an "align" step everytime we bump a monorepo library. This prevents monorepo library updates to affect existing releases.
The "align" step updates all monorepo libraries to use the updated bumped version: https://fburl.com/code/xfistiph
Reviewed By: huntie
Differential Revision: D52440454
fbshipit-source-id: ff071032f04bc554903dde153c594991163dfe2f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42121
## Changelog:
[General][Fixed] - TouchableWithoutFeedback and TouchableOpacity dropping touches with React 18.
TouchableWithoutFeedback and TouchableOpacity do not trigger onPress when used with React 18. This is because it resets its pressability configuration in `componentWillUnmount`. This is fine, we want to stop deliver events and restart all timers when component is unmounted.
```
componentWillUnmount(): void {
this.state.pressability.reset();
}
```
But TouchableWithoutFeedback and TouchableOpacity were not restarting the pressability configuration when component was mounted again. It was restarting the configuration in `componentDidUpdate`, which is not called when component is unmounted and mounted again.
Reviewed By: fkgozali
Differential Revision: D52388699
fbshipit-source-id: ef13194c6581c5d31d0f1cb465bfd0cf98d672ea
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42090
This change is the last pieces of removing `RCT_NEW_ARCH_ENABLED` flag and defragmenting the build setup on iOS.
Before, 3rd party libraries had to use the `#if RCT_NEW_ARCH_ENABLED` flag to compile in and out segment of code depending on whether the new architecture was turned on or not.
After the recent changes, we can now expose the `RCTIsNewArchEnabled()` function to read whether the New Arch is enabled at runtime or not.
This will promote better code practices as we can replace ugly, compile time, `#if-#else-#endif`s with a more readable and natural regular obj-c code.
We can also use inheritance to have different implementation based on the architecture.
To use the new function, a 3rd party library have to:
1. `#import <React/RCTUtils.h>` (if they use the `install_modules_dependencies` function we provide, they can already do it)
2. invoke `RCTIsNewArchEnabled()` which returns a BOOL.
3. implement the code accordingly, depending on the New arch state.
**Note:** we implemented also the `RCTSetNewArchEnabled` function. This is called as soon as React Native is initialized in the `RCTAppDelegate`. The method can be called only once per React Native lifecycle. Subsequent calls to that method are ignored.
## Changelog:
[iOS][Added] - Added the `RCTIsNewArchEnabled()` to check whether the New Arch is enabled at runtime.
Reviewed By: cortinico
Differential Revision: D52445107
fbshipit-source-id: 1b432832912d33c85687b4c37f9e360ce9699f59
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42088
This change adds an extra function to customise the RootView in both Bridge and Bridgeless mode.
To nudge users in a migration, we also add a warning message for next version that should push our users to migrate away from the old implementation to the new one.
*The Warning is shown ONLY when the user do customise the rootView*. For users which were not customising the Root View, the warning will not appear.
The documentation of the new method plus the warning should guide the users toward the right migration path.
## Changelog
[iOS][Added] - Added the customiseRootView method which is called in both bridge and bridgeless. Added also a warning for 0.74 with instructions on how to migrate.
Reviewed By: cortinico
Differential Revision: D52442598
fbshipit-source-id: 8b99b67f4741ee61989a8659a3d74c1eba27bc5b
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42097
Since we switched all apps from `getJSIModule()` to `getFabricUIManager()` from `ReactContext` and it's subclasses it's safe to delete this method.
NOTE: The fallback for FabricUIManager is still catalystInstance.getJSIModule() that's still there for backwards comptability just deleting the indirection through ReactContext
Changelog:
[Internal] Internal
Reviewed By: christophpurrer
Differential Revision: D51748655
fbshipit-source-id: dbf1a661f9e380307614662dd6079110f878d143
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42072
This existing flag was for experimental (WIP) purpose only, and is undocumented, by design. Let's rename it so to make it clear. Libraries/Apps should not use this flag.
Changelog: [Internal]
Reviewed By: christophpurrer
Differential Revision: D52424750
fbshipit-source-id: 742fc6e31d1887e68439849e157dd23aaa054e36
Summary:
See https://github.com/facebook/react-native/issues/41929 for an issue on multiple monorepo packages being installed. The reason is that `*` resolves to whatever is tagged `latest` on npm.
We still need to fix the fact that our monorepo publish script will update the latest tag everytime we publish. For now, we should remove these from `main` and we will also update this in the 0.73 release branch.
I've left the two peer dependencies on `react-native` to keep at `*`.
```
virtualized-lists/package.json
30: "react-native": "*"
rn-tester/package.json
32: "react-native": "*"
```
As a peer-dependency this won't be a problem in terms of installing a second `react-native`. I thought about updating these to `nightly`, but that would install multiple nightly react-natives as the tag will be updated with each nightly release. I think for now this is fine and something we can revisit.
Things left to do
[ ] Fix monorepo publish script to not update `--latest`
[ ] Remove ^ dependencies on monorepo packages: https://github.com/facebook/react-native/pull/41958
[ ] Re-evaluate how we bump and align monorepo packages when we cut a release branch. I forget if we manually update this when we cut or if there is a script. We may want to change the script and have `main` dependencies point to some fake version like `1000.0.0` and only update these on nightly publishes. Regardless, this will need some discussion.
## Changelog:
[GENERAL] [CHANGED] - Be explicit about what monorepo versions we are using
Pull Request resolved: https://github.com/facebook/react-native/pull/42081
Test Plan: N/A
Reviewed By: cortinico, cipolleschi
Differential Revision: D52435234
Pulled By: lunaleaps
fbshipit-source-id: 67da029d2b637e3997c12c21fe2a9ab9bc344399
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42085
Deprecating the old JSI module APIs: `getJSIModule(JSIModuleType moduleType)`, `addJSIModules(List<JSIModuleSpec> jsiModules)` and `setTurboModuleManager(JSIModule getter)` to further delete them in future release. Deprecating them as of now to cater the OSS use-cases
Changelog:
[Internal] internal
Reviewed By: christophpurrer
Differential Revision: D50927292
fbshipit-source-id: 1d25f9f28b8aaf34979a90e4792317b263ae1714
Summary:
This is a continuation of my [last PR](https://github.com/facebook/react-native/pull/40914) which improved the symbolication of unhandled promise rejections.
While I was developing another library I noticed I still got an error stack of the log adding and not of the error itself. The library I'm trying to debug does not throw a standard error object but rather a custom one, but it still contains the stack field. By passing this stack field to the logbox call I was able to get a better symbolicated stack trace. The exact line of the failure is not displayed but at least the correct file is.
## Changelog:
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
[GENERAL] [ADDED] - Unhandled promise rejection - attach non-standard Error object stack info if possible
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
Pull Request resolved: https://github.com/facebook/react-native/pull/42079
Test Plan:
Test any unhandled promise rejection with a non-standard error (line 23, toString must not return `[object Error]`) and see if the correct (or at least a better) stack trace is shown.
Here is the one I got before and after this change:
<img src="https://github.com/facebook/react-native/assets/1634213/3d07faad-9535-42c9-8032-b4d8fe407e88" width="200" />
<img src="https://github.com/facebook/react-native/assets/1634213/2c39bd82-c7a1-4f58-8ac4-5c479bb96b6e" width="200" />
Reviewed By: huntie
Differential Revision: D52431711
Pulled By: cipolleschi
fbshipit-source-id: be2172d3b1e2fc3f72812faac372c83bc6dface2
Summary:
Building native modules from source, may take a long time. Xcode already helps bring this down, by providing incremental builds, as long as the user doesn't delete their `ios/build` directory. But in some situations, i.e. when iterating the native code of an app or library or when the developer need to delete that `ios/build` directory, it's advantageous to use a compiler cache, such as ccache. This is already outlined in our ["Speeding up your Build phase"](https://reactnative.dev/docs/build-speed#xcode-specific-setup) guide.
But setting up an Xcode project to use Ccache with the correct configuration, isn't trivial in a way that doesn't require symlinking `clang` and `clang++` or passing configuration via environment variables on every `npm run ios` invokation.
This PR takes its inspiration from the existing guide on [setting up Ccache for Xcode](https://reactnative.dev/docs/build-speed#xcode-specific-setup), but applies the build settings only if an installation of `ccache` is detected and the feature is explicitly opted into via an argument to the `react_native_post_install` function or a `USE_CCACHE` environment variable. It uses two shell scripts to wrap the call to `ccache`, which both injects a default `CCACHE_CONFIGPATH` environment variable (i.e. it won't override this if already provided, to allow for customisations on CI), pointing to a `ccache.config` which works well with React Native projects (it has the same values as the guide mentions).
For context, I posted about this change in the ios channel of the contributors Discord server, where I discussed it with cipolleschi and saadnajmi
### Additional output printed when running `pod install`
#### When `ccache_available and ccache_enabled`
```
[Ccache]: Ccache found at /opt/homebrew/bin/ccache
[Ccache]: Setting CC, LD, CXX & LDPLUSPLUS build settings
```
#### When `ccache_available and !ccache_enabled`
```
[Ccache]: Ccache found at /opt/homebrew/bin/ccache
[Ccache]: Pass ':ccache_enabled => true' to 'react_native_post_install' in your Podfile or set environment variable 'USE_CCACHE=1' to increase the speed of subsequent builds
```
#### When `!ccache_available and ccache_enabled`
```
[!] [Ccache]: Install ccache or ensure your neither passing ':ccache_enabled => true' nor setting environment variable 'USE_CCACHE=1'
```
#### Otherwise
If the user doesn't have ccache installed and doesn't explicitly opt into this feature, nothing will be printed.
bypass-github-export-checks
## Changelog:
[IOS] [ADDED] - Added better support for `ccache`, to speed up subsequent builds of native code. After installing `ccache` and running `pod install`, the Xcode project is injected with compiler and linker build settings pointing scripts that loads a default Ccache configuration and invokes the `ccache` executable.
Pull Request resolved: https://github.com/facebook/react-native/pull/42051
Test Plan:
I've tested this manually - would love some inspiration on how to automate this, if the reviewer deem it needed.
To test this locally:
1. Install Ccache and make sure the `ccache` executable is in your `PATH` (verify by running `ccache --version`)
2. Create a new template app instance and apply the changes of this PR to the `node_modules/react-native` package.
3. Set the `USE_CCACHE` environment variable using `export USE_CCACHE=1`.
4. Run `pod install` in the `ios` directory.
5. Check the stats of Ccache (running `ccache -s`).
6. Run `npm run ios` or build the project from Xcode.
7. Check the Ccache stats again to verify ccache is intercepting compilation ("Cacheable calls" should ideally be 100%).
8. To check the speed gain:
a. Delete the `ios/builds` directory
b. Zero out the ccache stats (by running `ccache -z`)
c. Run `pod install` again (only needed if you ran the initial `pod install` with new architecture enabled `RCT_NEW_ARCH_ENABLED=1`).
d. Run `npm run ios` or build the project from Xcode.
e. This last step should be significantly faster and you should see "Hits" under "Local storage" in the ccache stats approach 100%.
Reviewed By: huntie
Differential Revision: D52431507
Pulled By: cipolleschi
fbshipit-source-id: 6cfe39acd6250fae03959f0ee74d1f2fc46b0827
Summary:
This PR contains the changes from https://github.com/facebook/react-native/pull/30981 that got closed due to inactivity.
Many thanks to nickdowell for this bug report & fix. We encountered this error in our project when we had an Xcode scheme that contains a space (like `AppName alpha`).
This change fixes the generation of source maps for Xcode projects where the output path contains spaces.
The `EXTRA_ARGS` environment variable, being a plain string, would be split into arguments by whitespace - so a path containing spaces was being treated as several arguments rather than one.
This change uses an array to contain the arguments instead, allowing the proper handling of arguments that may contain spaces.
bypass-github-export-checks
## Changelog:
[iOS] [Fixed] - Fix support for --sourcemap-output path containing spaces
Pull Request resolved: https://github.com/facebook/react-native/pull/40937
Test Plan:
Tested using a sample project with the following "Bundle React Native code and images" Xcode build phase
```
export SOURCEMAP_FILE="$CONFIGURATION_BUILD_DIR/$UNLOCALIZED_RESOURCES_FOLDER_PATH/main.jsbundle.map"
set -e
export NODE_BINARY=node
../node_modules/react-native/scripts/react-native-xcode.sh
```
and a `CONFIGURATION_BUILD_DIR` that contains spaces - `~/Library/Xcode/Derived Data`. **You can also try an XCode-scheme that contains a space.**
### Before
```
+ EXTRA_ARGS=
+ case "$PLATFORM_NAME" in
+ BUNDLE_PLATFORM=ios
+ EMIT_SOURCEMAP=
+ [[ ! -z /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map ]]
+ EMIT_SOURCEMAP=true
+ PACKAGER_SOURCEMAP_FILE=
+ [[ true == true ]]
+ [[ '' == true ]]
+ PACKAGER_SOURCEMAP_FILE='/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
+ EXTRA_ARGS=' --sourcemap-output /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
+ node /Users/nick/Desktop/RN064/node_modules/react-native/cli.js bundle --entry-file index.js --platform ios --dev false --reset-cache --bundle-output '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle' --assets-dest '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app' --sourcemap-output /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map
Welcome to Metro!
Fast - Scalable - Integrated
info Writing bundle output to:, /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle
info Writing sourcemap output to:, /Users/nick/Library/Developer/Xcode/Derived
```
Note the incorrect sourcemap output path.
### After
```
+ EXTRA_ARGS=()
+ case "$PLATFORM_NAME" in
+ BUNDLE_PLATFORM=ios
+ EMIT_SOURCEMAP=
+ [[ ! -z /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map ]]
+ EMIT_SOURCEMAP=true
+ PACKAGER_SOURCEMAP_FILE=
+ [[ true == true ]]
+ [[ '' == true ]]
+ PACKAGER_SOURCEMAP_FILE='/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
+ EXTRA_ARGS+=("--sourcemap-output")
+ EXTRA_ARGS+=("$PACKAGER_SOURCEMAP_FILE")
+ node /Users/nick/Desktop/RN064/node_modules/react-native/cli.js bundle --entry-file index.js --platform ios --dev false --reset-cache --bundle-output '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle' --assets-dest '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app' --sourcemap-output '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
Welcome to Metro!
Fast - Scalable - Integrated
info Writing bundle output to:, /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle
info Writing sourcemap output to:, /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map
```
sourcemap output path fixed 🎉
Reviewed By: arushikesarwani94
Differential Revision: D52431057
Pulled By: cipolleschi
fbshipit-source-id: 528217c84fe3f467a30baa15cfa4dcb2ed713165
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42060
For removal of JSIModule getting rid of the inheritance relationship b/w interfaces TurboModuleManager & JSIModule by directly defining `invalidate()`. `initialize()` here isn't being used hence not defining it.
Changelog:
[Internal] internal
Reviewed By: philIip, mdvacca
Differential Revision: D49977957
fbshipit-source-id: 8de644b1f344d8ce8d4a78655556829f860a2b10
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41802
Those tests are not executing at all, they're just compiled.
Our internal infra is still depending on some bits of it though, so I'm moving them to `fbandroid/java/com/facebook/fbreact
Changelog:
[Internal] [Changed] - Move legacy tests from OSS to fbandroid/java/com/facebook/fbreact
Reviewed By: rshest
Differential Revision: D51805702
fbshipit-source-id: 2c5cec68efa9854184e981220202d8f356ff690a
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42078
Resubmit of D51891716 and D52033328
I'm doing a pass and converting the last Java Unit Tests we had to Kotlin
I've also re-enabled multiple tests that were disabled in the past.
Changelog:
[Internal] [Changed] - Convert the last Unit Tests to Kotlin
Reviewed By: rshest
Differential Revision: D52430728
fbshipit-source-id: e6b4a6ed88d852024d959cf5148e992e97a84434
Summary:
I noticed this comment is still in Java in the Kotlin template. It also doesn't really work anymore since there is no packages variable.
To fix it I completed the comment with all code needed for it to work in kotlin. I think an older version of the template used to be more like:
```kotlin
val packages = PackageList(this).packages
// packages.add(MyReactNativePackage())
return packages
```
But then it requires adding a lint suppress annotation since packages variable can be simplified. I think this is simpler even if it makes the comment a few more lines.
## Changelog:
[GENERAL] [FIXED] - Fix comment about adding packages in android template
Pull Request resolved: https://github.com/facebook/react-native/pull/41856
Test Plan: Tested that uncommenting that code works
Reviewed By: cipolleschi
Differential Revision: D51987483
Pulled By: cortinico
fbshipit-source-id: d0135b5b536960017ccc7b25f92c75b3bd863cd9
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42076
## Changelog:
[Internal] -
In https://github.com/facebook/react-native/pull/41519 we introduced usage of C++20s range operations, which broke MacOSX desktop builds for the x86_64 targets (e.g. on Intel Mac laptops).
This appears to be a [known issue](https://stackoverflow.com/questions/73929080/error-with-clang-15-and-c20-stdviewsfilter), fixed in the later clang versions, however we need to support the earlier ones as well.
This changes the code to use the good old imperative style to do the same thing, but without using `std::views::filter`, thus working around the problem.
Reviewed By: christophpurrer
Differential Revision: D52428984
fbshipit-source-id: 6d0a390549c462b7040b5c0e669c00932bd99af7
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42059
Getting rid of old APIs in FabricUIManagerProvider and also clearing it of the inheritance dependency it has on JSIModule post it's references have been cleared.
Reviewed By: christophpurrer
Differential Revision: D51001239
fbshipit-source-id: c3d4650c292e957e9f939304662932c11af7a24f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42069
Refactor React to get rid of JSIModule and its dependencies now that the changes are rolled out for all internal apps.
Changelog:
[Internal] Internal
Reviewed By: christophpurrer
Differential Revision: D51058885
fbshipit-source-id: 07a7335235605fbc07657f8da8588ec548bce797
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42068
These methods are overriden in UIManager.js
Let's pull them out, so that we don't get distracted by them.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D52350348
fbshipit-source-id: 3d4b446c40be9d8797ec787f45335f42f8982956
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41999
Just a cosmetic change, that will remove noise from the subsequent diffs.
Now, all the raiseSoftError are on similar columns in the file.
Changelog: [internal]
Reviewed By: cortinico
Differential Revision: D52041976
fbshipit-source-id: bc22add358becf1ad8d5f7602253e2af28697d42
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42071
Proxying background handling to set up Meta internal test infra.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D52420805
fbshipit-source-id: a68645b7b630f976dfd9e863b0c985c738c658ec
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42000
I think this makes BridgelessUIManager easier to read: if the getUIManagerConstants method exists, get the cached constants.
Also, this unifies the nomenclature between PaperUIManager and BridgleessUIManager. That way, it's easy to compare/constrast the two files.
Changelog: [Internal]
Reviewed By: dmytrorykun, luluwu2032
Differential Revision: D52002910
fbshipit-source-id: 01bfbd5fedbe3f995b4a1f68309714d84027133b
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42001
## Rational
Every call-site was calling into console.error.
Some call-sites were using string concatination.
This diff introduces a new error reporting method: raiseSoftError, that hides all the string concatination and console.errors.
I believe this makes the error reporting logic within BridgelessUIManager more readable.
Changelog: [Internal]
Reviewed By: luluwu2032
Differential Revision: D52002911
fbshipit-source-id: 186842a4835ca65f326dda35b1c0db50f8ff149c
Summary:
The Android minSdk has been bumped in https://github.com/facebook/react-native/pull/38874 but not in the README.
## Changelog:
[General] [Fixed] - Updated docs to match Android 6.0 (API 23) minimum requirement.
Pull Request resolved: https://github.com/facebook/react-native/pull/42034
Test Plan: N/A
Reviewed By: fkgozali
Differential Revision: D52364522
Pulled By: arushikesarwani94
fbshipit-source-id: b04b5aa94b629380b559b2717e12a882f8817a6f
The internal and external repositories are out of sync. This Pull Request attempts to brings them back in sync by patching the GitHub repository. Please carefully review this patch. You must disable ShipIt for your project in order to merge this pull request. DO NOT IMPORT this pull request. Instead, merge it directly on GitHub using the MERGE BUTTON. Re-enable ShipIt after merging.
Summary:
X-link: https://github.com/facebook/yoga/pull/1533
Pull Request resolved: https://github.com/facebook/react-native/pull/42031
I have some reservations about some of the conditional setting of trailing position in general, and some of the repeated transformations that neccesitates this, but these functions don't belong in `CalculateLayout.h`. For now, just move these to their own header.
Reviewed By: joevilches
Differential Revision: D52292121
fbshipit-source-id: 4a998a4390a8d045af45f5424adaf049ed635e7a
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41519
Changelog: [Internal] - Refactor hover tracking logic to the shared c++ renderer
This diff refactors our hover tracking logic out of the platform (in this case only iOS, integrating Android is going to require extra work due to their pointer events being untyped) and puts it into the event intercepting infra in Fabric's C++ core. This is a big-ish diff so I'm going to try my best to use this summary to guide you through the changes.
To begin with — the changes inside `RCTSurfacePointerHandler.mm` are mostly about removing the existing hover tracking logic. The logic of the hover tracking is largely the same between the objective-c and c++ implementations with minor tweaks in order to be a better citizen when it comes to storing node references. One small "addition" to this file is explicitly firing a `pointerleave` event from the iOS layer when we detect that the pointer has left the app entirely because the C++ would otherwise not know when the pointer leaves the app. We don't need to include a special case for `pointerenter` because we can derive that in C++ from the first `pointermove` event that gets sent once the pointer re-enters the app's root view.
Next I think it makes most sense to continue onto the `PointerEventsProcessor.h/mm` which is the central class we're working in. One small change is adding a flag to the `ActivePointer` struct (`shouldLeaveWhenReleased`) which we will set during `ActivePointer` registration — setting to false if the pointer in question exists in the (also) newly added `previousHoverTrackersPerPointer_` registry. This logic is primarily used for knowing later when the pointer is released and whether we should emit the synthetic leave/out event on release. If the pointer existed in `previousHoverTrackersPerPointer_` **before** the `ActivePointer` registration that implies that the pointer is capable of hovering to some degree and we should **not** emit those leave/out events yet.
The real meat & potatoes of this diff is the `handleIncomingPointerEventOnNode` method which matches the `handleIncomingPointerEvent` method we removed from `RCTSurfacePointerHandler.mm`. This method derives the enter/leave/over/out pointer events by comparing the current event's target path (list of nodes from the root node to the target node of the event) to the previously recorded event target path. The representation of this event path is through the new `PointerHoverTracker` class which stores a pointer to just the root node and the target node as we can recreate the entire event path from these.
For over/out events all that matters is when the deepest-most target changes which is checked in `handleIncomingPointerEventOnNode` by leveraging `PointerHoverTracker`'s `hasSameTarget` method. For enter/leave events we need to fire discrete events for every node in the path which has either been removed or added, so the `diffEventPath` method was introduced on `PointerHoverTracker` to provide that.
Reviewed By: yungsters
Differential Revision: D51317492
fbshipit-source-id: e15ac3a396d5afa7ab921e4589861b43b07a33b5