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

Add await to onBackgroundMessage callback #5762

Merged
merged 1 commit into from
Feb 24, 2022
Merged

Conversation

zwu52
Copy link
Member

@zwu52 zwu52 commented Nov 29, 2021

To resolve issue

@google-cla google-cla bot added the cla: yes label Nov 29, 2021
@changeset-bot
Copy link

changeset-bot bot commented Nov 29, 2021

⚠️ No Changeset found

Latest commit: e8ddd8b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@zwu52 zwu52 requested review from Feiyang1 and hsubox76 November 29, 2021 19:24
@google-oss-bot
Copy link
Contributor

Binary Size Report

Affected SDKs

  • @firebase/analytics

    Type Base (086df7c) Head (c812778) Diff
    browser ? 18.9 kB ? (?)
    esm5 ? 23.5 kB ? (?)
    main ? 24.6 kB ? (?)
    module ? 18.9 kB ? (?)
  • @firebase/analytics-compat

    Type Base (086df7c) Head (c812778) Diff
    browser ? 2.57 kB ? (?)
    esm5 ? 2.80 kB ? (?)
    main ? 3.24 kB ? (?)
    module ? 2.57 kB ? (?)
  • @firebase/api-documenter

    Type Base (086df7c) Head (c812778) Diff
    main ? 3.95 kB ? (?)
  • @firebase/app

    Type Base (086df7c) Head (c812778) Diff
    browser ? 7.74 kB ? (?)
    esm5 ? 9.37 kB ? (?)
    main ? 10.2 kB ? (?)
    module ? 7.74 kB ? (?)
  • @firebase/app-check

    Type Base (086df7c) Head (c812778) Diff
    browser ? 25.1 kB ? (?)
    esm5 ? 29.7 kB ? (?)
    main ? 30.9 kB ? (?)
    module ? 25.1 kB ? (?)
  • @firebase/app-check-compat

    Type Base (086df7c) Head (c812778) Diff
    browser ? 2.27 kB ? (?)
    esm5 ? 2.48 kB ? (?)
    main ? 2.94 kB ? (?)
    module ? 2.27 kB ? (?)
  • @firebase/app-compat

    Type Base (086df7c) Head (c812778) Diff
    browser ? 5.28 kB ? (?)
    esm5 ? 6.13 kB ? (?)
    lite ? 4.06 kB ? (?)
    main ? 6.73 kB ? (?)
    module ? 5.28 kB ? (?)
  • @firebase/auth-compat

    Type Base (086df7c) Head (c812778) Diff
    browser ? 19.8 kB ? (?)
    esm5 ? 26.6 kB ? (?)
    main ? 29.1 kB ? (?)
    module ? 19.8 kB ? (?)
  • @firebase/auth/cordova

    Type Base (086df7c) Head (c812778) Diff
    browser ? 179 kB ? (?)
    module ? 179 kB ? (?)
  • @firebase/auth/internal

    Type Base (086df7c) Head (c812778) Diff
    browser ? 163 kB ? (?)
    esm5 ? 211 kB ? (?)
    main ? 179 kB ? (?)
    module ? 163 kB ? (?)
  • @firebase/auth/react-native

    Type Base (086df7c) Head (c812778) Diff
    browser ? 162 kB ? (?)
    module ? 162 kB ? (?)
  • @firebase/component

    Type Base (086df7c) Head (c812778) Diff
    browser ? 6.54 kB ? (?)
    esm5 ? 8.61 kB ? (?)
    main ? 8.99 kB ? (?)
    module ? 6.54 kB ? (?)
  • @firebase/database

    Type Base (086df7c) Head (c812778) Diff
    browser ? 247 kB ? (?)
    esm5 ? 275 kB ? (?)
    main ? 280 kB ? (?)
    module ? 247 kB ? (?)
  • @firebase/database-compat

    Type Base (086df7c) Head (c812778) Diff
    browser ? 17.9 kB ? (?)
    esm5 ? 21.0 kB ? (?)
    main ? 21.7 kB ? (?)
    module ? 17.9 kB ? (?)
  • @firebase/database-compat/standalone

    Type Base (086df7c) Head (c812778) Diff
    main ? 369 kB ? (?)
  • @firebase/firestore

    Type Base (086df7c) Head (c812778) Diff
    browser ? 227 kB ? (?)
    esm5 ? 284 kB ? (?)
    main ? 426 kB ? (?)
    module ? 227 kB ? (?)
    react-native ? 228 kB ? (?)
  • @firebase/firestore-compat

    Type Base (086df7c) Head (c812778) Diff
    browser ? 20.5 kB ? (?)
    esm5 ? 27.6 kB ? (?)
    main ? 20.9 kB ? (?)
    module ? 20.5 kB ? (?)
    react-native ? 20.5 kB ? (?)
  • @firebase/firestore-lite

    Type Base (086df7c) Head (c812778) Diff
    browser ? 72.4 kB ? (?)
    esm5 ? 85.7 kB ? (?)
    main ? 125 kB ? (?)
    module ? 72.4 kB ? (?)
    react-native ? 72.6 kB ? (?)
  • @firebase/functions

    Type Base (086df7c) Head (c812778) Diff
    browser ? 8.99 kB ? (?)
    esm5 ? 11.1 kB ? (?)
    main ? 11.8 kB ? (?)
    module ? 8.99 kB ? (?)
  • @firebase/functions-compat

    Type Base (086df7c) Head (c812778) Diff
    browser ? 1.67 kB ? (?)
    esm5 ? 1.83 kB ? (?)
    main ? 2.20 kB ? (?)
    module ? 1.67 kB ? (?)
  • @firebase/installations

    Type Base (086df7c) Head (c812778) Diff
    browser ? 17.3 kB ? (?)
    esm5 ? 22.3 kB ? (?)
    main ? 23.1 kB ? (?)
    module ? 17.3 kB ? (?)
  • @firebase/installations-compat

    Type Base (086df7c) Head (c812778) Diff
    browser ? 944 B ? (?)
    esm5 ? 1.16 kB ? (?)
    main ? 1.48 kB ? (?)
    module ? 944 B ? (?)
  • @firebase/logger

    Type Base (086df7c) Head (c812778) Diff
    esm5 ? 4.65 kB ? (?)
    main ? 5.32 kB ? (?)
    module ? 3.25 kB ? (?)
  • @firebase/messaging

    Type Base (086df7c) Head (c812778) Diff
    browser ? 20.8 kB ? (?)
    esm5 ? 26.1 kB ? (?)
    main ? 26.8 kB ? (?)
    module ? 20.8 kB ? (?)
  • @firebase/messaging-compat

    Type Base (086df7c) Head (c812778) Diff
    browser ? 2.08 kB ? (?)
    esm5 ? 2.51 kB ? (?)
    main ? 2.90 kB ? (?)
    module ? 2.08 kB ? (?)
  • @firebase/messaging-sw

    Type Base (086df7c) Head (c812778) Diff
    module ? 22.8 kB ? (?)
  • @firebase/performance

    Type Base (086df7c) Head (c812778) Diff
    browser ? 28.4 kB ? (?)
    esm5 ? 30.1 kB ? (?)
    main ? 30.6 kB ? (?)
    module ? 28.4 kB ? (?)
  • @firebase/performance-compat

    Type Base (086df7c) Head (c812778) Diff
    browser ? 1.10 kB ? (?)
    esm5 ? 1.37 kB ? (?)
    main ? 1.69 kB ? (?)
    module ? 1.10 kB ? (?)
  • @firebase/remote-config

    Type Base (086df7c) Head (c812778) Diff
    browser ? 18.8 kB ? (?)
    esm5 ? 23.7 kB ? (?)
    main ? 24.9 kB ? (?)
    module ? 18.8 kB ? (?)
  • @firebase/remote-config-compat

    Type Base (086df7c) Head (c812778) Diff
    browser ? 1.85 kB ? (?)
    esm5 ? 2.76 kB ? (?)
    main ? 3.09 kB ? (?)
    module ? 1.85 kB ? (?)
  • @firebase/rules-unit-testing

    Type Base (086df7c) Head (c812778) Diff
    main ? 17.6 kB ? (?)
  • @firebase/storage

    Type Base (086df7c) Head (c812778) Diff
    browser ? 54.5 kB ? (?)
    esm5 ? 60.5 kB ? (?)
    main ? 57.3 kB ? (?)
    module ? 54.5 kB ? (?)
  • @firebase/storage-compat

    Type Base (086df7c) Head (c812778) Diff
    browser ? 5.67 kB ? (?)
    esm5 ? 8.41 kB ? (?)
    main ? 8.82 kB ? (?)
    module ? 5.67 kB ? (?)
  • @firebase/util

    Type Base (086df7c) Head (c812778) Diff
    browser ? 20.5 kB ? (?)
    esm5 ? 21.9 kB ? (?)
    main ? 26.7 kB ? (?)
    module ? 20.5 kB ? (?)
  • @firebase/webchannel-wrapper

    Type Base (086df7c) Head (c812778) Diff
    esm5 ? 44.7 kB ? (?)
    main ? 50.8 kB ? (?)
    module ? 43.2 kB ? (?)
  • bundle

    Click to show 34 binary size changes.
    Type Base (086df7c) Head (c812778) Diff
    analytics (logEvent) ? 34.7 kB ? (?)
    app-check (CustomProvider) ? 27.4 kB ? (?)
    app-check (ReCaptchaEnterpriseProvider) ? 29.6 kB ? (?)
    app-check (ReCaptchaV3Provider) ? 29.6 kB ? (?)
    auth (Anonymous) ? 56.9 kB ? (?)
    auth (EmailAndPassword) ? 61.0 kB ? (?)
    auth (GoogleFBTwitterGitHubPopup) ? 80.7 kB ? (?)
    auth (GooglePopup) ? 80.4 kB ? (?)
    auth (GoogleRedirect) ? 80.6 kB ? (?)
    auth (Phone) ? 66.9 kB ? (?)
    database (Append to a list of data) ? 137 kB ? (?)
    database (Filtering data) ? 136 kB ? (?)
    database (Listen for child events) ? 152 kB ? (?)
    database (Listen for value events) ? 152 kB ? (?)
    database (Read data once) ? 144 kB ? (?)
    database (Save data as transactions) ? 154 kB ? (?)
    database (Sort data) ? 138 kB ? (?)
    database (Write data) ? 136 kB ? (?)
    firestore (Query) ? 190 kB ? (?)
    firestore (Read data once) ? 195 kB ? (?)
    firestore (Realtime updates) ? 180 kB ? (?)
    firestore-lite (Query) ? 59.4 kB ? (?)
    firestore-lite (Read data once) ? 57.2 kB ? (?)
    functions (call) ? 18.3 kB ? (?)
    messaging (send + receive) ? 37.8 kB ? (?)
    performance (trace) ? 42.3 kB ? (?)
    remote-config (getAndFetch) ? 36.7 kB ? (?)
    storage (getDownloadURL) ? 28.7 kB ? (?)
    storage (getMetadata) ? 28.1 kB ? (?)
    storage (list + listAll) ? 27.5 kB ? (?)
    storage (updateMetadata) ? 28.4 kB ? (?)
    storage (uploadBytes) ? 32.9 kB ? (?)
    storage (uploadBytesResumable) ? 42.4 kB ? (?)
    storage (uploadString) ? 33.1 kB ? (?)
  • firebase

    Click to show 29 binary size changes.
    Type Base (086df7c) Head (c812778) Diff
    firebase-analytics-compat.js ? 26.0 kB ? (?)
    firebase-analytics.js ? 107 kB ? (?)
    firebase-app-check-compat.js ? 22.7 kB ? (?)
    firebase-app-check.js ? 89.8 kB ? (?)
    firebase-app-compat.js ? 17.9 kB ? (?)
    firebase-app.js ? 51.3 kB ? (?)
    firebase-auth-compat.js ? 123 kB ? (?)
    firebase-auth-cordova.js ? 91 B ? (?)
    firebase-auth-react-native.js ? 101 B ? (?)
    firebase-auth.js ? 410 kB ? (?)
    firebase-compat.js ? 753 kB ? (?)
    firebase-database-compat.js ? 165 kB ? (?)
    firebase-database.js ? 603 kB ? (?)
    firebase-firestore-compat.js ? 280 kB ? (?)
    firebase-firestore-lite.js ? 245 kB ? (?)
    firebase-firestore.js ? 766 kB ? (?)
    firebase-functions-compat.js ? 7.95 kB ? (?)
    firebase-functions.js ? 30.9 kB ? (?)
    firebase-messaging-compat.js ? 38.0 kB ? (?)
    firebase-messaging-sw.js ? 102 kB ? (?)
    firebase-messaging.js ? 101 kB ? (?)
    firebase-performance-compat.js ? 30.8 kB ? (?)
    firebase-performance-standalone-compat.es2017.js ? 78.9 kB ? (?)
    firebase-performance-standalone-compat.js ? 57.0 kB ? (?)
    firebase-performance.js ? 119 kB ? (?)
    firebase-remote-config-compat.js ? 27.5 kB ? (?)
    firebase-remote-config.js ? 108 kB ? (?)
    firebase-storage-compat.js ? 38.2 kB ? (?)
    firebase-storage.js ? 145 kB ? (?)
  • functions

    Type Base (086df7c) Head (c812778) Diff
    main ? 46 B ? (?)

