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

feat(BREAKING)!: forward-port to firebase-android-sdk v26 / firebase-ios-sdk v7 #4471

Merged
merged 16 commits into from
Nov 10, 2020

Conversation

mikehardy
Copy link
Collaborator

@mikehardy mikehardy commented Oct 30, 2020

Forward-port react-native-firebase to meet the new API surface area of firebase-ios-sdk v7 and firebase-android-sdk v26

This drops a large number of deprecated APIs

This also changes how you statically link firebase-ios-sdk. If you have special linking needs, read:

BREAKING CHANGE: alter ML imports, check iOS linking, remove old API as noted


iOS and Android are all passing local tests and should pass in CI as well
Docs are updated
Lint / Spellcheck all pass

There is a committed (on my part) dependency on having @react-native-mlkit up and running well enough to give developers a 1:1 migration path when releasing this

Review considerations:

  • the initial commits are designed to be reviewed (and committed) one at a time. The last one has to go at once
  • I collapsed all of the ML stuff to one directory as this seemed the direction Firebase ML was going after peeling it's MLKit APIs off - there wasn't much left and differentiating didn't seem sensible
  • the way I handled the change to remote-config android getAll method is not great, but I am not great at the Tasks interface. I feel there is something clean + easy to change it too that I'm missing
  • I have commented out quite a few Firestore tests as they were failing locally, I'd like to re-enable these - it will depend on fixing the security rules in the local emulator

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

@vercel
Copy link

vercel bot commented Oct 30, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/k5mdkqt4a
✅ Preview: https://react-native-firebase-git-forward-port-v7ios-v26android.invertase.vercel.app

@codecov
Copy link

codecov bot commented Oct 30, 2020

Codecov Report

Merging #4471 (7e1a836) into master (8a638b5) will increase coverage by 23.64%.
The diff coverage is 93.55%.

@@             Coverage Diff             @@
##           master    #4471       +/-   ##
===========================================
+ Coverage   41.92%   65.55%   +23.64%     
===========================================
  Files          35      109       +74     
  Lines        1119     3698     +2579     
  Branches      278      276        -2     
===========================================
+ Hits          469     2424     +1955     
- Misses        489     1148      +659     
+ Partials      161      126       -35     

@mikehardy mikehardy changed the title BREAKING CHANGE: forward-port to firebase-android-sdk v26 / firebase-ios-sdk v7 feat(BREAKING): forward-port to firebase-android-sdk v26 / firebase-ios-sdk v7 Oct 31, 2020
@mikehardy mikehardy force-pushed the forward-port-v7ios-v26android branch from d39294c to 0b5d852 Compare October 31, 2020 03:39
@mikehardy mikehardy force-pushed the forward-port-v7ios-v26android branch from 0b5d852 to 1ac391e Compare October 31, 2020 17:48
@mikehardy mikehardy force-pushed the forward-port-v7ios-v26android branch from 1ac391e to d184a9e Compare October 31, 2020 18:37
@mikehardy mikehardy force-pushed the forward-port-v7ios-v26android branch from d184a9e to 29dbc70 Compare October 31, 2020 19:12
@mikehardy mikehardy requested review from Ehesp and Salakar October 31, 2020 19:32
@mikehardy
Copy link
Collaborator Author

mikehardy commented Oct 31, 2020

@Salakar / @Ehesp I think the RNFB forward-port to the new upstream releases is ready. It passes local testing and CI (modulo android E2E flakiness - I swear it works locally) with docs polished. Once it passes review (with whatever changes are necessary) I'd like to release it as an NPM @next or similar for test integration then launch it. I'm going to turn my focus to @react-native-mlkit now so it's ready when this lands

@mikehardy
Copy link
Collaborator Author

mikehardy commented Oct 31, 2020

@Salakar release note: this will require @react-native-firebase/ml package creation on npmjs, as the home for the consolidated cloud-only ML package

@Salakar Salakar changed the title feat(BREAKING): forward-port to firebase-android-sdk v26 / firebase-ios-sdk v7 feat(BREAKING)!: forward-port to firebase-android-sdk v26 / firebase-ios-sdk v7 Nov 2, 2020
@ewfian
Copy link

ewfian commented Nov 3, 2020

@mikehardy
Copy link
Collaborator Author

@ewfian - I want to do a "minimal intervention" forward port first - which is another way of saying I understand how it looks like an opportunity but as a maintainer this looks like a very risky set of changes with a lot of potential user upheaval, and not the time to do more than the miminum at first just to make sure I haven't done anything terribly wrong. react-native-mlkit and the auth emulator are then immediate / almost-stop-work priorities and only then will I feel as if I can take on new APIs etc.

That's my personal task list / priorities at least - I am more than happy to work with any contributor that takes a chance at implementing something. I will do my utmost to shepherd any community PR to merge if you would like to try

