upgrade-support: Flipper shouldn't be included in Release builds

Environment

npx react-native info
info Fetching system and libraries information...
System:
    OS: macOS Mojave 10.14.5
    CPU: (4) x64 Intel(R) Core(TM) i7-6567U CPU @ 3.30GHz
    Memory: 270.57 MB / 16.00 GB
    Shell: 5.3 - /bin/zsh
  Binaries:
    Node: 13.12.0 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.4 - /usr/local/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.8.4 - /Users/javi/.rvm/gems/ruby-2.5.3/bin/pod
  SDKs:
    iOS SDK:
      Platforms: iOS 13.1, DriverKit 19.0, macOS 10.15, tvOS 13.0, watchOS 6.0
    Android SDK:
      API Levels: 23, 25, 26, 28
      Build Tools: 27.0.3, 28.0.2, 28.0.3, 29.0.0
      System Images: android-28 | Google Play Intel x86 Atom, android-29 | Google Play Intel x86 Atom
      Android NDK: Not Found
  IDEs:
    Android Studio: 3.5 AI-191.8026.42.35.5900203
    Xcode: 11.1/11A1027 - /usr/bin/xcodebuild
  Languages:
    Python: 2.7.16 - /usr/local/bin/python
  npmPackages:
    @react-native-community/cli: Not Found
    react: 16.11.0 => 16.11.0
    react-native: 0.62.0 => 0.62.0
  npmGlobalPackages:
    *react-native*: Not Found

Upgrading version

From 0.61.3 to 0.62.0

Description

After upgrading to 0.62, when we created our first iOS Release build, we noticed some warnings related to Flipper’s pods such as:

⚠️  [redacted]]ios/Pods/Headers/Public/Flipper-Folly/folly/Memory.h:66:30: possible misuse of comma operator here [-Wcomma]
  return rc == 0 ? (errno = 0, ptr) : (errno = rc, nullptr);
    ^~~~~
⚠️  [redacted]]ios/Pods/Headers/Public/Flipper-Folly/folly/Memory.h:66:50: possible misuse of comma operator here [-Wcomma]
  return rc == 0 ? (errno = 0, ptr) : (errno = rc, nullptr);
              ^
⚠️  [redacted]]ios/Pods/Headers/Public/Flipper-Folly/folly/ScopeGuard.h:125:52: possible misuse of comma operator here [-Wcomma]
      auto catcher = []() -> R { warnAboutToCrash(), std::terminate(); };
              ^
⚠️  [redacted]]ios/Pods/Headers/Public/Flipper-Folly/folly/FBString.h:1879:72: possible misuse of comma operator here [-Wcomma]
                  "basic_fbstring: null pointer initializer not valid"),
                                 ^~~~~~~~~~~~~~~~~~
⚠️  [redacted]]ios/Pods/Headers/Public/Flipper-Folly/folly/Conv.h:1184:44: possible misuse of comma operator here [-Wcomma]
      [&](Tgt res) { return void(out = res), src; });
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Building library libFlipperKit.a

This made us wonder why is Flipper included in a build using the Release mode configuration. We’ve seen that the new Podfile template only includes the FlipperKit pods for the Debug build configuration:

def add_flipper_pods!
  version = '~> 0.33.1'
  pod 'FlipperKit', version, :configuration => 'Debug'
  pod 'FlipperKit/FlipperKitLayoutPlugin', version, :configuration => 'Debug'
  pod 'FlipperKit/SKIOSNetworkPlugin', version, :configuration => 'Debug'
  pod 'FlipperKit/FlipperKitUserDefaultsPlugin', version, :configuration => 'Debug'
  pod 'FlipperKit/FlipperKitReactPlugin', version, :configuration => 'Debug'
end

However the Cocoapods’ docs notes that:

Note that transitive dependencies are included in all configurations and you have to manually specify build configurations for them as well in case this is not desired.

The pod Flipper-Folly is a dependency of Flipper, and Flipper seems to be dependency of FlipperKit/Core and FlipperKit/CppBridge.

Is this why it’s being built and linked on Release build? Should add_flipper_pods! specify the Flipper and Flipper-Folly pods so that they get excluded from Release builds?

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 17 (5 by maintainers)

Commits related to this issue

Most upvoted comments

Definitely shouldn’t be included in release builds. We exclude it in Android release builds, too.

@passy do you mean it’s already excluded for Android in the templates from RN 0.62 or that your team has excluded them for Android (making additional changes)?

@alloy I’ll give it a try with a fresh project. I tried once yesterday but Flipper was still being included, but I suspect is because we had react-native-flipper and rn-redux-middleware-flipper in our package.json (and they may depend on Flipper’s pod too).

@javiercr Already excluded in 0.62. 😃

I thought that we had excluded this from release builds, but the only thing I can find is an open PR for Andorid. cc @passy @alloy https://github.com/facebook/react-native/pull/28257

To readers needing immediate support, make the following changes to your Podfile: https://github.com/facebook/react-native/pull/28504/commits/1c007d553cc71884952def921a15e7d088084854

Is this why it’s being built and linked on Release build? Should add_flipper_pods! specify the Flipper and Flipper-Folly pods so that they get excluded from Release builds?

@javiercr Yup, looks like it, good catch. Mind sending a PR after verifying those changes work for you?