invertase / react-native-firebase

🔥 A well-tested feature-rich modular Firebase implementation for React Native. Supports both iOS & Android platforms for all Firebase services.
https://rnfirebase.io
Other
11.54k stars 2.19k forks source link

[📚] Misleading documentation for use_frameworks! and $RNFirebaseAsStaticFramework #6594

Closed sasweb closed 1 year ago

sasweb commented 1 year ago

Documentation Feedback

The documentation about use_frameworks could be improved.

Taken from the docs: Beginning with firebase-ios-sdk v9+ (react-native-firebase v15+) you must tell CocoaPods to use frameworks.

And further: To use Static Frameworks on iOS, you also need to manually enable this for the project with the following global to the top of your /ios/Podfile file:

$RNFirebaseAsStaticFramework = true

use_frameworks! declares that you want to use dynamic frameworks, instead of static libraries. However, the documentation and also the variable naming implies that use_frameworks! enables static libraries.

Suggestions

  1. Rephrase *Beginning with firebase-ios-sdk v9+ (react-native-firebase v15+) you must tell CocoaPods to use frameworks.* to *Beginning with firebase-ios-sdk v9+ (react-native-firebase v15+) you must tell CocoaPods to use **dynamic** frameworks.*
  2. Rename $RNFirebaseAsStaticFramework = true to $RNFirebaseAsDynamicFramework = true

I hope I didn't mix it up but all resources I found are pretty clear about the fact that use_frameworks enables dynamic frameworks.

mikehardy commented 1 year ago

You know what is even more subtle? For Hermes to work with use_frameworks! you must use static linkage, and that's poorly documented in other areas. Moreover, for certain libraries and certain distribution styles (e.g., the zip distribution which our precompiled firestore uses: ) you also must use static linkage:

https://firebase.google.com/docs/ios/link-firebase-static-dynamic

So knowing what I know now, I would recommend strongly that people use use_frameworks! :linkage => :static so that hermes usage is possible, to the point where I would now say not specifying linkage, or specifying it as dynamic is not supported.

Then, what I really want to do is remove the $RNFirebaseAsStaticFramework variable entirely, as the firestore frameworks should be included as static frameworks all the time now.

So the work here is actually to specify use_frameworks + the static linkage style, and then further to remove the conditional stanzas in the packages/*/podspec files, instead specifying the deps static every time

Happy to take a PR on that if you have the time to clear it up

sasweb commented 1 year ago

Ok ok, I see. Let me do some experiments to see if get it right. I'll come up with a PR if things make sense to me. Thanks!

panoramix360 commented 1 year ago

I'm recently configuring the library on a project and got a little confused by the documentation as well.

We should put the $RNFirebaseAsStaticFramework = true or the $RNFirebaseAsDynamicFramework = true for now?

It could be good to eliminate the usage of this variable.

Regarding, the New Architecture not working, do you guys know what this could cause for the performance of React Native?

Thank you!

mikehardy commented 1 year ago

The docs are not ambiguous, they could just be simplified, just follow them: https://rnfirebase.io/#altering-cocoapods-to-use-frameworks

They say

$RNFirebaseAsStaticFramework = true

Regarding this:

Regarding, the New Architecture not working, do you guys know what this could cause for the performance of React Native?

No. There are not that many tests comparing the two, you can search for and see some though. Regardless, in my computer science education it was always 1) correctness, 2) performance

Since it does not even work, we are on step 1, step 2 is immaterial until step 1 is handled

panoramix360 commented 1 year ago

Fair enough, thank you for your help!

If you need any help with what we need to do to make the New Architecture work, we can talk about it, maybe I could help with something.

mikehardy commented 1 year ago

Per some discussion on the releases working group discussions and the react-native Discord server, this (and Flipper) working with use_frameworks are both known issues for Meta but will require them to lead the file restructure necessary to get it working, there's no action we can take unfortunately (other than disable flipper, disable new architecture for now)

LunatiqueCoder commented 1 year ago

@mikehardy Sorry for a late post and maybe for a stupid question, but I also found this: https://stackoverflow.com/questions/72289521/swift-pods-cannot-yet-be-integrated-as-static-libraries-firebasecoreinternal-lib

It seemed to help me, what are your opinions about this solution?

mikehardy commented 1 year ago

@criszz77 that's an unsupportable workaround to avoid use_frameworks, it is temporarily working for you at best, does not work already if you use storage or functions modules, and other modules will stop working as they fully convert to Swift. I strongly discourage anyone from using that.

The correct solution is to incorporate use_frameworks and drop dependencies that don't support it or work in those repos that you cannot drop such that it is supported