Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DISABLE_FLIPPER=1 breaks CI #18

Closed
rlam3 opened this issue Aug 22, 2022 · 8 comments
Closed

DISABLE_FLIPPER=1 breaks CI #18

rlam3 opened this issue Aug 22, 2022 · 8 comments
Labels
bug Something isn't working SDK 46 Affects plugin in Expo SDK 46

Comments

@rlam3
Copy link
Contributor

rlam3 commented Aug 22, 2022

Setting export FLIPPER_DISABLE=1 breaks eas build

Hey @jakobo, when I set my environmental variable to be disabled in as an expo secret and CI and I was hit with the following error:

It seems as though when we set DISABLE_FLIPPER=1, react-native-flipper is not disabled and is looking for Flipper plugin as well. Is there a way to tell cocoapods to not install react-native-flipper?

Thanks!

[RUN_FASTLANE] › Compiling react-native-flipper Pods/react-native-flipper » react-native-flipper-dummy.m
[RUN_FASTLANE] › Compiling react-native-flipper Pods/react-native-flipper » FlipperReactNativeJavaScriptPluginManager.m
[RUN_FASTLANE]
❌  fatal error: 'FlipperKit/FlipperClient.h' file not found


[RUN_FASTLANE] › Compiling react-native-flipper Pods/react-native-flipper » FlipperReactNativeJavaScriptPlugin.m
[RUN_FASTLANE]
❌  (node_modules/react-native-flipper/ios/FlipperReactNativeJavaScriptPlugin.h:9:9)

   7 |
   8 | #if DEBUG
>  9 | #import <FlipperKit/FlipperConnection.h>
     |         ^ 'FlipperKit/FlipperConnection.h' file not found
  10 | #import <FlipperKit/FlipperPlugin.h>
  11 |
  12 | NS_ASSUME_NONNULL_BEGIN

@jakobo
Copy link
Owner

jakobo commented Aug 22, 2022

Hey, can you run expo prebuild and post your Podfile contents + expo SDK version?

@jakobo jakobo added the investigating Ongoing conversation about the issue label Aug 22, 2022
@rlam3
Copy link
Contributor Author

rlam3 commented Aug 23, 2022

The following is the Podfile after I ran expo prebuild --clean -p ios

// package.json 
...
    "expo": "^46.0.0", 
    "expo-community-flipper": "^46.0.0",
    "react-native-flipper": "^0.161.0",

...
# Podfile
require File.join(File.dirname(`node --print "require.resolve('expo/package.json')"`), "scripts/autolinking")
require File.join(File.dirname(`node --print "require.resolve('react-native/package.json')"`), "scripts/react_native_pods")
require File.join(File.dirname(`node --print "require.resolve('@react-native-community/cli-platform-ios/package.json')"`), "native_modules")

require 'json'
podfile_properties = JSON.parse(File.read(File.join(__dir__, 'Podfile.properties.json'))) rescue {}

platform :ios, podfile_properties['ios.deploymentTarget'] || '12.4'
install! 'cocoapods',
  :deterministic_uuids => false

target 'XXX' do
  use_expo_modules!
  config = use_native_modules!

  use_frameworks! :linkage => podfile_properties['ios.useFrameworks'].to_sym if podfile_properties['ios.useFrameworks']

  # Flags change depending on the env values.
  flags = get_default_flags()

  use_react_native!(
    :path => config[:reactNativePath],
    :hermes_enabled => flags[:hermes_enabled] || podfile_properties['expo.jsEngine'] == 'hermes',
    :fabric_enabled => flags[:fabric_enabled],
    # An absolute path to your application root.
    :app_path => "#{Dir.pwd}/.."
  )
# @generated begin expo-community-flipper - expo prebuild (DO NOT MODIFY) sync-eaa63d5375e7930ea096abf972908f639babe2c9

      # Flipper support successfully added via expo config plugin
      # https://www.npmjs.com/package/expo-community-flipper
      if !ENV['FLIPPER_DISABLE']
        use_flipper!()
      end
    
