fix: onTokenChange method causing app to crash

[focux]

Description

After being several days trying to find what was causing my app to crash with a very undetailed message:

TypeError: Properties can only be defined on Objects

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

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • Yes
• My change supports the following platforms;
  • Android
  • iOS
• I have updated TypeScript types that are affected by my change.
• This is a breaking change;
  • Yes
  • No

---

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

• 👉 Star this repo on GitHub ⭐️
• 👉 Follow React Native Firebase and Invertase on Twitter

[CLAassistant]

All committers have signed the CLA.

[Salakar]

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

[focux]

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.

[Salakar]

Thanks for replying - I'd be inclined then to just remove the work around entirely then I think, let's remove the Object.defineProperty and change listener(tokenStringWithTokenAccessor); to listener(token);.

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

[focux]

Great! I just pushed the requested changes. Let me know if it's okay or if you want to change anything.

[Salakar]

Awesome, thanks! I'll get this reviewed and released tomorrow (late here) 🎉

[alexfoxy]

@Salakar getting this issue, when will this fix be merged into a release?

[mikehardy]

@focux check out the patch-package package - if you try it yourself and it works, you have a nice persistent (across CI other devs etc) fix locally plus you can report success here - win/win

[focux]

@focux check out the patch-package package - if you try it yourself and it works, you have a nice persistent (across CI other devs etc) fix locally plus you can report success here - win/win

Actually, I already use patch-package and I applied the fix since day one, it's working well. We just need to get this PR merged and publish a new major. I think that @Salakar was going to do that.

[Salakar]

Batching a few commits into the next release (nothing extreme) so will manually trigger a release tomorrow, sorry for the delay 🙈

[focux]

Love it. Thank you.

[smxdevst]

Any updates when this one will be publishing? @Salakar