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.

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

Add firebase-specific screen_view params to Analytics #5070

Merged
merged 4 commits into from
Jul 16, 2021
Merged

Add firebase-specific screen_view params to Analytics #5070

merged 4 commits into from
Jul 16, 2021

Conversation

hsubox76
Copy link
Contributor

Add firebase_screen and firebase_screen_class to EventParams type and put them in the logEvent('screen_view', ...) overload. This should only affect autocomplete and type checking. Adding this to the "guides" in the docs site is also in progress.

This change is for both exp and non-modular.

API proposal: go/firebase-js-screen-view

@changeset-bot
Copy link

changeset-bot bot commented Jun 24, 2021 •
edited
Loading

🦋 Changeset detected

Latest commit: 579ee34

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@firebase/analytics-types Minor
firebase Minor
@firebase/analytics Patch
@firebase/rules-unit-testing 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

@github-actions
Copy link
Contributor

github-actions bot commented Jun 24, 2021 •
edited
Loading

Changeset File Check ⚠️

  • Changeset formatting error in following file:

    Package "firebase-compat-typings-test" must depend on the current version of "firebase-exp": "9.0.0-beta.6" vs "file:../../packages-exp/firebase-exp"
Package "firebase-compat-typings-test" must depend on the current version of "firebase-exp": "9.0.0-beta.6" vs "file:../../packages-exp/firebase-exp"
Error: could not parse changeset - invalid frontmatter: ---
@firebase/storage: minor
---

Add NodeJS support to Cloud Storage for Firebase. This release changes the `main` field in package.json to point to a Node specific build. If you are building a bundle for borwser usage, please make sure that your bundler uses the `browser` field (the default).

    at Object.parseChangesetFile [as default] (/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/@changesets/parse/dist/parse.cjs.dev.js:35:11)
    at /home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/@changesets/read/dist/read.cjs.dev.js:137:71
    at async Promise.all (index 0)
    at async Object.getChangesets [as default] (/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/@changesets/read/dist/read.cjs.dev.js:141:48)
    at async Object.getReleasePlan [as default] (/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/@changesets/get-release-plan/dist/get-release-plan.cjs.dev.js:70:22)
    at async getStatus (/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/@changesets/cli/dist/cli.cjs.dev.js:972:23)
    at async run$1 (/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/@changesets/cli/dist/cli.cjs.dev.js:1227:11)

  • Warning: This PR modifies files in the following packages but they have not been included in the changeset file:

    • @firebase/analytics-exp

    Make sure this was intentional.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jun 24, 2021 •
edited
Loading

Binary Size Report

Affected SDKs

  • @firebase/analytics

    Type Base (3ea1e1f) Head (c01b518) Diff
    main 24.2 kB 24.2 kB -38 B (-0.2%)
    module 23.4 kB 23.4 kB -38 B (-0.2%)

  • @firebase/app

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 11.0 kB 11.0 kB -26 B (-0.2%)
    esm2017 9.81 kB 9.80 kB -10 B (-0.1%)
    lite 8.95 kB 8.93 kB -14 B (-0.2%)
    lite-esm2017 7.93 kB 7.93 kB +2 B (+0.0%)
    main 10.2 kB 10.1 kB -16 B (-0.2%)
    module 11.0 kB 11.0 kB -26 B (-0.2%)
    react-native 9.87 kB 9.86 kB -16 B (-0.2%)

  • @firebase/app-check

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 22.3 kB 22.5 kB +223 B (+1.0%)
    esm2017 18.5 kB 18.7 kB +172 B (+0.9%)
    main 22.3 kB 22.5 kB +223 B (+1.0%)
    module 21.9 kB 22.1 kB +211 B (+1.0%)

  • @firebase/auth

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 181 kB 181 kB +94 B (+0.1%)
    main 181 kB 181 kB +94 B (+0.1%)
    module 181 kB 181 kB +94 B (+0.1%)

  • @firebase/component

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 8.33 kB 8.33 kB -2 B (-0.0%)
    esm2017 6.32 kB 6.32 kB -2 B (-0.0%)
    main 8.67 kB 8.67 kB -2 B (-0.0%)
    module 8.33 kB 8.33 kB -2 B (-0.0%)

  • @firebase/database

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 296 kB 296 kB -53 B (-0.0%)
    esm2017 265 kB 265 kB -41 B (-0.0%)
    main 299 kB 299 kB -25 B (-0.0%)
    module 296 kB 296 kB -53 B (-0.0%)

  • @firebase/database-compat

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 86.5 kB 86.6 kB +39 B (+0.0%)
    main 103 kB 103 kB +43 B (+0.0%)
    module 86.5 kB 86.6 kB +39 B (+0.0%)

  • @firebase/database-exp

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 246 kB 246 kB -18 B (-0.0%)
    main 278 kB 278 kB -13 B (-0.0%)
    module 246 kB 246 kB -18 B (-0.0%)

  • @firebase/firestore

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 284 kB 284 kB +163 B (+0.1%)
    esm2017 226 kB 226 kB +13 B (+0.0%)
    main 531 kB 531 kB -23 B (-0.0%)
    module 284 kB 284 kB +163 B (+0.1%)
    react-native 227 kB 227 kB +13 B (+0.0%)

  • @firebase/firestore-compat

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 28.9 kB 29.0 kB +8 B (+0.0%)
    main 38.1 kB 38.1 kB +22 B (+0.1%)
    module 28.9 kB 29.0 kB +8 B (+0.0%)
    react-native 28.7 kB 28.7 kB +8 B (+0.0%)

  • @firebase/firestore-exp

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 224 kB 224 kB +12 B (+0.0%)
    main 510 kB 510 kB +35 B (+0.0%)
    module 224 kB 224 kB +12 B (+0.0%)
    react-native 224 kB 224 kB +12 B (+0.0%)

  • @firebase/firestore-lite

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 72.5 kB 72.6 kB +42 B (+0.1%)
    main 149 kB 149 kB +141 B (+0.1%)
    module 72.5 kB 72.6 kB +42 B (+0.1%)
    react-native 72.7 kB 72.8 kB +42 B (+0.1%)

  • @firebase/firestore/bundle

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 290 kB 290 kB +20 B (+0.0%)
    esm2017 177 kB 177 kB +13 B (+0.0%)
    main 527 kB 527 kB -27 B (-0.0%)
    module 290 kB 290 kB +20 B (+0.0%)
    react-native 177 kB 177 kB +13 B (+0.0%)

  • @firebase/firestore/memory

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 216 kB 216 kB +111 B (+0.1%)
    esm2017 172 kB 172 kB +13 B (+0.0%)
    main 325 kB 325 kB -4 B (-0.0%)
    module 216 kB 216 kB +111 B (+0.1%)
    react-native 172 kB 172 kB +13 B (+0.0%)

  • @firebase/firestore/memory-bundle

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 224 kB 224 kB +13 B (+0.0%)
    esm2017 177 kB 177 kB +13 B (+0.0%)
    main 322 kB 322 kB +4 B (+0.0%)
    module 224 kB 224 kB +13 B (+0.0%)
    react-native 177 kB 177 kB +13 B (+0.0%)

  • @firebase/functions

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 10.7 kB 10.7 kB -36 B (-0.3%)
    main 11.1 kB 11.1 kB -36 B (-0.3%)
    module 10.7 kB 10.7 kB -36 B (-0.3%)

  • @firebase/logger

    Type Base (3ea1e1f) Head (c01b518) Diff
    main 5.38 kB 5.40 kB +24 B (+0.4%)

  • @firebase/rules-unit-testing

    Type Base (3ea1e1f) Head (c01b518) Diff
    main 14.8 kB 15.3 kB +427 B (+2.9%)

  • @firebase/storage

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 63.4 kB 63.7 kB +305 B (+0.5%)
    esm2017 54.6 kB 54.9 kB +289 B (+0.5%)
    main 63.8 kB 55.4 kB -8.39 kB (-13.1%)
    module 63.4 kB 63.7 kB +305 B (+0.5%)

  • @firebase/storage-compat

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 10.2 kB 10.3 kB +60 B (+0.6%)
    main 29.1 kB 29.3 kB +205 B (+0.7%)
    module 10.2 kB 10.3 kB +60 B (+0.6%)

  • @firebase/storage-exp

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 51.5 kB 51.6 kB +110 B (+0.2%)
    main 52.6 kB 53.0 kB +413 B (+0.8%)
    module 51.5 kB 51.6 kB +110 B (+0.2%)

  • @firebase/util

    Type Base (3ea1e1f) Head (c01b518) Diff
    browser 21.2 kB 21.2 kB -19 B (-0.1%)
    esm2017 20.0 kB 19.9 kB -19 B (-0.1%)
    main 25.8 kB 25.8 kB +2 B (+0.0%)
    module 21.2 kB 21.2 kB -19 B (-0.1%)

  • @firebase/webchannel-wrapper

    Type Base (3ea1e1f) Head (c01b518) Diff
    esm2017 43.2 kB 43.2 kB +19 B (+0.0%)
    main 50.7 kB 50.7 kB +19 B (+0.0%)
    module 44.6 kB 44.8 kB +140 B (+0.3%)

  • firebase

    Type Base (3ea1e1f) Head (c01b518) Diff
    firebase-app-check.js 22.7 kB 22.8 kB +92 B (+0.4%)
    firebase-auth.js 177 kB 177 kB +96 B (+0.1%)
    firebase-database.js 187 kB 187 kB +21 B (+0.0%)
    firebase-firestore.js 336 kB 336 kB +168 B (+0.1%)
    firebase-firestore.memory.js 271 kB 271 kB +172 B (+0.1%)
    firebase-storage.js 41.5 kB 41.6 kB +69 B (+0.2%)
    firebase.js 893 kB 893 kB +526 B (+0.1%)

Test Logs

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jun 24, 2021 •
edited
Loading

Size Analysis Report

Affected Products

Diffs between base commit (3ea1e1f) and head commit (3c09b22) are too large (257,083 characters) to display.

Please check below links to see details from the original test log.

@hsubox76 hsubox76 mentioned this pull request Jun 25, 2021
Merged
Feiyang1
Feiyang1 approved these changes Jul 7, 2021
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.

Approved for code changes.

@Feiyang1 Feiyang1 removed their assignment Jul 7, 2021
egilmorez
egilmorez reviewed Jul 15, 2021
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.

Only one nit for you: if screen_name and screen_class are literals, we should backtick them.

Thanks!

packages-exp/analytics-exp/src/public-types.ts Outdated Show resolved Hide resolved
egilmorez
egilmorez approved these changes Jul 16, 2021
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.

Thanks Christina!

@hsubox76 hsubox76 merged commit 02586c9 into master Jul 16, 2021
@hsubox76 hsubox76 deleted the ch-analytics-screenview branch July 16, 2021 00:39
@google-oss-bot google-oss-bot mentioned this pull request Jul 20, 2021
Merged
@hsubox76 hsubox76 mentioned this pull request Jul 27, 2021
Open
@firebase firebase locked and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Feiyang1 Feiyang1 Feiyang1 approved these changes

@egilmorez egilmorez egilmorez approved these changes

@hiranya911 hiranya911 Awaiting requested review from hiranya911

Assignees

@egilmorez egilmorez

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@hsubox76 @google-oss-bot @Feiyang1 @egilmorez