# @generated end expo-community-flipper

  # Uncomment to opt-in to using Flipper
  # Note that if you have use_frameworks! enabled, Flipper will not work
  #
  # if !ENV['CI']
  #   use_flipper!()
  # end

  post_install do |installer|
# @generated begin expo-community-flipper-post-install - expo prebuild (DO NOT MODIFY) sync-616283d65c598f00dc6cc76cb3079af422438f93

      # Flipper support successfully added via expo config plugin
      # https://www.npmjs.com/package/expo-community-flipper
      if !ENV['FLIPPER_DISABLE']
        flipper_post_install(installer)
      end
    
# @generated end expo-community-flipper-post-install
    react_native_post_install(installer)
    __apply_Xcode_12_5_M1_post_install_workaround(installer)
  end

  post_integrate do |installer|
    begin
      expo_patch_react_imports!(installer)
    rescue => e
      Pod::UI.warn e
    end
  end

end

@jakobo
Copy link
Owner

jakobo commented Aug 23, 2022

tl;dr: Definitely a bug, the kind we would have never seen because the backwards incompatible change was in a patch release of React Native.


So, here's what I've learned

  • In RN 0.69.2, flipper init was changed in a backwards incompatible way: https://react-native-community.github.io/upgrade-helper/?from=0.69.1&to=0.69.2 I'm currently sobbing at semver's grave because there's no way we would have caught this breaking change in their patch release. (For reference, expo uses react native 0.69.4 which has this problem)
  • The plugin for SDK 46 needs to be rewritten on the Podfile side to change the use_react_native call and drop the use_flipper and postinstall calls we relied on for SDK 45.
  • The rewrite can pin to the :fabric_enabled => flags[:fabric_enabled], line in the podfile
  • An additional line not included by expo production = ENV["PRODUCTION"] == "1" is likely needed

Full Changelog from 0.68 to Expo's RN Version: https://react-native-community.github.io/upgrade-helper/?from=0.68.3&to=0.69.4

@jakobo jakobo added bug Something isn't working SDK 46 Affects plugin in Expo SDK 46 and removed investigating Ongoing conversation about the issue labels Aug 23, 2022
@jakobo
Copy link
Owner

jakobo commented Aug 23, 2022

@rlam3 there's a PR worth testing at #20 which should hopefully fix the CI issue. You can either use the PR contents, or the below zip w/ a build of the withFlipper plugin. Extract into your expo app under a subfolder, and point your plugin to the withFlipper.js file.

pr20-build.zip

@rlam3
Copy link
Contributor Author

rlam3 commented Aug 23, 2022

@jakobo here are my results

I ran the following:
export FLIPPER_DISABLE=1; expo prebuild --platform ios on my local machine

the new podfile is different from before. however when i startup the xcode workspace the podfile that seems to still be having trouble is still react-native-flipper for some odd reason

Screen Shot 2022-08-23 at 9 27 04 AM

#Podfile
require File.join(File.dirname(`node --print "require.resolve('expo/package.json')"`), "scripts/autolinking")
require File.join(File.dirname(`node --print "require.resolve('react-native/package.json')"`), "scripts/react_native_pods")
require File.join(File.dirname(`node --print "require.resolve('@react-native-community/cli-platform-ios/package.json')"`), "native_modules")

require 'json'
podfile_properties = JSON.parse(File.read(File.join(__dir__, 'Podfile.properties.json'))) rescue {}

platform :ios, podfile_properties['ios.deploymentTarget'] || '12.4'
install! 'cocoapods',
  :deterministic_uuids => false

