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

Flipper should not get built on iOS release builds #1275

Closed
TPXP opened this issue Jun 18, 2020 · 14 comments
Closed

Flipper should not get built on iOS release builds #1275

TPXP opened this issue Jun 18, 2020 · 14 comments

Comments

@TPXP
Copy link

TPXP commented Jun 18, 2020

Hello,

As pointed by someone else in a related issue, when adding Flipper to a React-Native project (or I guess any project), building times get significantly increased. This is not desirable.

Impact: longer build times react-native-community/discussions-and-proposals#226

Proposed solution: I'm thinking about adding flag DISABLE_FLIPPER, and wrap all files with #ifndef DISABLE_FLIPPER. I guess that would definitely help, but requires patching dependencies as well. Thoughts?

@Monkeystation
Copy link

Also when creating the archive, the file size is significantly larger. In the screenshot you'll see the default 'AwesomeProject' with Flipper (283MB) and without Flipper (removed from Podfile, 88MB)

Screenshot 2020-07-06 at 17 45 54

@mweststrate
Copy link
Contributor

@TPXP all our files for react-native-flipper have been wrapped recently in #ifdef DEBUG flags, however, that doesn't remove the build times for transitive dependencies as you noticed. I think these kind of issues should be solved on an infrastructural level by having conditional builds / dependencies or something, rather than here by monkey patching dependencies. Many deps are quit generic which might have a lot of unforeseen effect (e.g some other library might depend on a dependency flipper has a well, and a monkey patch would break those in production, which would bring us in a worse situation).

