Fix searching for FlipperClient.h headers in productions builds if PRODUCTION=1 passed

[MuhmdRaouf]

Summary

In newer builds of react-native if you passed parameters PRODUCTION=1, flipper pods won't get installed
you can see that in their codebase here react_native_pods.rb

so when building ios react-native-flipper set compiler_flags = '-DFB_SONARKIT_ENABLED=1' in the podspec file
which initializeFlipper and lead to the following error:
node_modules/react-native-flipper/ios/FlipperReactNativeJavaScriptPluginManager.h:12:9: 'FlipperKit/FlipperClient.h' file not found

Changelog

use same condition as react-native to wrap s.compiler_flags = compiler_flags
and conditionally pass compiler_flags = '-DFB_SONARKIT_ENABLED=1' to compiler_flags to avoid build problems

Test Plan

To reproduce

• create new react-native app
• install react-native-flipper yarn add react-native-flipper
• run env PRODUCTION=1 npx pod-install
• run yarn ios

Before patch:

node_modules/react-native-flipper/ios/FlipperReactNativeJavaScriptPluginManager.h:12:9: 'FlipperKit/FlipperClient.h' file not found

After patch:

▸ Build Succeeded
success Successfully built the app

[coveralls]

Pull Request Test Coverage Report for Build 3409071923

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 9 unchanged lines in 1 file lost coverage.
• Overall coverage remained the same at 49.969%

---

Files with Coverage Reduction	New Missed Lines	%
desktop/flipper-server-core/src/devices/ios/IOSBridge.tsx	9	38.66%

Totals
Change from base Build 3395015229:	0.0%
Covered Lines:	7846
Relevant Lines:	14243

---

💛 - Coveralls

[facebook-github-bot]

@mweststrate has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@MuhmdRaouf has updated the pull request. You must reimport the pull request before landing.

[mweststrate]

We deliberately do not want to be included in production builds, as it is a huge risk to both security and performance. If you want to include Flipper in prod builds at your own risk, feel free, but we don't want to provide any infra to encourage such thing as we consider it a really bad idea.

Let me know if I misunderstood the PR :)

[MuhmdRaouf]

@mweststrate Hey, this PR was focused on fixing the problem located here

flipper/react-native/react-native-flipper/ios/FlipperReactNativeJavaScriptPlugin.m

Line 8 in 1adcf2b

#if defined(DEBUG) || defined(FB_SONARKIT_ENABLED)

as you can see you check if defined(DEBUG) || defined(FB_SONARKIT_ENABLED)
and defined(FB_SONARKIT_ENABLED) is always true due PodSpec file here

flipper/react-native/react-native-flipper/react-native-flipper.podspec

Line 9 in 1adcf2b

compiler_flags = '-DFB_SONARKIT_ENABLED=1'

which leads Flipper to always work even in a production env because of the OR condition

I hope that makes sense so far.

so to this PR, If I make env PRODUCTION=1 npx pod-install

react-native will exclude Flipper pods from installing and when making iOS build the condition defined(FB_SONARKIT_ENABLED) will make the lib runs to look for FlipperClient.h which will cause the error

node_modules/react-native-flipper/ios/FlipperReactNativeJavaScriptPluginManager.h:12:9: 'FlipperKit/FlipperClient.h' file not found cause flipper pods haven't got installed.

so this PR prevents this error from happing as it's removing the compiler_flags = '-DFB_SONARKIT_ENABLED=1'
which will lead both conditions to false and build complete successfully.

also, I would suggest changing the condition here

flipper/react-native/react-native-flipper/ios/FlipperReactNativeJavaScriptPlugin.m

Line 8 in 1adcf2b

#if defined(DEBUG) || defined(FB_SONARKIT_ENABLED)

to be nested instead

#if defined(DEBUG)
#if defined(FB_SONARKIT_ENABLED)
 // logic goes here
#endif // FB_SONARKIT_ENABLED
#endif // DEBUG

or change the condition to be AND

#if defined(DEBUG) && defined(FB_SONARKIT_ENABLED)
// logic goes here
#endif // DEBUG && FB_SONARKIT_ENABLED

I would love to make that PR too, just let me know how it will go with this PR and we can go from there.

[facebook-github-bot]

@MuhmdRaouf has updated the pull request. You must reimport the pull request before landing.

[mweststrate]

@lblasa would you mind taking a closer look on the above? thanks!

[robinclaes]

When setting :flipper_configuration => FlipperConfiguration.disabled in the Podfile, the React-native build indeed fails with
node_modules/react-native-flipper/ios/FlipperReactNativeJavaScriptPluginManager.h:12:9: 'FlipperKit/FlipperClient.h' file not found

I think this error should not occur when disabling Flipper, regardless of the environment variable (PRODUCTION) being set or not.
So I think the solution in the PR might not be the ideal solution.

In our case, our Podfile looks something like this:

:flipper_configuration => ENV['APPCENTER_BUILD_ID'] ? FlipperConfiguration.disabled : FlipperConfiguration.enabled(["Debug"], { 'Flipper' => flipperkit_version })```

[facebook-github-bot]

@mweststrate merged this pull request in 0c442bf.