Skip to content

FCM Pre Modularization #3234

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 18 commits into from
Aug 11, 2020
Merged

FCM Pre Modularization #3234

merged 18 commits into from
Aug 11, 2020

Conversation

zwu52
Copy link
Member

@zwu52 zwu52 commented Jun 18, 2020 •
edited
Loading

[ariane/4026631 IS APPROVED]

This PR implement "Milestone I" in section: API Change Plan: Deprecation, Removal, Addition. Specially, this PR:

Mark Deprecate:

  • setBackgroundHandler (instead divert developer to use onBackgroundMessage)
  • onTokenRefresh (currently no-op and we have no plan to implement in foreseeable future)
  • useVapidKey (divert developer to use getToken(options:{serviceWorkerRegistration, vapidKey})
  • useServiceWorker (divert developer to use getToken(options:{serviceWorkerRegistration, vapidKey})
  • getToken() (divert developer to use getToken(options:{serviceWorkerRegistration, vapidKey})

Add:

  • getToken(options:{serviceWorkerRegistration, vapidKey}
  • onBackgroundMessage
  • Typing for onBackgroundMessage: this would make easy for user to know what parameters are gettable.

@zwu52 zwu52 requested a review from egilmorez as a code owner June 18, 2020 16:34
@zwu52 zwu52 marked this pull request as draft June 18, 2020 16:34
@zwu52 zwu52 requested review from Feiyang1, hsubox76 and a team June 18, 2020 16:35
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jun 18, 2020 •
edited
Loading

Binary Size Report

Affected SDKs

  • @firebase/analytics

    Type Base (b07f822) Head (93d6d22) Diff
    esm2017 8.68 kB 9.89 kB +1.20 kB (+13.9%)
    main 9.69 kB 11.1 kB +1.42 kB (+14.7%)
    module 9.37 kB 10.8 kB +1.45 kB (+15.5%)

  • @firebase/auth

    Type Base (b07f822) Head (93d6d22) Diff
    browser ? 177 kB ? (?)

  • @firebase/database

    Type Base (b07f822) Head (93d6d22) Diff
    browser 268 kB 269 kB +849 B (+0.3%)
    esm2017 235 kB 236 kB +524 B (+0.2%)
    main 269 kB 270 kB +849 B (+0.3%)
    module 267 kB 268 kB +844 B (+0.3%)

  • @firebase/firestore

    Type Base (b07f822) Head (93d6d22) Diff
    browser 247 kB 249 kB +1.87 kB (+0.8%)
    esm2017 194 kB 195 kB +1.33 kB (+0.7%)
    main 495 kB 474 kB -20.8 kB (-4.2%)
    module 245 kB 246 kB +1.64 kB (+0.7%)
    react-native 194 kB 195 kB +1.33 kB (+0.7%)

  • @firebase/firestore/exp

    Type Base (b07f822) Head (93d6d22) Diff
    browser ? 189 kB ? (?)
    main 400 kB 467 kB +67.0 kB (+16.8%)
    module ? 189 kB ? (?)
    react-native ? 189 kB ? (?)

  • @firebase/firestore/lite

    Type Base (b07f822) Head (93d6d22) Diff
    browser ? 64.5 kB ? (?)
    main 124 kB 141 kB +17.2 kB (+13.9%)
    module ? 64.5 kB ? (?)
    react-native ? 64.6 kB ? (?)

  • @firebase/firestore/memory

    Type Base (b07f822) Head (93d6d22) Diff
    browser 185 kB 187 kB +2.24 kB (+1.2%)
    esm2017 145 kB 146 kB +1.30 kB (+0.9%)
    main 364 kB 348 kB -15.8 kB (-4.3%)
    module 183 kB 185 kB +2.04 kB (+1.1%)
    react-native 145 kB 146 kB +1.30 kB (+0.9%)

  • @firebase/messaging

    Type Base (b07f822) Head (93d6d22) Diff
    esm2017 23.2 kB 25.9 kB +2.68 kB (+11.5%)
    main 31.4 kB 34.6 kB +3.18 kB (+10.1%)
    module 30.9 kB 34.1 kB +3.17 kB (+10.2%)

  • @firebase/performance

    Type Base (b07f822) Head (93d6d22) Diff
    browser 26.7 kB 27.1 kB +376 B (+1.4%)
    esm2017 24.7 kB 25.1 kB +359 B (+1.5%)
    main 26.7 kB 27.1 kB +376 B (+1.4%)
    module 26.4 kB 26.8 kB +376 B (+1.4%)

  • @firebase/storage

    Type Base (b07f822) Head (93d6d22) Diff
    esm2017 56.4 kB 54.9 kB -1.42 kB (-2.5%)
    main 62.7 kB 61.3 kB -1.38 kB (-2.2%)
    module 62.5 kB 61.1 kB -1.39 kB (-2.2%)

  • @firebase/util

    Type Base (b07f822) Head (93d6d22) Diff
    browser 19.6 kB 20.5 kB +893 B (+4.6%)
    esm2017 17.5 kB 18.3 kB +758 B (+4.3%)
    main 19.6 kB 20.5 kB +893 B (+4.6%)
    module 18.7 kB 19.5 kB +804 B (+4.3%)

  • @firebase/webchannel-wrapper

    Type Base (b07f822) Head (93d6d22) Diff
    esm2017 37.7 kB 39.4 kB +1.69 kB (+4.5%)
    main 38.8 kB 41.0 kB +2.21 kB (+5.7%)
    module 38.4 kB 40.6 kB +2.21 kB (+5.8%)

  • firebase

    Click to show 15 binary size changes.
    Type Base (b07f822) Head (93d6d22) Diff
    firebase-analytics.js 26.6 kB 28.3 kB +1.72 kB (+6.5%)
    firebase-app.js 19.9 kB 20.0 kB +42 B (+0.2%)
    firebase-auth.js 174 kB 173 kB -184 B (-0.1%)
    firebase-database.js 187 kB 187 kB -486 B (-0.3%)
    firebase-firestore.js 285 kB 288 kB +2.30 kB (+0.8%)
    firebase-firestore.memory.js 224 kB 227 kB +3.05 kB (+1.4%)
    firebase-functions.js 9.84 kB 9.88 kB +43 B (+0.4%)
    firebase-installations.js 19.2 kB 19.2 kB +40 B (+0.2%)
    firebase-messaging.js 39.2 kB 40.9 kB +1.76 kB (+4.5%)
    firebase-performance-standalone.es2017.js 72.0 kB 72.1 kB +173 B (+0.2%)
    firebase-performance-standalone.js 47.4 kB 47.4 kB -24 B (-0.1%)
    firebase-performance.js 37.8 kB 37.8 kB -36 B (-0.1%)
    firebase-remote-config.js 37.0 kB 37.0 kB +40 B (+0.1%)
    firebase-storage.js 40.9 kB 39.9 kB -979 B (-2.4%)
    firebase.js 819 kB 823 kB +3.81 kB (+0.5%)

Test Logs

@changeset-bot
Copy link

changeset-bot bot commented Jun 28, 2020 •
edited
Loading

🦋 Changeset is good to go

Latest commit: 374dc78

We got this.

This PR includes changesets to release 13 packages
Name Type
firebase Minor
@firebase/messaging
@firebase/messaging-types Minor
rxfire Major
@firebase/testing Patch
firebase-browserify-test Patch
firebase-package-typings-test Patch
firebase-messaging-selenium-test Patch
firebase-typescript-test Patch
firebase-webpack-test Patch
@firebase/functions Patch
@firebase/functions-exp Patch
firebase-exp Patch

Not sure what this means? Click here to learn what changesets are.

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

@zwu52 zwu52 requested a review from kroikie June 29, 2020 18:26
@zwu52 zwu52 marked this pull request as ready for review June 29, 2020 18:40
kroikie
kroikie approved these changes Jul 1, 2020
View reviewed changes
Copy link

@kroikie kroikie left a comment

Choose a reason for hiding this comment

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

I would also recommend letting @egilmorez have a look at the comments.

egilmorez
egilmorez reviewed Jul 7, 2020
View reviewed changes
Copy link
Contributor

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

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

Some wordsmithing things to look at Kai. Thanks!

Feiyang1
Feiyang1 suggested changes Jul 8, 2020
View reviewed changes
Copy link
Member

@Feiyang1 Feiyang1 left a comment

Choose a reason for hiding this comment

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

Added some comments. There is a bunch of formatting issues. Can you please run prettier on all files?

@Feiyang1 Feiyang1 removed their assignment Jul 8, 2020
zwu52
zwu52 commented Jul 9, 2020
View reviewed changes
Copy link
Member Author

@zwu52 zwu52 left a comment

Choose a reason for hiding this comment

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

requested change will be included in the upcoming commit

packages/messaging/src/controllers/window-controller.ts Outdated
TAG +
'no sw has been provided explicitly. Attempting to find firebase-messaging-sw.js in default directory.'
);
await this.registerDefaultSw();
Copy link
Member Author

Choose a reason for hiding this comment

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

absolutely

@zwu52 zwu52 requested review from Feiyang1 and egilmorez July 9, 2020 23:02
egilmorez
egilmorez approved these changes Jul 10, 2020
View reviewed changes
Copy link
Contributor

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

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

LG with a one-word nit, thanks!

Feiyang1
Feiyang1 reviewed Aug 10, 2020
View reviewed changes
@zwu52 zwu52 requested a review from Feiyang1 August 11, 2020 20:00
@zwu52 zwu52 merged commit 29327b2 into master Aug 11, 2020
@google-oss-bot google-oss-bot mentioned this pull request Aug 11, 2020
Merged
@firebase firebase locked and limited conversation to collaborators Sep 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kroikie kroikie kroikie approved these changes

@Feiyang1 Feiyang1 Feiyang1 approved these changes

@egilmorez egilmorez egilmorez approved these changes

@hsubox76 hsubox76 Awaiting requested review from hsubox76

@alikn alikn Awaiting requested review from alikn

@andirayo andirayo Awaiting requested review from andirayo

@avolkovi avolkovi Awaiting requested review from avolkovi

@erikeldridge erikeldridge Awaiting requested review from erikeldridge

@sam-gc sam-gc Awaiting requested review from sam-gc

@scottcrossen scottcrossen Awaiting requested review from scottcrossen

@yuchenshi yuchenshi Awaiting requested review from yuchenshi

@zijianjoy zijianjoy Awaiting requested review from zijianjoy

Assignees

@kroikie kroikie

@Feiyang1 Feiyang1

@egilmorez egilmorez

@hsubox76 hsubox76

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@zwu52 @google-oss-bot @kroikie @Feiyang1 @egilmorez @hsubox76