target 'XXX' do
  use_expo_modules!
  config = use_native_modules!

  use_frameworks! :linkage => podfile_properties['ios.useFrameworks'].to_sym if podfile_properties['ios.useFrameworks']

  # Flags change depending on the env values.
  flags = get_default_flags()

  use_react_native!(
    :path => config[:reactNativePath],
# @generated begin expo-community-flipper-isprod - expo prebuild (DO NOT MODIFY) sync-e002ec08de24fbbe5b9a843ae9231b9627b8b9c4

            # https://www.npmjs.com/package/expo-community-flipper
            :production => ENV["PRODUCTION"] == "1" ? true : false,
# @generated end expo-community-flipper-isprod
    :hermes_enabled => flags[:hermes_enabled] || podfile_properties['expo.jsEngine'] == 'hermes',
    :fabric_enabled => flags[:fabric_enabled],
# @generated begin expo-community-flipper-urn - expo prebuild (DO NOT MODIFY) sync-4e2a82fdf3281fe0e3d2b3cffce0e78c985bced0

            # https://www.npmjs.com/package/expo-community-flipper
            :flipper_configuration => ENV['FLIPPER_DISABLE'] ? FlipperConfiguration.disabled : FlipperConfiguration.enabled,
# @generated end expo-community-flipper-urn
    # An absolute path to your application root.
    :app_path => "#{Dir.pwd}/.."
  )

  # Uncomment to opt-in to using Flipper
  # Note that if you have use_frameworks! enabled, Flipper will not work
  #
  # if !ENV['CI']
  #   use_flipper!()
  # end

  post_install do |installer|
    react_native_post_install(installer)
    __apply_Xcode_12_5_M1_post_install_workaround(installer)
  end

  post_integrate do |installer|
    begin
      expo_patch_react_imports!(installer)
    rescue => e
      Pod::UI.warn e
    end
  end

end

@jakobo
Copy link
Owner

jakobo commented Aug 23, 2022

Alright, the Podfile now looks correct matching Flipper's podfile changes post 0.69.2. Because it's been a while since I started a new project, I went back through react-native init to see what else changed. It seems that as of latest, you no longer need the javascript module.

Can you make the following changes and retest? (We're currently Mac-less over here so I can't check the xcode output). These steps remove the JS module. We won't be able to use custom plugins such as the Redux flipper plugin yet, but it will get rid of possible noise.

  1. Remove react-native-flipper from your package.json and refresh your dependencies using yarn install. If that isn't possible, update react-native-flipper to 0.158.0 or later (removing seems to be the right answer, as 0.161.0 is exhibiting the weird linking error)
  2. Pull down the latest build from the rn69-compat branch or zip file here: rn69v2.zip- this might not be required, but I removed the codemods that checked for linking information based on the latest react native init template
  3. Re-run prebuild

@rlam3
Copy link
Contributor Author

rlam3 commented Aug 23, 2022

@jakobo I was able to get the export FLIPPER_DISABLE=1; expo prebuild --clean -p ios command to build without errors now locally. I was able to build the app in Xcode inside the iOS directory that was generated. No build errors here. Thank you for the help.

Whenever I want to enable flipper by building locally in my local environment with the following command it successfully builds my XXX.app and I'm able to install it into my iOS simulator. However, the app is not showing up in the Flipper desktop app which previously was showing up before removing react-native-flipper.

eas build --platform ios --profile=local --non-interactive --local

Is there some other thing that I would require to edit in my configuration?

Screen Shot 2022-08-23 at 7 04 43 PM

By default if we install this module, are we expecting FLIPPER_DISABLE=0?

Thanks!

@jakobo
Copy link
Owner

jakobo commented Aug 24, 2022

I'm glad we traced the error to the npm module. That at least will help the Flipper team if there's something going wrong with autolinking. I'll begin looking into their module this evening in case there's some weird interaction w/ it and expo.

By default if we install this module, are we expecting FLIPPER_DISABLE=0?

No, currently flipper will treat any value for FLIPPER_DISABLE as turning off flipper. The most recent updates in #20 now check explcitly for "1" to avoid this issue. If you update the plugin code or manually change line 37 of withFlipper.ios.js to

return `:flipper_configuration => ENV['FLIPPER_DISABLE'] == "1" ? ${inactive} : ${active}`;

That will keep flipper enabled by default (like it is in React Native) except if making a production build or Flipper is explicitly disabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working SDK 46 Affects plugin in Expo SDK 46
Projects
None yet
Development

No branches or pull requests

2 participants