From 3690e4413be0459b2e5fc42ca266c1a03f3c04fa Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Fri, 12 Jan 2024 08:10:35 -0800 Subject: [PATCH] Restore RCT_NEW_ARCH_ENABLED semantics for backward compatibility (#42259) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/42259 At the end of last year, we reduce build fragmentation in iOS making sure that we were always building both architecture. In the process, we break the semantic od RCt_NEW_ARCH_ENABLED flag, making several libs stop working in one of the two archs. This change should restore the semantic, so libraries that were using RCT_NEW_ARCH_ENABLED to run conditional code will still work in the same way. While doing so, I also removed the new USE_NEW_ARCH as we don't want unnecessary flags ## CHANGELOG: [iOS][Fixed] - Bring the old RCT_NEW_ARCH_ENABLED semantic back for compatibility Reviewed By: cortinico Differential Revision: D52727792 fbshipit-source-id: e211b10e7885eada83dd2886375575133ca76c8c --- ...rateThirdPartyFabricComponentsProviderH.js | 3 ++- ...hirdPartyFabricComponentsProviderObjCpp.js | 2 ++ ...artyFabricComponentsProviderH-test.js.snap | 3 ++- ...abricComponentsProviderObjCpp-test.js.snap | 2 ++ .../Libraries/AppDelegate/RCTAppDelegate.mm | 2 +- .../AppDelegate/React-RCTAppDelegate.podspec | 6 ++--- .../__tests__/new_architecture-test.rb | 12 +++++----- .../scripts/cocoapods/codegen_utils.rb | 1 - .../scripts/cocoapods/new_architecture.rb | 23 +++++++++---------- .../react-native/scripts/react_native_pods.rb | 3 +-- 10 files changed, 30 insertions(+), 27 deletions(-) diff --git a/packages/react-native-codegen/src/generators/components/GenerateThirdPartyFabricComponentsProviderH.js b/packages/react-native-codegen/src/generators/components/GenerateThirdPartyFabricComponentsProviderH.js index 40b168372af7c1..6598ba357f99b0 100644 --- a/packages/react-native-codegen/src/generators/components/GenerateThirdPartyFabricComponentsProviderH.js +++ b/packages/react-native-codegen/src/generators/components/GenerateThirdPartyFabricComponentsProviderH.js @@ -35,11 +35,12 @@ extern "C" { #endif Class RCTThirdPartyFabricComponentsProvider(const char *name); - +#if RCT_NEW_ARCH_ENABLED #ifndef RCT_DYNAMIC_FRAMEWORKS ${lookupFuncs} +#endif #endif #ifdef __cplusplus diff --git a/packages/react-native-codegen/src/generators/components/GenerateThirdPartyFabricComponentsProviderObjCpp.js b/packages/react-native-codegen/src/generators/components/GenerateThirdPartyFabricComponentsProviderObjCpp.js index 06e964845d3f52..ced1c4e908f39f 100644 --- a/packages/react-native-codegen/src/generators/components/GenerateThirdPartyFabricComponentsProviderObjCpp.js +++ b/packages/react-native-codegen/src/generators/components/GenerateThirdPartyFabricComponentsProviderObjCpp.js @@ -34,9 +34,11 @@ const FileTemplate = ({lookupMap}: {lookupMap: string}) => ` Class RCTThirdPartyFabricComponentsProvider(const char *name) { static std::unordered_map sFabricComponentsClassMap = { + #if RCT_NEW_ARCH_ENABLED #ifndef RCT_DYNAMIC_FRAMEWORKS ${lookupMap} #endif + #endif }; auto p = sFabricComponentsClassMap.find(name); diff --git a/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateThirdPartyFabricComponentsProviderH-test.js.snap b/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateThirdPartyFabricComponentsProviderH-test.js.snap index f1b25981b31701..e20b751dda7807 100644 --- a/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateThirdPartyFabricComponentsProviderH-test.js.snap +++ b/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateThirdPartyFabricComponentsProviderH-test.js.snap @@ -22,7 +22,7 @@ extern \\"C\\" { #endif Class RCTThirdPartyFabricComponentsProvider(const char *name); - +#if RCT_NEW_ARCH_ENABLED #ifndef RCT_DYNAMIC_FRAMEWORKS Class NoPropsNoEventsComponentCls(void) __attribute__((used)); // NO_PROPS_NO_EVENTS @@ -57,6 +57,7 @@ Class ExcludedAndroidComponentCls(void) __attribute__( Class MultiFileIncludedNativeComponentCls(void) __attribute__((used)); // EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES +#endif #endif #ifdef __cplusplus diff --git a/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateThirdPartyFabricComponentsProviderObjCpp-test.js.snap b/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateThirdPartyFabricComponentsProviderObjCpp-test.js.snap index eb6d622917a2d0..89d82d1eedb55d 100644 --- a/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateThirdPartyFabricComponentsProviderObjCpp-test.js.snap +++ b/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GenerateThirdPartyFabricComponentsProviderObjCpp-test.js.snap @@ -21,6 +21,7 @@ Map { Class RCTThirdPartyFabricComponentsProvider(const char *name) { static std::unordered_map sFabricComponentsClassMap = { + #if RCT_NEW_ARCH_ENABLED #ifndef RCT_DYNAMIC_FRAMEWORKS {\\"NoPropsNoEventsComponent\\", NoPropsNoEventsComponentCls}, // NO_PROPS_NO_EVENTS @@ -82,6 +83,7 @@ Class RCTThirdPartyFabricComponentsProvider(const char {\\"MultiFileIncludedNativeComponent\\", MultiFileIncludedNativeComponentCls}, // EXCLUDE_IOS_TWO_COMPONENTS_DIFFERENT_FILES #endif + #endif }; auto p = sFabricComponentsClassMap.find(name); diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm index c0065b7271535f..55bd24452b9b05 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm @@ -232,7 +232,7 @@ - (void)windowScene:(UIWindowScene *)windowScene - (BOOL)newArchEnabled { -#if USE_NEW_ARCH +#if RCT_NEW_ARCH_ENABLED return YES; #else return NO; diff --git a/packages/react-native/Libraries/AppDelegate/React-RCTAppDelegate.podspec b/packages/react-native/Libraries/AppDelegate/React-RCTAppDelegate.podspec index d115ef4e56b4a8..a51df7ff58f9d8 100644 --- a/packages/react-native/Libraries/AppDelegate/React-RCTAppDelegate.podspec +++ b/packages/react-native/Libraries/AppDelegate/React-RCTAppDelegate.podspec @@ -19,11 +19,11 @@ end folly_flags = ' -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1 -DFOLLY_CFG_NO_COROUTINES=1 -DFOLLY_HAVE_CLOCK_GETTIME=1' folly_compiler_flags = folly_flags + ' ' + '-Wno-comma -Wno-shorten-64-to-32' -is_new_arch_enabled = ENV["USE_NEW_ARCH"] == "1" +is_new_arch_enabled = ENV["RCT_NEW_ARCH_ENABLED"] == "1" use_hermes = ENV['USE_HERMES'] == nil || ENV['USE_HERMES'] == '1' -new_arch_enabled_flag = (is_new_arch_enabled ? " -DUSE_NEW_ARCH" : "") -is_fabric_enabled = is_new_arch_enabled || ENV["RCT_FABRIC_ENABLED"] +new_arch_enabled_flag = (is_new_arch_enabled ? " -DRCT_NEW_ARCH_ENABLED" : "") +is_fabric_enabled = true #is_new_arch_enabled || ENV["RCT_FABRIC_ENABLED"] hermes_flag = (use_hermes ? " -DUSE_HERMES" : "") other_cflags = "$(inherited)" + folly_flags + new_arch_enabled_flag + hermes_flag diff --git a/packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb b/packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb index c7239e3f6417bd..2fd12ff5b535eb 100644 --- a/packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb +++ b/packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb @@ -87,12 +87,12 @@ def test_modifyFlagsForNewArch_whenOnOldArch_doNothing NewArchitectureHelper.modify_flags_for_new_architecture(installer, false) # Assert - assert_equal(first_xcconfig.attributes["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") - assert_equal(first_xcconfig.save_as_invocation, []) - assert_equal(second_xcconfig.attributes["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") - assert_equal(second_xcconfig.save_as_invocation, []) - assert_equal(react_core_debug_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") - assert_equal(react_core_release_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") + assert_equal(first_xcconfig.attributes["OTHER_CPLUSPLUSFLAGS"], "$(inherited) -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1 -DFOLLY_CFG_NO_COROUTINES=1 -DFOLLY_HAVE_CLOCK_GETTIME=1 -Wno-comma -Wno-shorten-64-to-32") + assert_equal(first_xcconfig.save_as_invocation, ["a/path/First.xcconfig"]) + assert_equal(second_xcconfig.attributes["OTHER_CPLUSPLUSFLAGS"], "$(inherited) -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1 -DFOLLY_CFG_NO_COROUTINES=1 -DFOLLY_HAVE_CLOCK_GETTIME=1 -Wno-comma -Wno-shorten-64-to-32") + assert_equal(second_xcconfig.save_as_invocation, ["a/path/Second.xcconfig"]) + assert_equal(react_core_debug_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited) -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1 -DFOLLY_CFG_NO_COROUTINES=1 -DFOLLY_HAVE_CLOCK_GETTIME=1 -Wno-comma -Wno-shorten-64-to-32") + assert_equal(react_core_release_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited) -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1 -DFOLLY_CFG_NO_COROUTINES=1 -DFOLLY_HAVE_CLOCK_GETTIME=1 -Wno-comma -Wno-shorten-64-to-32") assert_equal(yoga_debug_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") assert_equal(yoga_release_config.build_settings["OTHER_CPLUSPLUSFLAGS"], "$(inherited)") end diff --git a/packages/react-native/scripts/cocoapods/codegen_utils.rb b/packages/react-native/scripts/cocoapods/codegen_utils.rb index f37864d7d42293..8b92268fb2c8d3 100644 --- a/packages/react-native/scripts/cocoapods/codegen_utils.rb +++ b/packages/react-native/scripts/cocoapods/codegen_utils.rb @@ -72,7 +72,6 @@ def generate_react_codegen_podspec!(spec, codegen_output_dir, file_manager: File def get_react_codegen_spec(package_json_file, folly_version: get_folly_config()[:version], hermes_enabled: true, script_phases: nil, file_manager: File) package = JSON.parse(file_manager.read(package_json_file)) version = package['version'] - new_arch_disabled = ENV['RCT_NEW_ARCH_ENABLED'] != "1" use_frameworks = ENV['USE_FRAMEWORKS'] != nil folly_compiler_flags = '-DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1 -DFOLLY_CFG_NO_COROUTINES=1 -DFOLLY_HAVE_CLOCK_GETTIME=1 -Wno-comma -Wno-shorten-64-to-32' boost_compiler_flags = '-Wno-documentation' diff --git a/packages/react-native/scripts/cocoapods/new_architecture.rb b/packages/react-native/scripts/cocoapods/new_architecture.rb index 65fbe4b4d7f22f..95a1221deb827c 100644 --- a/packages/react-native/scripts/cocoapods/new_architecture.rb +++ b/packages/react-native/scripts/cocoapods/new_architecture.rb @@ -9,8 +9,6 @@ require_relative "./helpers.rb" \ class NewArchitectureHelper - @@new_arch_cpp_flags = " -DRCT_NEW_ARCH_ENABLED=1 #{Helpers::Constants.folly_config()[:compiler_flags]}" - @@cplusplus_version = "c++20" @@NewArchWarningEmitted = false # Used not to spam warnings to the user. @@ -42,26 +40,28 @@ def self.set_clang_cxx_language_standard_if_needed(installer) end end + def self.computeFlags(is_new_arch_enabled) + new_arch_flag = is_new_arch_enabled ? "-DRCT_NEW_ARCH_ENABLED=1 " : "" + return " #{new_arch_flag}#{Helpers::Constants.folly_config()[:compiler_flags]}" + end + def self.modify_flags_for_new_architecture(installer, is_new_arch_enabled) - unless is_new_arch_enabled - return - end - # Add RCT_NEW_ARCH_ENABLED to Target pods xcconfig + # Add flags to Target pods xcconfig installer.aggregate_targets.each do |aggregate_target| aggregate_target.xcconfigs.each do |config_name, config_file| - ReactNativePodsUtils.add_flag_to_map_with_inheritance(config_file.attributes, "OTHER_CPLUSPLUSFLAGS", @@new_arch_cpp_flags) + ReactNativePodsUtils.add_flag_to_map_with_inheritance(config_file.attributes, "OTHER_CPLUSPLUSFLAGS", self.computeFlags(is_new_arch_enabled)) xcconfig_path = aggregate_target.xcconfig_path(config_name) config_file.save_as(xcconfig_path) end end - # Add RCT_NEW_ARCH_ENABLED to generated pod target projects + # Add flags to Target pods xcconfig installer.target_installation_results.pod_target_installation_results.each do |pod_name, target_installation_result| # The React-Core pod may have a suffix added by Cocoapods, so we test whether 'React-Core' is a substring, and do not require exact match if pod_name.include? 'React-Core' target_installation_result.native_target.build_configurations.each do |config| - ReactNativePodsUtils.add_flag_to_map_with_inheritance(config.build_settings, "OTHER_CPLUSPLUSFLAGS", @@new_arch_cpp_flags) + ReactNativePodsUtils.add_flag_to_map_with_inheritance(config.build_settings, "OTHER_CPLUSPLUSFLAGS", self.computeFlags(is_new_arch_enabled)) end end end @@ -109,9 +109,8 @@ def self.install_modules_dependencies(spec, new_arch_enabled, folly_version = ge spec.dependency "RCT-Folly", folly_version spec.dependency "glog" - if new_arch_enabled - ReactNativePodsUtils.add_flag_to_map_with_inheritance(current_config, "OTHER_CPLUSPLUSFLAGS", @@new_arch_cpp_flags) - end + + ReactNativePodsUtils.add_flag_to_map_with_inheritance(current_config, "OTHER_CPLUSPLUSFLAGS", self.computeFlags(new_arch_enabled)) spec.dependency "React-RCTFabric" # This is for Fabric Component spec.dependency "React-Codegen" diff --git a/packages/react-native/scripts/react_native_pods.rb b/packages/react-native/scripts/react_native_pods.rb index 5944bb0aed73de..db8fd0140883c6 100644 --- a/packages/react-native/scripts/react_native_pods.rb +++ b/packages/react-native/scripts/react_native_pods.rb @@ -90,10 +90,9 @@ def use_react_native! ( # Better to rely and enable this environment flag if the new architecture is turned on using flags. relative_path_from_current = Pod::Config.instance.installation_root.relative_path_from(Pathname.pwd) react_native_version = NewArchitectureHelper.extract_react_native_version(File.join(relative_path_from_current, path)) - ENV['USE_NEW_ARCH'] = NewArchitectureHelper.compute_new_arch_enabled(new_arch_enabled, react_native_version) + ENV['RCT_NEW_ARCH_ENABLED'] = NewArchitectureHelper.compute_new_arch_enabled(new_arch_enabled, react_native_version) fabric_enabled = fabric_enabled || NewArchitectureHelper.new_arch_enabled - ENV['RCT_NEW_ARCH_ENABLED'] = "1" ENV['RCT_FABRIC_ENABLED'] = fabric_enabled ? "1" : "0" ENV['USE_HERMES'] = hermes_enabled ? "1" : "0"