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.

Sign up for GitHub

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

Jump to bottom

feat!: Update Firebase iOS SDK version to v11 #8028

Merged
merged 11 commits into from
Sep 26, 2024
Merged

feat!: Update Firebase iOS SDK version to v11 #8028

merged 11 commits into from
Sep 26, 2024

Conversation

Salakar
Copy link
Contributor

@Salakar Salakar commented Sep 19, 2024

This is a breaking change since the minimum version of iOS and macOS has changed

Description

Related issues

Release Summary

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

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

@vercel Vercel
Copy link

vercel bot commented Sep 19, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-firebase ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 25, 2024 2:16am

mikehardy
mikehardy reviewed Sep 19, 2024
View reviewed changes
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh darn - I was hoping this was non-breaking but forgot about the bumps to the min supported version on platforms, quite right on that one

podspecs and docs may not be inline with regards to minimums speced in podfile and/or described as supported in docs - haven't scanned

code may have @available branches that are no longer applicable as new minimum means previous checked ver is always available, could use a grep for available perhaps

test app may need xcproj update to change min supported ?

that's all I can think of as those things bump...

@mikehardy
Copy link
Collaborator

mikehardy commented Sep 24, 2024 •
edited
Loading

Task was to determine what was causing storage emulator to crash the app on iOS

  • updated all the dependencies so software was current since this was likely some sort of upstream issue, perhaps current emulator and current firebase-ios-sdk played nicer together?
    • this required strong typings on the functions calls now, so quickly forward-ported those to current signatures and v2 functions where possible
    • added signBlob IAM permission to the new v2 cloud functions service account for appCheck token e2e test
    • this required a couple little fixes to the functions deploy so the new functions were in the cloud as needed (with new V2 names)
    • storage test helpers manual rule push now had a type error, but it looks like the storage emulator is correctly ingesting the rules from storage rules json so just deleted those
  • ✅ root cause was a bunch of upstream firebase-ios-sdk storage init changes where they were trying to eliminate some data race and now more strongly guard against misconfigured storage instances (emulator vs non-emulator) which were happily data-racing past before it seems. Correctly calling useEmulator for the secondaryFromNative app during e2e test app init fixes things.

While I was there I took care of my own review comments and bumped firebase-android-sdk:

  • updated docs with new minimums
  • eliminated dead code paths on iOS guarded by @available checks that are always available with new minimum iOS target
  • verified podspecs were enforcing things correctly

I think this is good to go? It passed e2e tests locally now and I reproduced the earlier failure. Hopefully CI agrees.

Salakar and others added 11 commits September 24, 2024 21:11
@Salakar @mikehardy
feat!: Update Firebase iOS SDK version to v11
fa084cf 
This is a breaking change since the minimum version of iOS and macOS has changed
@mikehardy
fix(firestore, ios): remove nanopb transitive dep from podspec
2f676bf 
Upstream firestore-ios-sdk-frameworks issue is fixed and the Firestore binary distribution now directly depends on nanopb as it should, so we should remove the workaround here.

This tangentially fixes compile of InAppMessaging which was specifying a mutually exclusive nanopb version range
@mikehardy
fix(crashlytics, ios): dependencies init param removed upstream in v11+
85c5d19 
firebase-ios-sdk v11+ removed one of the parameters from their component initialization API, we must remove it here to use firebase-ios-sdk v11+
@mikehardy
fix(android, sdk): adopt firebase-android-sdk 33.3.0
2445fc1
@mikehardy
docs(app): document new SDK versions for anyone overriding
499a8b9
@mikehardy
chore(ios): remove Apple platform code paths for dropped versions
df947df
@mikehardy
test(functions): yarn2+ does not have a --prefix arg, use --cwd
688fd15 
only noticed when attempting to deploy our test functions, this must
have been missed in the yarn2+ forward-port
@mikehardy
test(functions): use v2 test functions
6a453fd 
with exception of custom region - it does not work well with v2
@mikehardy
build(deps): bump javascript dependencies
ca7f516 
the goal here is mostly to get firebase emulator up to date
@mikehardy
test(storage, ci): storage rules no longer need to be manual
50b0fdb 
...and in fact they cause a problem with a TypeError that URL.pathname
does not have a setter any longer
@mikehardy
test(storage): configure storage secondary app to use emulator
411c018 
somehow this wasn't flagged as an error before, but on firebase-ios-sdk
v11+ if you attempt to use a secondary app with incorrect emulator config
the app will crash now
@witcher-shailesh
Copy link

If the PR looks fine, can we merge and release a new version?

We are facing issues with latest versions of iOS 😞

@mikehardy
Copy link
Collaborator

@witcher-shailesh well, sure - but you realize it only looks fine since approx 12 hours ago and that was me just hammering on it without review 😆 . It's in review today :-)

@witcher-shailesh
Copy link

Oh understood @mikehardy, We currently needed it for local development mostly, do we don't release beta or preview versions where we can opt during development phase and suggest feedback?

@mikehardy
Copy link
Collaborator

@witcher-shailesh there are a variety of ways to get things early:

@exaby73 exaby73 merged commit 6bc6728 into main Sep 26, 2024
18 of 19 checks passed
@exaby73 exaby73 deleted the bump-ios-sdk branch September 26, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@exaby73 exaby73 exaby73 approved these changes

@mikehardy mikehardy mikehardy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Salakar @mikehardy @witcher-shailesh @exaby73