Test Logs

Copy link

@philippevezina philippevezina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍🏼 I've been having this issue and looking for an alternative until this fix is released. It looks like using synchronized-promise in a compiled Webpack app works.

import { initializeApp } from 'firebase/app'
import { getMessaging, onBackgroundMessage } from 'firebase/messaging/sw'
import sp from 'synchronized-promise'

const showNotification = sp((title, options) => {
  return self.registration.showNotification(title, options);
});

onBackgroundMessage(messaging, () => {
  showNotification('Title', {}));
});

@Eros1990
Copy link

Eros1990 commented Jan 13, 2022

Looks good to me 👍🏼 I've been having this issue and looking for an alternative until this fix is released. It looks like using synchronized-promise in a compiled Webpack app works.

import { initializeApp } from 'firebase/app'
import { getMessaging, onBackgroundMessage } from 'firebase/messaging/sw'
import sp from 'synchronized-promise'

const showNotification = sp((title, options) => {
  return self.registration.showNotification(title, options);
});

onBackgroundMessage(messaging, () => {
  showNotification('Title', {}));
});

This solution doesn't work in a browser 😟

@philippevezina
Copy link

Looks good to me 👍🏼 I've been having this issue and looking for an alternative until this fix is released. It looks like using synchronized-promise in a compiled Webpack app works.

import { initializeApp } from 'firebase/app'
import { getMessaging, onBackgroundMessage } from 'firebase/messaging/sw'
import sp from 'synchronized-promise'

const showNotification = sp((title, options) => {
  return self.registration.showNotification(title, options);
});

onBackgroundMessage(messaging, () => {
  showNotification('Title', {}));
});

This solution doesn't work in a browser

You are right, this is actually raising an error that I hadn't noticed when developing. That said, my notification is still displaying properly without Chrome's default notification, and it seems like the thrown error might be what is "fixing" the initial problem 😅

@masterT
Copy link

masterT commented Feb 22, 2022

@philippevezina It might work because your bundler added polyfill for path (required by deasync which is used in synchronized-promise).

@alexmaurizio
Copy link

alexmaurizio commented Feb 23, 2022

Hello!
Any update on the merging of this PR?
We have some blocking issue and we're considering forking the project while it's resolved, to avoid affecting our customers.

Resolution proposed in this PR works in our testing, and looks like it's enough.

Thank you very much :)

Copy link
Contributor

@hsubox76 hsubox76 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a changeset?

@zwu52 zwu52 merged commit 091e867 into master Feb 24, 2022
@zwu52 zwu52 deleted the fix-silent-push-warning branch February 24, 2022 17:03
@masterT
Copy link