@kuznetsov-from-wonderland

Dear all, how it is going? We need this to get #4299 resolved

This is part of the forward port to firebase-ios-sdk v7.0.0
This is part of the forward port to firebase-android-sdk v26.0.0

BREAKING CHANGE: there is no replacement for the setMinimumSessionDuration API
This is part of the forward port to firebase-android-sdk v26 / firebase-ios-sdk v7

All of these APIs were marked as deprecated in an earlier release and no longer functioned. This just removes them

BREAKING CHANGE: drop defaultConfig, settings, isDeveloperModeEnabled, minimumFetchInterval, setLogLevel
Until the emulator works correctly locally, data-dependent firestore tests are flaky
This is part of the forward-port to firebase-ios-sdk v7.0.0
rework github action to use the added run script
…s-sdk v7

This drops a large number of deprecated APIs and separates MLKit into cloud APIs (supported) vs on-device (moved to react-native-mlkit)

- on-device MLKit model APIs are in `@react-native-mlkit` now
- cloud ml-vision APIs have moved to just 'ml', in general change all 'vision' to 'ml' (following casing)
- deprecated remote-config APIs are purged
- many other changes, please refer to the upstream release notes for details:
  - https://firebase.google.com/support/release-notes/android#2020-10-27
  - https://firebase.google.com/support/release-notes/ios#version_700_-_october_26_2020

This also may give you options on how you link firebase-ios-sdk. If you have special linking needs, read:
  - https://firebase.google.com/docs/ios/link-firebase-static-dynamic

BREAKING CHANGE: alter ML imports, check iOS linking, remove old API as noted
@mikehardy mikehardy force-pushed the forward-port-v7ios-v26android branch from 26cc36c to 79c1f80 Compare November 10, 2020 18:16
@mikehardy mikehardy merged commit 79c1f80 into invertase:master Nov 10, 2020
@mikehardy mikehardy deleted the forward-port-v7ios-v26android branch November 10, 2020 18:57
@sanjaypojo
Copy link

@mikehardy awesome to have this merged 🙌 🙌 What's the right way to install and try it out? We currently use:

    "@react-native-firebase/app": "8.4.5",
    "@react-native-firebase/crashlytics": "8.4.9",
    "@react-native-firebase/messaging": "7.9.0",

Can I just install @next for all three?

@mikehardy
Copy link
Collaborator Author

mikehardy commented Nov 10, 2020

@sanjaypojo

This PR is now released and live for real, the publish ignored my intention to target the 'next' dist-tag, so it's up on 'latest' which is what the npm and yarn managers just grab for you.

I have it test-integrated already into my main work app and I can't find anything wrong. My make-demo.sh test-harness script also worked flawlessly.

To use it, just update any + all react-native-firebase packages you use to current, and make sure you don't have version overrides for the underlying firebase-android-sdk and firebase-ios-sdk (or if you do, update them to 26.0.0 and 7.0.0 respectively)

Any and all testing feedback welcome. Mind the 'BREAKING CHANGE' notes (with migration information) in the various package changelogs, available off: https://rnfirebase.io/releases/

@sanjaypojo
Copy link

@mikehardy works like a charm in beta and rolling out to production soon as well :) Amazing work!

@Donhv
Copy link

Donhv commented Nov 11, 2020

Hi @mikehardy
i cannot run pod install after upgrade latest version.
my version pod: 1.10.0
i use
"@react-native-firebase/analytics": "^8.0.1",
"@react-native-firebase/app": "^9.0.0",
"@react-native-firebase/crashlytics": "^8.5.2",
"@react-native-firebase/messaging": "^8.0.1",
"@react-native-firebase/ml": "^8.0.2",
Screen Shot 2020-11-11 at 08 54 40

@mikehardy
Copy link
Collaborator Author

@Donhv you don't post the text of the error, just a partial screen grab, so I'm just guessing

pod repo update
cd ios && rm -f Podfile.lock && pod install

you'll probably be fine with those commands

mikehardy added a commit that referenced this pull request Dec 24, 2020
Note that the breaking change was in #4471 and was called out in release notes
This is just a remnant where the pod utilities will likely give you more notice about it
hmhm2292 pushed a commit to hmhm2292/react-native-firebase that referenced this pull request Jul 13, 2021


Note that the breaking change was in invertase#4471 and was called out in release notes
This is just a remnant where the pod utilities will likely give you more notice about it
androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Aug 9, 2021


Note that the breaking change was in invertase#4471 and was called out in release notes
This is just a remnant where the pod utilities will likely give you more notice about it
androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Aug 9, 2021


Note that the breaking change was in invertase#4471 and was called out in release notes
This is just a remnant where the pod utilities will likely give you more notice about it
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.

6 participants