(disclaimer: I'm not too familiar with iOS builds in general)

@alloy
Copy link
Contributor

alloy commented Jul 16, 2020

@TPXP There is no need to add extra configuration, simply remove the lines that include Flipper from your Podfile.

@mweststrate
Copy link
Contributor

@alloy can that be made conditional based on debug vs release build?

@alloy
Copy link
Contributor

alloy commented Jul 16, 2020

That already is conditional, alas it’s being hidden behind a layer of indirection https://github.com/facebook/react-native/blob/eb2a561ecb10a97d458cce53ee27b0fba655f018/scripts/react_native_pods.rb#L62-L90.

To re-iterate, it will be build for release builds, but it will not be linked.

@TPXP
Copy link
Author

TPXP commented Jul 17, 2020

Thanks for your feedback, I didn't know that the files for react-native-flipper were wrapped in #ifdef DEBUG. Any reason why the files for Flipper and its subpackages don't have the same optimization?

Granted, I could remove the lines about flipper from the PodFile before creating a release build, but that requires installing pods again. It would work, but I'm sure we can find a better way to handle this!

@mweststrate
Copy link
Contributor

Because these packages are used in different other environments, or could actually be a transitive dependency of your real project as well. So wrapping them could have very unintended consequences.

Closing the issues as there is further not anything we can do from our side.

@TPXP
Copy link
Author

TPXP commented Jul 17, 2020

How about wrapping the code of these dependencies in a custom flag like #ifndef DISABLE_FLIPPER? This way, I think we can avoid building these tools when building production builds while not affecting projects using these tools as dependencies

@alloy
Copy link
Contributor

alloy commented Jul 20, 2020

@TPXP Have you by any chance tried and measured the actual impact of doing so?

@andreialecu
Copy link

I'm not sure why this wasn't proposed yet, but a simple ENV variable can help with this.

  if !ENV['CI']
    use_flipper!
    post_install do |installer|
      flipper_post_install(installer)
    end
  end

On CircleCI the above tweak resulted in dropping build times from about 14 minutes to 8 minutes.

@TPXP
Copy link
Author

TPXP commented Aug 3, 2020

Well, such changes require to install dependencies for the Pods project to be updated (and Xcode to not build Flipper and its dependencies). It definitely makes sense in a CI environment, which is where build minutes are the most "costly". And since CI systems have to install pods anyway, it's smart to prevent bundling of flipper here.

I'm trying to see if this can be disabled also if the app is built on dev machines but your approach is definitely a good step forward 👍

@TPXP
Copy link
Author

TPXP commented Aug 3, 2020

I dug some more into this today. It seems we cannot inject custom definitions when working with Cocoapods anyway, since the only one ("Debug") is hardcoded, and configuration definition does not take into account the target for which the configuration is generated (see https://github.com/CocoaPods/CocoaPods/blob/56538172fac99b410046812df67087a8829a7019/lib/cocoapods/project.rb#L379).

This issue of pods being built even if not used in the configuration has already been raised to Cocoapods: CocoaPods/CocoaPods#9658 (comment) . A few things have been attempted, but nothing landed yet.

I guess we'll have to implement a few things on Cocoapods before we can make progress on this.

mergify bot pushed a commit to celo-org/celo-monorepo that referenced this issue Jan 27, 2021
…on (#6488)

### Description

This PR enables Flipper on iOS along with `Redux Debugger` and `Reactotron` plugins.

This allows viewing / debugging the following:
- React DevTools (Components and Profiling)
- Network connections
- View hierarchy
- Redux State / Actions
- AsyncStorage
- App preferences
- Hermes
- and more ;)

It also opens the door for creating/adding our own Flipper plugins (for instance for viewing transactions 😉).

Setup instructions were added to the mobile README.

### Details

When we upgraded to React Native 0.63.x and Reanimated 2, the introduction of TurboModules broke the ability to use remote debugging with the Chrome DevTools, see [this discussion with more details](react-native-community/discussions-and-proposals#206).

As an alternative we can use Flipper to provide similar debugging functionalities.

Flipper was already working on Android, but on iOS there was a conflict between the OpenSSL lib shipped as part of `react-native-fast-crypto` and the one included by `Flipper`. To workaround this problem, we now use [a "headers" only version of `OpenSSL`](https://github.com/celo-org/OpenSSL-headers) that makes `Flipper` happy while building. 

The 2nd issue was that Flipper in React Native doesn't work with `use_frameworks!` enabled in the `Podfile`, which we need because we have Swift dependencies, which don't work without it.
I bent CocoaPods a little more so that everything Flipper related is built as static frameworks.

I've integrated both `Redux Debugger` and `Reactotron` as they provide different lenses to debug the app.
There's also a `redux-saga` plugin for `Reactotron` but it triggered some odd unhandled promise rejections, so I didn't include it.

While working on this I noticed the state in the `exchange` reducer is very big and made the `Redux Debugger` plugin quite unusable within Flipper. There's an issue open which should address this (#6284). In the meantime I've filtered out that part of the state in `Redux Debugger`. `Reactotron` can still be used to see it and is not affected.

Overall this gives us more tools to see what is happening within the app.

### Tested

- Flipper works on both iOS and Android along with their `Redux Debugger` and `Reactotron` plugins.
- iOS release builds don't include Flipper (Flipper modules are built for release builds, but NOT linked - this makes the build take more time, but unfortunately no simple way around this with CocoaPods for now, or we'd need to manage multiple `Podfile.lock` files, see facebook/flipper#1275).

<img width="1554" alt="Screenshot 2021-01-15 at 11 38 52" src="https://user-images.githubusercontent.com/57791/104722566-4bfe4800-572e-11eb-973c-63c7af58b94f.png">
<img width="1554" alt="Screenshot 2021-01-15 at 11 39 19" src="https://user-images.githubusercontent.com/57791/104722570-4f91cf00-572e-11eb-9bb7-4fe0abf4336c.png">
<img width="1554" alt="Screenshot 2021-01-15 at 12 30 47" src="https://user-images.githubusercontent.com/57791/104722693-836cf480-572e-11eb-8ef0-83e54a52dfe8.png">
<img width="1554" alt="Screenshot 2021-01-15 at 11 40 26" src="https://user-images.githubusercontent.com/57791/104722580-5587b000-572e-11eb-886e-8fff2da8cffd.png">
<img width="1554" alt="Screenshot 2021-01-15 at 11 40 48" src="https://user-images.githubusercontent.com/57791/104722594-5a4c6400-572e-11eb-86bd-873add6027b1.png">
<img width="1554" alt="Screenshot 2021-01-15 at 11 47 17" src="https://user-images.githubusercontent.com/57791/104722632-6801e980-572e-11eb-8412-d86a8a29d383.png">
<img width="1554" alt="Screenshot 2021-01-15 at 11 49 12" src="https://user-images.githubusercontent.com/57791/104722649-6df7ca80-572e-11eb-8ac5-bae41abd6488.png">
<img width="1554" alt="Screenshot 2021-01-15 at 11 49 32" src="https://user-images.githubusercontent.com/57791/104722659-7223e800-572e-11eb-9734-59512f3987f9.png">

### Related issues

- Discussed on Slack: https://celo-org.slack.com/archives/CL7BVQPHB/p1610040529072600

### Backwards compatibility

Yes
@matt-dalton
Copy link

Has anyone managed to successfully cache pod dependencies using this approach (with circleCI for example)?

When we run pod install locally using this approach, we create a Podfile.lock including Flipper. On CI, Flipper is rightly excluded.

This then causes an issue trying to cache with circleCI.

            - restore_cache:
                  key: podfile-{{ checksum "./ios/Podfile.lock" }}
            - run:
                  name: Install iOS pods
                  command: |
                      pod install --project-directory=ios/
                      git --no-pager diff --exit-code ios/Podfile.lock | echo "Podfile has been generated differently in CI to the codebase. This means it won't be cached & will be expensive/slow to run each time."

            - save_cache:
                  paths:
                      - ios/Pods
                  key: podfile-{{ checksum "./ios/Podfile.lock" }}

This will always create changes in the Podfile.lock relative to what's been checked in.

Has anyone got a sensible strategy for this situation? Is there a better checksum to use?

@sacha-c
Copy link

sacha-c commented Oct 4, 2023

I have a similar question to @matt-dalton's regarding the consequences on the lockfile of excluding flipper

I've set up NO_FLIPPER == 1 in my CI for RN to exclude flipper
However I'm also trying to use pod install --deployment when installing dependencies in the CI, which should make my pod installation fail if there are changes to the lockfile (I want reproducible builds)
But since Podfile.lock is modified when using NO_FLIPPER, the --deployment argument is making it fail since it doesn't expect any changes.

I've thought of adding an additional CI step just to check my podfile with pod install --deployment & NO_FLIPPER=0, but I would prefer not having to install pods twice just to check this 😅

Any ideas or suggestions for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants