Skip to content

v5 Update all Firebase SDK dependencies to current #2240

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

Merged
merged 3 commits into from
Jun 18, 2019

Conversation

mikehardy
Copy link
Collaborator

@mikehardy mikehardy commented Jun 16, 2019

Summary

I want react-native-firebase v5 to stay current with the underlying SDKs during the v6 transition

Checklist

Test Plan

I ran all the e2e tests on android and iOS while making the changes, everything still passes

The changes were actually very trivial I think, so this was much easier than I feared

Other than removing invites APIs and hard-switching dynamic links to domainURIPrefix this is easy to match on v6, I can propose a PR over there if you like

Release Plan

[BREAKING CHANGE][DynamicLinks][domainUrlPrefix] use prefixURIDomain instead of domainUrlPrefix (start with ‘https://')
[BREAKING CHANGE][Invites] Invites is gone, use DynamicLinks
[BREAKING CHANGE][Perf][incrementCount] Android: performance.Trace.incrementCounter() is gone
[Chore][Dependencies][Firebase SDK] update Firebase SDKs to current, alter APIs accordingly
[Fix][Test][GoogleSignin] You have to include ‘GoogleSignIn’, ‘~>4.4’ yourself now
[Feat][Test][AdMob] Integrate AdMob into test app


Think react-native-firebase is great? Please consider supporting the project with any of the below:

[BREAKING CHANGE][DynamicLinks][domainUrlPrefix] use prefixURIDomain instead of domainUrlPrefix (start with ‘https://')
[BREAKING CHANGE][Invites] Invites is gone, use DynamicLinks
[BREAKING CHANGE][Perf][incrementCount] Android: performance.Trace.incrementCounter() is gone
[Chore][Dependencies][Firebase SDK] update Firebase SDKs to current, alter APIs accordingly
[Fix][Test][GoogleSignin] You have to include ‘GoogleSignIn’, ‘~>4.4’ yourself now
[Feat][Test][AdMob] Integrate AdMob into test app
@codecov
Copy link

codecov bot commented Jun 16, 2019

Codecov Report

Merging #2240 into v5.x.x will increase coverage by 2.12%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           v5.x.x    #2240      +/-   ##
==========================================
+ Coverage   74.16%   76.27%   +2.12%     
==========================================
  Files          64       61       -3     
  Lines        1629     1576      -53     
==========================================
- Hits         1208     1202       -6     
+ Misses        421      374      -47

@mikehardy
Copy link
Collaborator Author

First time green CI 😎 but codecov didn't get an upload for some reason? Not sure if that started happening after the ci.android.build change or not?

I think this is ready to go though

@Ehesp
Copy link
Member

Ehesp commented Jun 17, 2019

@mikehardy

[BREAKING CHANGE][Invites] Invites is gone, use DynamicLinks

We've still got this in v6 until it is fully deprecated, I don't think we can remove it yet as this would be a severe breaking change for anyone using it?

@Salakar
Copy link
Contributor

Salakar commented Jun 17, 2019

What @Ehesp said, it's deprecated but still needs to remain until the end of the year, at least that's what the Firebase Docs website says.

@mikehardy
Copy link
Collaborator Author

🤔, but, how can it remain for the end of the year, when one of the SDKs has already removed it completely? Any PR I put up is just a starting point for discussion so realize I'm honestly just trying to understand.

My thinking:

  • I don't know how to leave code in that does not exist :-). Since this module straddles both SDKs it seems sensible to drop something if it's scheduled to be dropped and one SDK has done so
  • I get that it's a breaking change, but at the same time, no one has to update right? v5.4 can receive maintenance patches and v5.5 can roll along (and when I say that, I'm saying "I can personally issue maintenance patches..." I'm not trying to make work for others and my work effort here so far hopefully shows I care about user support as well)
  • Without the current SDKs no one can open valid issues in the upstream repository, so for me and anyone with underlying issues this actually is needed

So summing it up I was thinking, make space for the break in v5 (done #2214 ), but get v5 well set to be maintainable (done), then move on with the breaks upstream forces

Maybe there's a better way?

@Salakar
Copy link
Contributor

Salakar commented Jun 17, 2019

I was basing the date off of the one here: https://firebase.google.com/docs/invites/deprecation, I assumed the SDKs would still be there until that date, but I assumed wrong as it looks like they have removed it from iOS - though I have a feeling this was done prematurely, yay.

So let's just go with this release and remove it now, shame the effort implementing invites for v6 was a complete waste of time 😫

@mikehardy
Copy link
Collaborator Author

So let's just go with this release and remove it now, shame the effort implementing invites for v6 was a complete waste of time

but hey, reduced surface area? (trying for the brighter side here, sorry ;-) )

I still have your quick search through image from discord on what you thought was deprecated etc when I proposed updating all the SDKs and I want to check through it again, and think about this again (it's a big change), but if it all looks good then I will continue pursuing it for a v5.5.0

Then I can get busy on my app with memory profiling in iOS to find a ...delightful native memory corruption crash from iOS firebase SDK

@mikehardy
Copy link
Collaborator Author

Actually - reading the link you posted, the servers will start returning errors in 6 months breaking the whole invite system, which means that removing it "downstream" (us, and the iOS SDK) makes sense really, if this whole system will soft-fail (they say they'll error gracefully via SDK handling) starting that soon then really no one should be using it on any new code, and pushing people to migrate via a breaking change doesn't seem so evil to me

But if there's an urgent unrelated patch I'll happily work up a 5.4.x maintenance release as well - no intention to strand people. And hopefully it's moot with v6 soon enough anyway.

@mikehardy
Copy link
Collaborator Author

No good deed unpunished! Check this out, just today they released breaking / majors for everything https://developers.google.com/android/guides/releases

Welcome to AndroidX migration pain.

I guess I need to actually do this react-native-community/discussions-and-proposals#129 and have the AndroidX / Android react-native chasm bridged

@Salakar
Copy link
Contributor

Salakar commented Jun 17, 2019

Oh boy, well the Firebase Android SDK hasn't changed yet: https://firebase.google.com/support/release-notes/android

Happy for this to go with this as is, less to maintain for v5.x.x anyway :P

…ative

Older flow binaries can't handle new react-native
Newer flow binaries can't handle v5 react-native-firebase

So I stayed on old flow binary and type-ignored node_modules so that
`yarn validate:all:flow` would pass
@mikehardy
Copy link
Collaborator Author

They are just lagging on the publish - the new dependency versions are up per the original link I sent, and the one you linked is probably just going to re-render tonight in a nightly process or something - it's definitely out.

AndroidX maybe for a v5.6.0 but I don't want to push change out to quickly. v5.5.x will be non-AndroidX. At least the iOS Pods will be up to date...

I just pushed a commit that bumped the main package.json packages all to current (react-native 0.59.9) except flow-bin. Once that builds cleanly I think a 5.5.0 will be possible

@mikehardy mikehardy merged commit f16a2a0 into invertase:v5.x.x Jun 18, 2019
@mikehardy mikehardy deleted the v5-sdk-update branch June 18, 2019 13:17
@mikehardy
Copy link
Collaborator Author

Okay, we are on the road to a v5.5.0 release.

It is not AndroidX but for those that need it, I did the AndroidX refactor on my app, integrated this branch, then integrated the jetifier package and ran npx jetify and everything actually worked. So there is a path for you, you are not blocked.

mikehardy added a commit to invertase/react-native-firebase-docs that referenced this pull request Jun 19, 2019
* docs: update all Firebase SDKs to current, API changes to match
        works with invertase/react-native-firebase#2240
* docs: added one more breaking change for v5.5.0, firestore ids never null
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

Successfully merging this pull request may close these issues.

3 participants