-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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: onTokenChange method causing app to crash #3552
Conversation
Is this Hermes by any chance? Guessing it is Also we're planning on removing this now, was just a hacky backwards compatibility fix - though I'm fine to merge this fix until we remove it |
Actually no, it’s not Hermes. I’m using the default engine and RN 0.60 if it matters. For the record, I tested it in v8 and in JSC, and in both, it throws an error.
|
Thanks for replying - I'd be inclined then to just remove the work around entirely then I think, let's remove the Each RNFirebase package is independently versioned so I don't see any harm in going ahead with the breaking change now - if you don't mind making the change? Once merged a new semver major of messaging will get automatically published to NPM, so can get this out pretty quickly |
…tive-firebase into fix-on-token-change-crash
Great! I just pushed the requested changes. Let me know if it's okay or if you want to change anything. |
Awesome, thanks! I'll get this reviewed and released tomorrow (late here) 🎉 |
@Salakar getting this issue, when will this fix be merged into a release? |
@focux check out the |
Actually, I already use |
Batching a few commits into the next release (nothing extreme) so will manually trigger a release tomorrow, sorry for the delay 🙈 |
Love it. Thank you. |
Any updates when this one will be publishing? @Salakar |
Removed deprecation. * fix: onTokenChange method causing app to crash * fix: use token instead of token with string ancestor Co-authored-by: Mike Diarmid <[email protected]>
Removed deprecation. * fix: onTokenChange method causing app to crash * fix: use token instead of token with string ancestor Co-authored-by: Mike Diarmid <[email protected]>
Description
After being several days trying to find what was causing my app to crash with a very undetailed message:
I found that the error is due to calling
onTokenChange
method from the messaging package. It was trying to define a new property in a string instead of doing it in his proto. Also, it seems that bug what's introduced by this commit 1940d6c.Related issues
Release Summary
Fix onTokenChange bug that was crashing app.
Checklist
Android
iOS
Think
react-native-firebase
is great? Please consider supporting the project with any of the below:React Native Firebase
andInvertase
on Twitter