Merge up to January 2nd upstream #2013
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: Pull Request resolved: facebook#41661 This aligns the Kotlin version used inside fbsource to the one used for React Native GitHub Changelog: [Internal] [Changed] - Kotlin to 1.8.22 Reviewed By: NickGerleman Differential Revision: D51587726 fbshipit-source-id: 5f985bd50c7688e4d369184b79dbf1bdc799876e
Summary: Pull Request resolved: facebook#41660 While working on other things, I noticed those warnings firing on console which I'm fixing here. Changelog: [Internal] [Changed] - Fix several build warnings on RN Tester Android Reviewed By: cipolleschi Differential Revision: D51589072 fbshipit-source-id: 1ddb29afd0d150f1ccbc7a8def9f27ecedb69724
…ok#41667) Summary: Pull Request resolved: facebook#41667 changelog: [internal] I made a mistake during refactor in D51471667 where I removed the check if rawProps is nullptr. We must check if props are empty during `UIManager::clone`, leaving the check for `ConcreteComponentDescriptor::cloneProps` does not lead to the same result. There is a deeper problem here that needs to be analysed but this should resolve the lunch blocker. Reviewed By: javache Differential Revision: D51614396 fbshipit-source-id: 055694c4a71a914d8732a3632c50026cc24cbe7d
…animation Summary: caused performace problems with react app Reviewed By: gpalves Differential Revision: D51617862 fbshipit-source-id: 38c0c06dacdd7aa862fd523a7ce136e54ed55fa2
Summary: Pull Request resolved: facebook#41675 FBJni expects the HybridData object to exist on the mHybridData property of the java object. So, we have to call this propery mHybridData. Otherwise, this fbjni class just won't work. Changelog: [Internal] Reviewed By: javache Differential Revision: D51550621 fbshipit-source-id: d169266474717f0a38799ede7c07af57461012b7
Summary: Pull Request resolved: facebook#41674 The problem: The Java runtime couldn't find CatalystCxxReactPackage.initHybrid. Why: I think is because CxxReactPackage loads CatalystCxxReactPackage's so in its constructor. This might be too late to load the derived class's so. So, I just switched derived delegates to load the so in the static initializer (i.e: the recomended approch for so loading): https://www.internalfb.com/code/fbsource/[91c4e41c49ed191ac864250ccaec52c01ddaeccc]/fbandroid/libraries/soloader/java/com/facebook/soloader/SoLoader.java?lines=52-54%2C60-61%2C63-67 This way: 1. The So's are loaded plenty early (the method not found error went away). 2. We don't create our own so loading infra, which complicates this abstraction, and makes it harder to work with, even more. Changelog: [Internal] Reviewed By: javache Differential Revision: D51550622 fbshipit-source-id: f4782d6fa9387f21fbf611191e9483e2a58b3a34
Summary: Pull Request resolved: facebook#41673 ## The Problem The cxxReactPackage property isn't initialized by the time initHybrid was executed. So, initHybrid was being called with null as the cxxReactPackage. Why; 1. The default turbomodule manager delegate receives cxxReactPackage as a constructor param. 2. Java then executes all the constructors of the class hierarchy. This executes initHybrid. 3. Java finally initializes the properties of the derived class: default tmmd.cxxReactPackage (i.e: the parameter to initHybrid). **This is too late** ## The Fix Refactor the code such that hybrid data creation doesn't depend on property initialization: 1. Create a static initHybrid method in default tmmd. 2. Call this static method with the cxxReactPackage, and assign the resultant HybridData to mHybridData (in tmmd). Changelog: [Internal] Reviewed By: javache Differential Revision: D51550623 fbshipit-source-id: ed2b7587351cfca408cda3c8cef4dcf7547e5f1e
Summary: Fixes UI operation not on the main thread.   ## Changelog: [IOS] [FIXED] - Move ActionSheet UI operation to main thread Pull Request resolved: facebook#41666 Test Plan: None. Reviewed By: philIip Differential Revision: D51613525 Pulled By: cipolleschi fbshipit-source-id: e5f543bbcc9bc0f5b6dda5bc2deb20279e851946
Summary: Pull Request resolved: facebook#41679 This fixes a bug in the original implementation of image attached callbacks (still experimental). The problem was that we were unconditionally caching the ref passed to the underlying image component, which meant that whenever users passed new ref setters we wouldn't call them again. This fixes that by forcing the creation of a new ref value whenever a new ref is passed to the image component. Changelog: [internal] Reviewed By: jehartzog Differential Revision: D51618512 fbshipit-source-id: ac15160c528563c2131e8b3444dea4a6096f20bc
…#41680) Summary: Pull Request resolved: facebook#41680 Just like how React Native can have n ReactPackages, it will support n CxxReactPackages. This way, many applications can share common CxxReactPackages. Changelog: [Internal] Reviewed By: christophpurrer Differential Revision: D51484844 fbshipit-source-id: b9b70cab719e80a7ff7e635057d710f1a86fb1c9
Summary: Pull Request resolved: facebook#41575 We currently do not validate the incoming native exception message before passing it to the char* constructor of TwineChar16. Treat it as UTF-8 and convert it to UTF-16 before creating the JavaScript exception. Changelog: [Internal] Reviewed By: tmikov Differential Revision: D49551640 fbshipit-source-id: 762f8038b29818d804bda5a7f3b4762621c94336
Summary: Pull Request resolved: facebook#41506 Adding APIs for `getFabricUIManager()` to ReactContext and it's subclasses. This will replace the `getJSIModule()` post JSI module deletion. Thereby replacing the callsite to context.getFabricUIManager() in UIManagerHelper. NOTE: This still has fallback to getJSIModule() in case the UIManagerProvider is not set Changelog: [Internal] internal Reviewed By: philIip Differential Revision: D50926218 fbshipit-source-id: f311affb0f82895b254fd4664aa8ea23ab31bac0
Summary: Pull Request resolved: facebook#41481 This will allow us to keep RN on it's "pseudo-static" mode, while changing the Yoga default back to relative, to avoid breaking existing layouts. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D51182861 fbshipit-source-id: 25489d7f0642c4ff78340438c2b266e95a5fb207
Summary: Pull Request resolved: facebook#41480 X-link: facebook/yoga#1469 The previous version of static didn't do anything inside of Yoga. Now that we're making it do something, this changes the default back to relative so that users with no errata set don't see their deafult styles changing. Reviewed By: joevilches Differential Revision: D51182955 fbshipit-source-id: c0ea357694e1367fb6786f1907dfff784b19a4bc
Summary: Pull Request resolved: facebook#41693 This diff fixes app warm start time. Before this change, we cache the first time when app start timing is logged, and ignore future loggings. Some apps are warm started and the startup time should be updated. Reviewed By: dmitry-voronkevich Differential Revision: D50481710 fbshipit-source-id: 03e00b75ee7ac578209ae3478adabe567e92a950
Summary: Pull Request resolved: facebook#41676 This has been used in a significant amount of production for about 2 months, with no consistently statistically significant metric impact. Let's ship it. Note that we would not want to keep this change in a holdout if we remove the warning, since new usages could be added that relied on the behavior not in the holdout. It didn't seem worth the churn to make the same change to Paper, which leaves a question on how to handle the JS-side warning. Instead of jimmying in impl detection, I thought it might be more sane to remove the warning, though that also has a potential hit to Paper DevX. Changelog: [iOS][Changed] - `scrollEventThrottle` no longer needs to be set for continuous scroll events when using the new architecture. Reviewed By: javache Differential Revision: D51608970 fbshipit-source-id: 193019de208f3088519e6f6333dbec4e6b45a1eb
…zation (facebook#41496) Summary: Pull Request resolved: facebook#41496 Refactoring `DefaultReactNativeHost` to use the new way of Fabric initialization through `FabricUIManagerProviderImpl` Reviewed By: christophpurrer Differential Revision: D51224854 fbshipit-source-id: 2af8021404365fa2adc9388f44bcc7c6301137dc
) Summary: Pull Request resolved: facebook#41669 In some previous changes ([a607692](facebook@a607692) and [6b53205](facebook@6b53205)) we make sure to always include all the pods (including Fabric) and we unify codegen to run in the same way on both architectures. While doing so, we enabled codegen to run on libraries tat already migrated to Fabric. These makes those libraries to fail when building as they were not including the Fabric code when the New Architecture is disabled. This change will make sue that the code is always included, thus the library should always build, and it also make sure that we can control the New/Old Architecture at build time. ## Changelog [iOS][Changed] - Make sure that libraries always include Fabric code also in the old architecture Reviewed By: dmytrorykun Differential Revision: D51617542 fbshipit-source-id: 883d1e258c341feb0405ad389bb8af34d64b59b8
Summary: Pull Request resolved: facebook#41698 With the previous changes on the Pod configuration, the build setup for the New and Old architecture are the same. The only observable difference happens at runtime. This change: 1. Removes the build job that are split by architecture (which is now duplicated work) 2. Add two more test jobs to run runtime tests (unit and integration test) to make sure that the two architectures continue working. ## Changelog: [Internal] - [CI] Remove duplicated build jobs, add tests jobs Reviewed By: cortinico Differential Revision: D51659275 fbshipit-source-id: 769c9ee004e7f4f1a7444f39c02b7083e007b780
…book#41678) Summary: This is my proposed solution to facebook#41677. Fixes facebook#41677. ## Changelog: [ANDROID] [FIXED] - Fix android root view group removal during instance re-creation Pull Request resolved: facebook#41678 Test Plan: Both with fabric enabled and disabled (new architecture): 1. Clone repro repo: https://github.com/wschurman/rn-reload-repro 2. Build and run on android (I use android studio) 3. Click reload button, see timestamp doesn't change (indicating that the view is not removed) 4. Apply this PR as a patch. 5. Re-build and run. 6. Click reload button, see view is correctly disposed of and the new view is set. Reviewed By: cortinico Differential Revision: D51658524 Pulled By: javache fbshipit-source-id: d9a026cde677ad1ec113230bc31bd9297bca8bfc
…book#41697) Summary: Currently, when we have an additional platform in `react-native.config.js`, users cannot use custom `resolver.resolveRequest` functions as they are overwritten by `reactNativePlatformResolver`. Goal of this PR is to allow OOT platforms to use additional custom resolvers besides remapping react native imports. ## Changelog: [GENERAL] [FIXED] - Allow Out Of Tree platforms to pass custom resolvers Pull Request resolved: facebook#41697 Test Plan: 1. Add additional platform in `react-native.config.js` 2. Pass custom resolver to `metro.config.js`: ```js resolveRequest: (context, moduleName, platform) => { console.log('resolveRequest', moduleName, platform); return context.resolveRequest(context, moduleName, platform); } ``` 3. Check if user's `resolveRequest` function is called. Reviewed By: huntie Differential Revision: D51659721 Pulled By: robhogan fbshipit-source-id: 952589b59a6fa34e9406d36c900be53a7c1a79c3
Summary: This PR converts to kotlin the java code for LayoutPropertyApplicatorTest, as requested in: facebook#38825 ## Changelog: [INTERNAL][CHANGED]: Convert LayoutPropertyApplicatorTest to Kotlin Pull Request resolved: facebook#41649 Test Plan: `./gradlew :packages:react-native:ReactAndroid:test ` Reviewed By: rshest Differential Revision: D51614921 Pulled By: cortinico fbshipit-source-id: 06ee403a496f34afc9abeabc0c406391e316538a
Summary: Pull Request resolved: facebook#41706 This change introduces a ReadME in the CircleCI folder. This can be used as documentation to learn more about our CircleCI setup and will also help GitHub employees in executing the migration. ## Changelog: [Internal] - Add CircleCI documentation Reviewed By: cortinico Differential Revision: D51665453 fbshipit-source-id: f61325ed26572c4a8d4a68db1cca5934d3d968fb
Summary: Pull Request resolved: facebook#41703 TSIA Changelog: [Internal] Reviewed By: javache Differential Revision: D51661861 fbshipit-source-id: 6fd5e253d9113f6acf7f4fe889f35f9aa580797e
Summary: Pull Request resolved: facebook#41708 RN-Tester is currently instacrashing due to a method accepting a `Float?` rather than a `Float`. `Float` from Kotlin gets converted to Java's `float`, while `Float?` gets converted to the boxed type, which is not recognized by the framework and is making the app crash. On top of this, the implementation of `setColor` was wrong as we don't properly handle the null case. Fixing it here as well. Changelog: [Internal] [Changed] - Fix broken RN Tester custom ViewManager Reviewed By: NickGerleman Differential Revision: D51667346 fbshipit-source-id: b7498a520936f81a0524ba53dc7230ad7ef57bf8
Summary: Pull Request resolved: facebook#41681 ## Rationale Make initHybrid static. So that the derived class can initialize the C++ part with constructor arguments. **Note:** This diff just applies the fix from D51550623. into CxxReactPackage. Changelog: [Internal] Reviewed By: christophpurrer Differential Revision: D51642219 fbshipit-source-id: 095e452e03848379288af960969789aa5e9c0542
Summary: Pull Request resolved: facebook#41695 When I went to update documentation, I kinda internalized how inconsistent the API is if we don't change iOS Paper. The potential for breaks is if an iOS-specific component ignores a warning, and uses `onScroll` without `scrollEventThrottle`, then relies on `onScroll` only being called once. It didn't seem like we hit this scenario in practice when migrating Fabric ComponentView behavior, and components will need to support it in new arch anyway, so this change takes the less conservative option of unifying the behavior everywhere. Changelog: [iOS][Changed] - scrollEventThrottle no longer needs to be set for continuous scroll events Reviewed By: cipolleschi Differential Revision: D51647202 fbshipit-source-id: e2a57f3501b9096e4033cb198bbc214d53e9913c
Summary: Pull Request resolved: facebook#41701 I did a hotfix for this logic in D51618512. This does a small refactor to improve the code (moving more shared code to the hook and avoiding creating a closure unnecessarily in every call to it). Changelog: [internal] Reviewed By: javache Differential Revision: D51660288 fbshipit-source-id: 472836840b19958402bd0de3e2c09c7cec004156
Summary: Pull Request resolved: facebook#41700 The type definition of `useMergeRefs` is incorrect, which forces all callsites to use `$FlowFixMe`. This fixes the definition and removes all the `$FlowFixMe`s caused by it. Changelog: [internal] Reviewed By: javache Differential Revision: D51660716 fbshipit-source-id: 4d4d3a72bdca8c409fd1dda59cc2c94113b024bb
…of Fabric initialization Differential Revision: D51224854 Original commit changeset: 2af802140436 Original Phabricator Diff: D51224854 fbshipit-source-id: 039337be7057c9625d4a6e53520a18cd5071813e
…2068) Summary: Pull Request resolved: facebook#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: facebook#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: facebook#42058 Getting rid of JSIModule from tests Changelog: [Internal] internal Reviewed By: christophpurrer Differential Revision: D50925102 fbshipit-source-id: bce1c9459ae2c4d690712e2c09a7e935fa8e4427
Summary: Pull Request resolved: facebook#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
…hain (facebook#42076) Summary: Pull Request resolved: facebook#42076 ## Changelog: [Internal] - In facebook#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: 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: facebook#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: facebook#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
…cebook#41802) Summary: Pull Request resolved: facebook#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: facebook#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
…ojects (facebook#40937) Summary: This PR contains the changes from facebook#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: facebook#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: 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: facebook#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
…info if possible (facebook#42079) Summary: This is a continuation of my [last PR](facebook#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: facebook#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: Pull Request resolved: facebook#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: See facebook#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: facebook#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: facebook#42081 Test Plan: N/A Reviewed By: cortinico, cipolleschi Differential Revision: D52435234 Pulled By: lunaleaps fbshipit-source-id: 67da029d2b637e3997c12c21fe2a9ab9bc344399
…acebook#42072) Summary: Pull Request resolved: facebook#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: The enums [UIBarStyleBlackOpaque](https://developer.apple.com/documentation/uikit/uibarstyle/uibarstyleblackopaque) and [UIBarStyleBlackTranslucent](https://developer.apple.com/documentation/uikit/uibarstyle/uibarstyleblacktranslucent) have been deprecated since iOS 13, already below React Native's minimum OS of iOS 13.4. Indeed, they are not available on visionOS and tvOS, making this a source of extra diffs. Rather than deprecate and remove those options, I noticed that we don't actually use that `RCTConvert` method in the core repo, and haven't since `0.58-stable` (presumably before the lean core effort). Let's just remove it, it's a conversion that should be easy enough to replicate elsewhere. However, removal is a breaking change, so let's deprecate it for one release (0.74) and remove it for the next one (0.75). For posterity, tracking deprecation with microsoft#2008 and removal with microsoft#2009 . ## Changelog: [IOS] [DEPRECATED] - Deprecate `[RCTConvert UIBarStyle:]` Pull Request resolved: facebook#42100 Test Plan: CI should pass Reviewed By: shwanton Differential Revision: D52458912 Pulled By: NickGerleman fbshipit-source-id: 5614b6624b9b929ba601ac976149b2002163ff54
Summary: Pull Request resolved: facebook#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: facebook#42061 For removal of JSIModule getting rid of the inheritance relationship b/w interfaces UIManager & JSIModule by directly defining `initialize()` and `invalidate()` Changelog: [Internal] internal Reviewed By: philIip, mdvacca Differential Revision: D49306312 fbshipit-source-id: 041870418e13bb4b2381e609b94331c87be5f6fa
Summary: Pull Request resolved: facebook#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
…cebook#42090) Summary: Pull Request resolved: facebook#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
|
I am looking forward to 4ccd6e1 to fix resolveRequest for OOT platforms. 🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please select one of the following
Summary:
TODO:
@react-native-mac/virtualized-listspackage`Merge up to January 2nd upstream. Namely, this brings some commits that will make visionOS integration easier.
Interesting commits:
UILocalNotificationand replaces with the cross-platform friendlyUNNotification. Many macOS diffs are removed as a result.Commit that help with visionOS integration (and therefore will be back ported to
0.73/0.72):UIMenuControllerwhich is no longer available on visionOS[UIScreen mainScreen]usage[UIScreen mainScreen]usageRCTKeyWindowChangelog:
[INTERNAL] [CHANGED] - Merge up to January 2nd upstream
Test Plan:
CI should pass