Skip to content

Conversation

@helenaford
Copy link
Member

@helenaford helenaford commented Aug 4, 2021

Description

This will always refresh the fcm token, but it's unclear to me if this is the correct approach or not based on the documentation and changelog

I've opened up a ticket on firebase/firebase-ios-sdk#8491 for guidance from the iOS team.

I don't believe there are any side-effects of always deleting the installations id, but it would be good to get confirmation as this approach always will delete it when refreshing the fcm token.

Related issues

Fixes #5570

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
Copy link

vercel bot commented Aug 4, 2021

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

react-native-firebase – ./

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/F5gse5SKK4R1xkY4Zm8PrBKaWfrG
✅ Preview: https://react-native-firebase-git-helenaford-fix-messa-2318a5-invertase.vercel.app

react-native-firebase-next – ./website_modular

🔍 Inspect: https://vercel.com/invertase/react-native-firebase-next/G8KYZkscL1eAm83JdyNUUet3NAvZ
✅ Preview: Canceled

@helenaford helenaford changed the title fix(messaging): deleteToken for iOS fix(messaging): ensure token is refreshed when calling deleteToken on iOS Aug 4, 2021
@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #5571 (b8fc083) into master (6b5dca5) will not change coverage.
The diff coverage is n/a.

❗ Current head b8fc083 differs from pull request most recent head 967b826. Consider uploading reports for the commit 967b826 to get more accurate results

@@           Coverage Diff           @@
##           master    #5571   +/-   ##
=======================================
  Coverage   74.17%   74.17%           
=======================================
  Files         106      106           
  Lines        4405     4405           
  Branches      941      941           
=======================================
  Hits         3267     3267           
  Misses       1067     1067           
  Partials       71       71           

@mikehardy
Copy link
Collaborator

Great - I just subscribed to the upstream issue and it looks like Paul B over there has already put it in a work queue, very curious to hear the result

@mikehardy
Copy link
Collaborator

I'm not sure we ever received the upstream guidance we were looking for, specifically if we should delete the installation token, or if simply deleting the messaging token is good enough?

They do appear to have a fix coming for messaging token delete though and it is tagged for firebase-ios-sdk 8.6.0

@mikehardy mikehardy added blocked: firebase-support Pending feedback or review from google support or response on official sdk repo issue. blocked: firebase-sdk Pending a confirmed fix landing on the official native sdk's (iOS/Android). labels Aug 11, 2021
@mikehardy
Copy link
Collaborator

This appears to be blessed by upstream but they also indicate they have fixed the underlying problem. I'd like to see if underlying sdk 8.6.0 resolves this before using an API from another module but will merge this if 860 has no effect

@mikehardy mikehardy removed the blocked: firebase-support Pending feedback or review from google support or response on official sdk repo issue. label Aug 17, 2021
@helenaford
Copy link
Member Author

@mikehardy thanks Mike. Yeah, thought that too - they seem to be happy with this and confirmed there are no side-effects to using it 😄 But agreed, probably don't need it if delete token is fixed.

@mikehardy
Copy link
Collaborator

mikehardy commented Aug 18, 2021

8.6.0 is rolling out now, already on cocoapods if you want to do a version override to test it? https://rnfirebase.io/#ios

The performance I'm getting from Installations.delete followed by getToken as I'm implementing that module is...wow 169099ms on Android is one of the faster ones. iOS is very fast but android is slow slow slow, so hopefully messaging tokens work...

@mikehardy mikehardy added Workflow: Waiting for User Response Blocked waiting for user response. and removed blocked: firebase-sdk Pending a confirmed fix landing on the official native sdk's (iOS/Android). labels Aug 19, 2021
@mikehardy mikehardy deleted the helenaford/fix-messaging-delete-token branch November 14, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Workflow: Waiting for User Response Blocked waiting for user response.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐛] FCM token does not refresh after deleting token via deleteToken() on iOS

3 participants