masterT commented Feb 24, 2022

🙏

@xblitz
Copy link

xblitz commented Mar 20, 2022

I am still having the same issues as described in #5516 ( currently on [email protected] ) , getting 2 notifications windows, first is the This site has been updated in the background and then my configured notification.
This is how I declared the onBackgroundMessage:

onBackgroundMessage(messaging, (payload) => {
  const notificationTitle = "Message from " + payload.data["contact"];
  const notificationOptions = {
    body: payload.data["message"],
    icon: "/img/icons/android-chrome-192x192.png",
    data: payload.data,
  };
  self.registration.showNotification(notificationTitle, notificationOptions);
  console.log("[service-worker] Received background message ", payload);
});

@Eros1990
Copy link

I am still having the same issues as described in #5516 ( currently on [email protected] ) , getting 2 notifications windows, first is the This site has been updated in the background and then my configured notification. This is how I declared the onBackgroundMessage:

onBackgroundMessage(messaging, (payload) => {
  const notificationTitle = "Message from " + payload.data["contact"];
  const notificationOptions = {
    body: payload.data["message"],
    icon: "/img/icons/android-chrome-192x192.png",
    data: payload.data,
  };
  self.registration.showNotification(notificationTitle, notification options);
  console.log("[service-worker] Received background message ", payload);
});

Did you try something like this?

onBackgroundMessage(messaging, (payload) => {
  const notificationTitle = "Message from " + payload.data["contact"];
  const notificationOptions = {
    body: payload.data["message"],
    icon: "/img/icons/android-chrome-192x192.png",
    data: payload.data,
  };
  console.log("[service-worker] Received background message ", payload);
  return self.registration.showNotification(notificationTitle, notificationOptions);
});

@xblitz
Copy link

xblitz commented Mar 21, 2022

oh yeah, for some reason I didn't see that last comment on the original issue (or didn't see that it was adding a return). so now I just did, and that actually works. but I really don't understand why! I mean, if it works it's all good, but I am curious though as to why this works.

@philippevezina
Copy link

oh yeah, for some reason I didn't see that last comment on the original issue (or didn't see that it was adding a return). so now I just did, and that actually works. but I really don't understand why! I mean, if it works it's all good, but I am curious though as to why this works.

self.registration.showNotification is actually async. Firebase uses await on the callback method you pass to onBackgroundMessage which makes it wait for the notification to be completely sent before continuing. If you don't return the Promise of showNotification, the notification won't be sent yet once onPush completes. That said, Chrome expects a notification to be shown when a onPush is triggered. When none is sent, it will push a default notification.

@firebase firebase locked and limited conversation to collaborators Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JS 9.0.2 - Notification Twice using onBackgroundMessage
8 participants