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

fix(messaging, ios): eliminate auth/messaging notification race #6455

Merged
merged 3 commits into from
Aug 7, 2022

Conversation

mikehardy
Copy link
Collaborator

Description

This carries over the fantastic work from and supercedes #6392 from @glesperance

It eliminates a race between auth and messaging on probe notifications, as documented in that PR

Release Summary

squash merge / PR title for conventional commits / semantic release

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

Apparently in use in production per discussion related PR, received a look from firebase-ios-sdk maintainers as well


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

@vercel
Copy link

vercel bot commented Aug 7, 2022

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

Name Status Preview Updated
react-native-firebase ✅ Ready (Inspect) Visit Preview Aug 7, 2022 at 7:21PM (UTC)
1 Ignored Deployment
Name Status Preview Updated
react-native-firebase-next ⬜️ Ignored (Inspect) Aug 7, 2022 at 7:21PM (UTC)

@codecov
Copy link

codecov bot commented Aug 7, 2022

Codecov Report

Merging #6455 (3e26f21) into main (62ee54b) will increase coverage by 17.82%.
The diff coverage is n/a.

❗ Current head 3e26f21 differs from pull request most recent head bc67c7d. Consider uploading reports for the commit bc67c7d to get more accurate results

@@              Coverage Diff              @@
##               main    #6455       +/-   ##
=============================================
+ Coverage     54.49%   72.30%   +17.82%     
=============================================
  Files           208      109       -99     
  Lines         10358     4657     -5701     
  Branches       1645     1048      -597     
=============================================
- Hits           5644     3367     -2277     
+ Misses         4433     1212     -3221     
+ Partials        281       78      -203     

@mikehardy mikehardy merged commit 7183118 into main Aug 7, 2022
@mikehardy mikehardy deleted the fix/messaging-didReceiveRemoteNotification-race branch August 7, 2022 20:05
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Aug 7, 2022
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.

1 participant