From 0bd523955385a3b1e622077b7ee4ea0df3c5c158 Mon Sep 17 00:00:00 2001 From: Franco Meloni Date: Tue, 24 May 2022 04:37:46 -0700 Subject: [PATCH] Move `use_flipper` logic inside `use_react_native` and simplify the Flipper dependencies logic (#33882) Summary: This PR tries to simplify the `use_flipper` logic: - makes `use_flipper` a configuration inside `use_react_native`'s options - uses the already present `production` flag in the `use_react_native`'s options to decide if add or not the Flipper pods - Simplifies the logic to download the flipper dependencies This PR also adds a workaround for https://github.com/facebook/react-native/issues/33764 ## Changelog [iOS] [Changed] - Move `use_flipper` logic inside `use_react_native` and simplify the Flipper dependencies logic Pull Request resolved: https://github.com/facebook/react-native/pull/33882 Test Plan: Executed a pod install with and without flipper and with isProduction true Reviewed By: cipolleschi Differential Revision: D36592338 Pulled By: f-meloni fbshipit-source-id: 3c3f773151513e27e251f18865986e942a96ffd9 --- packages/rn-tester/Podfile | 5 ++-- scripts/cocoapods/__tests__/flipper-test.rb | 13 ++--------- scripts/cocoapods/flipper.rb | 26 +++++++++++++++++---- scripts/react_native_pods.rb | 11 +++++++-- template/ios/Podfile | 11 ++++----- 5 files changed, 40 insertions(+), 26 deletions(-) diff --git a/packages/rn-tester/Podfile b/packages/rn-tester/Podfile index 91811ab85812e5..116fdb9e4706bd 100644 --- a/packages/rn-tester/Podfile +++ b/packages/rn-tester/Podfile @@ -32,8 +32,10 @@ def pods(options = {}) path: @prefix_path, fabric_enabled: fabric_enabled, hermes_enabled: hermes_enabled, + flipper_configuration: USE_FRAMEWORKS ? FlipperConfiguration.disabled : FlipperConfiguration.enabled, app_path: "#{Dir.pwd}", config_file_dir: "#{Dir.pwd}/node_modules", + production: !ENV['PRODUCTION'].nil? ) pod 'ReactCommon/turbomodule/samples', :path => "#{@prefix_path}/ReactCommon" @@ -48,9 +50,6 @@ end target 'RNTester' do pods() - if !USE_FRAMEWORKS - use_flipper! - end end target 'RNTesterUnitTests' do diff --git a/scripts/cocoapods/__tests__/flipper-test.rb b/scripts/cocoapods/__tests__/flipper-test.rb index 300d80406b488c..60961b8050f0a6 100644 --- a/scripts/cocoapods/__tests__/flipper-test.rb +++ b/scripts/cocoapods/__tests__/flipper-test.rb @@ -16,24 +16,15 @@ def setup # =========================== # # TEST - Install Dependencies # # =========================== # - def test_installFlipperDependencies_whenProductionIsFalse_installDependencies + def test_installFlipperDependencies_installDependencies # Act - install_flipper_dependencies(false, '../..') + install_flipper_dependencies('../..') # Assert assert_equal($podInvocationCount, 1) assert_equal($podInvocation['React-Core/DevSupport'][:path], "../../" ) end - def test_installFlipperDependencies_whenProductionIsTrue_skipDependencies - # Act - install_flipper_dependencies(true, '../..') - - # Assert - assert_equal($podInvocationCount, 0) - assert_true($podInvocation.empty?) - end - # ======================= # # TEST - Use Flipper Pods # # ======================= # diff --git a/scripts/cocoapods/flipper.rb b/scripts/cocoapods/flipper.rb index 798eb0f0457d12..965a20060b0ac3 100644 --- a/scripts/cocoapods/flipper.rb +++ b/scripts/cocoapods/flipper.rb @@ -22,10 +22,8 @@ # # @parameter production: a boolean that indicates whether we are in production or not. # @parameter pathToReactNative: the path to the React Native installation -def install_flipper_dependencies(production, pathToReactNative) - unless production - pod 'React-Core/DevSupport', :path => "#{pathToReactNative}/" - end +def install_flipper_dependencies(pathToReactNative) + pod 'React-Core/DevSupport', :path => "#{pathToReactNative}/" end @@ -92,3 +90,23 @@ def flipper_post_install(installer) end end end + +class FlipperConfiguration + attr_reader :flipper_enabled + attr_reader :configurations + attr_reader :versions + + def initialize(flipper_enabled, configurations, versions) + @flipper_enabled = flipper_enabled + @configurations = configurations + @versions = versions + end + + def self.enabled(configurations = ["Debug"], versions = {}) + return FlipperConfiguration.new(true, configurations, versions) + end + + def self.disabled + return FlipperConfiguration.new(false, [], {}) + end +end diff --git a/scripts/react_native_pods.rb b/scripts/react_native_pods.rb index a0e16d4895b171..28d339a23cbdfb 100644 --- a/scripts/react_native_pods.rb +++ b/scripts/react_native_pods.rb @@ -36,6 +36,8 @@ def use_react_native! (options={}) # Include Hermes dependencies hermes_enabled = options[:hermes_enabled] ||= false + flipper_configuration = options[:flipper_configuration] ||= FlipperConfiguration.disabled + if `/usr/sbin/sysctl -n hw.optional.arm64 2>&1`.to_i == 1 && !RUBY_PLATFORM.include?('arm64') Pod::UI.warn 'Do not use "pod install" from inside Rosetta2 (x86_64 emulation on arm64).' Pod::UI.warn ' - Emulated x86_64 is slower than native arm64' @@ -62,8 +64,6 @@ def use_react_native! (options={}) pod 'React-RCTVibration', :path => "#{prefix}/Libraries/Vibration" pod 'React-Core/RCTWebSocket', :path => "#{prefix}/" - install_flipper_dependencies(production, prefix) - pod 'React-bridging', :path => "#{prefix}/ReactCommon/react/bridging" pod 'React-cxxreact', :path => "#{prefix}/ReactCommon/cxxreact" pod 'React-jsi', :path => "#{prefix}/ReactCommon/jsi" @@ -128,6 +128,11 @@ def use_react_native! (options={}) end + if flipper_configuration.flipper_enabled && !production + install_flipper_dependencies(prefix) + use_flipper_pods(flipper_configuration.versions, :configurations => flipper_configuration.configurations) + end + pods_to_update = LocalPodspecPatch.pods_to_update(options) if !pods_to_update.empty? if Pod::Lockfile.public_instance_methods.include?(:detect_changes_with_podfile) @@ -142,6 +147,7 @@ def get_default_flags() flags = { :fabric_enabled => false, :hermes_enabled => false, + :flipper_configuration => FlipperConfiguration.disabled } if ENV['RCT_NEW_ARCH_ENABLED'] == '1' @@ -153,6 +159,7 @@ def get_default_flags() end def use_flipper!(versions = {}, configurations: ['Debug']) + Pod::UI.warn "use_flipper is deprecated, use the flipper_configuration option in the use_react_native function" use_flipper_pods(versions, :configurations => configurations) end diff --git a/template/ios/Podfile b/template/ios/Podfile index c814edb8d9d019..938146d23e839b 100644 --- a/template/ios/Podfile +++ b/template/ios/Podfile @@ -16,6 +16,11 @@ target 'HelloWorld' do # You can enabled/disable it manually by replacing `flags[:hermes_enabled]` with `true` or `false`. :hermes_enabled => flags[:hermes_enabled], :fabric_enabled => flags[:fabric_enabled], + # Enables Flipper. + # + # Note that if you have use_frameworks! enabled, Flipper will not work and + # you should disable the next line. + :flipper_configuration => FlipperConfiguration.enabled, # An absolute path to your application root. :app_path => "#{Pod::Config.instance.installation_root}/.." ) @@ -25,12 +30,6 @@ target 'HelloWorld' do # Pods for testing end - # Enables Flipper. - # - # Note that if you have use_frameworks! enabled, Flipper will not work and - # you should disable the next line. - use_flipper!() - post_install do |installer| react_native_post_install(installer) __apply_Xcode_12_5_M1_post_install_workaround(installer)