[@nativescript/firebase-messaging-core] feat: support async callbacks for messages and notification taps

[jkatins]

On iOS, after all registered callbacks for onMessage and onNotificationTap events are triggered, the completionHandler is called. However, when a callback returns a promise, the plugin doesn't wait for it to be settled but instead calls the completionHandler right away. This causes iOS to take away execution time from the app if it is in the background, even though it is still within the granted 30 seconds as per docs, essentially delaying the promise to settle until the next time the app is given execution time.

With these changes, the completionHandler is called only after all promises have settled.

[cla-bot[bot]]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla. CLA has not been signed by users: @jkatins. After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

[jkatins]

@cla-bot check

[cla-bot[bot]]

The cla-bot has been summoned, and re-checked this pull request!

[edusperoni]

I'm not sure this is the best approach, mainly because now the completionHandler will always be called async due to how promises resolve. Maybe a better solution is something like this:

const msg = deserialize(message);
const promises: Promise<unknown> = [];
            onMessageCallbacks.forEach((cb) => {
                const ret = cb(msg);
                                 if(ret instanceof Promise) { promises.push(ret); }
            });
            if(promises.length > 0) Promise.all(promises).catch().finally(() => completionHandler());
            else completionHandler();

[jkatins]

@edusperoni Thanks for your feedback. What are the downsides of the completionHandler always being called asynchronously, even if no callback returns a promise? As far as I can see, it doesn't make a difference to iOS nor to @nativescript/firebase-messaging or other depending plugins.

[NathanWalker]

Really nice solution @jkatins this is greatly appreciated 👍 I'll let @edusperoni weigh in before merging and getting an update out with this.

[edusperoni]

I'm saying this more as a failsafe than anything, so that the current behavior itself isn't altered. With the change I proposed we can have the best of both worlds I believe.

[gabrielbiga]

I can confirm the behavior that @jkatins just reported and fixed. Our app is freezing when opening a background notification on iOS. The solution so far was put a 1 sec setTimeout delay to take some action.

[NathanWalker]

Thank you @jkatins and @gabrielbiga for confirming in addition to the resolution here. We'll go ahead with a patch on this and later additional adjustment can be made per @edusperoni if needed.

[jkatins]

Thanks for merging this so swiftly @NathanWalker 🏆 Are there plans to get these changes into a new release anytime soon? Thanks again.