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 40b168372af..6598ba357f9 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 06e964845d3..ced1c4e908f 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 f1b25981b31..e20b751dda7 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 eb6d622917a..89d82d1eedb 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 c0065b72715..55bd24452b9 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm @@ -232,7 +232,7 @@ static NSDictionary *updateInitialProps(NSDictionary *initialProps, BOOL isFabri - (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 d115ef4e56b..a51df7ff58f 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 c7239e3f641..2fd12ff5b53 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 @@ class NewArchitectureTests < Test::Unit::TestCase 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 f37864d7d42..8b92268fb2c 100644 --- a/packages/react-native/scripts/cocoapods/codegen_utils.rb +++ b/packages/react-native/scripts/cocoapods/codegen_utils.rb @@ -72,7 +72,6 @@ class CodegenUtils 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 65fbe4b4d7f..95a1221deb8 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 "./utils.rb" 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 @@ class NewArchitectureHelper 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 @@ class NewArchitectureHelper 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 5944bb0aed7..db8fd014088 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"