Skip to content

Update Firestore type names #5135

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 7 commits into from
Jul 27, 2021
Merged

Update Firestore type names #5135

merged 7 commits into from
Jul 27, 2021

Conversation

Feiyang1
Copy link
Member

@Feiyang1 Feiyang1 commented Jul 9, 2021 •
edited
Loading

go/js-v9-types-naming

@Feiyang1 Feiyang1 added the v9 label Jul 9, 2021
@Feiyang1 Feiyang1 added this to the v9 GA milestone Jul 9, 2021
@changeset-bot
Copy link

changeset-bot bot commented Jul 9, 2021 •
edited
Loading

⚠️ No Changeset found

Latest commit: 69bc6ac

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

@github-actions
Copy link
Contributor

github-actions bot commented Jul 9, 2021 •
edited
Loading

Changeset File Check ✅

  • No modified packages are missing from the changeset file.
  • No changeset formatting errors detected.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jul 9, 2021 •
edited
Loading

Binary Size Report

Affected SDKs

  • @firebase/database

    Type Base (d56ab5c) Head (41c7df7) Diff
    browser 296 kB 296 kB -62 B (-0.0%)
    esm2017 265 kB 265 kB -14 B (-0.0%)
    main 299 kB 299 kB -62 B (-0.0%)
    module 296 kB 296 kB -62 B (-0.0%)

  • @firebase/database-exp

    Type Base (d56ab5c) Head (41c7df7) Diff
    browser 246 kB 246 kB -26 B (-0.0%)
    main 278 kB 278 kB -114 B (-0.0%)
    module 246 kB 246 kB -26 B (-0.0%)

  • @firebase/firestore

    Type Base (d56ab5c) Head (41c7df7) Diff
    browser 285 kB 285 kB +74 B (+0.0%)
    main 531 kB 531 kB -292 B (-0.1%)
    module 285 kB 285 kB +74 B (+0.0%)

  • @firebase/firestore-exp

    Type Base (d56ab5c) Head (41c7df7) Diff
    browser 224 kB 224 kB -8 B (-0.0%)
    main 511 kB 510 kB -376 B (-0.1%)
    module 224 kB 224 kB -8 B (-0.0%)
    react-native 224 kB 224 kB -8 B (-0.0%)

  • @firebase/firestore-lite

    Type Base (d56ab5c) Head (41c7df7) Diff
    browser 72.7 kB 72.7 kB -8 B (-0.0%)
    main 149 kB 149 kB -184 B (-0.1%)
    module 72.7 kB 72.7 kB -8 B (-0.0%)
    react-native 72.9 kB 72.9 kB -8 B (-0.0%)

  • @firebase/firestore/bundle

    Type Base (d56ab5c) Head (41c7df7) Diff
    main 527 kB 527 kB -286 B (-0.1%)

  • @firebase/firestore/memory

    Type Base (d56ab5c) Head (41c7df7) Diff
    main 325 kB 325 kB -180 B (-0.1%)

  • @firebase/firestore/memory-bundle

    Type Base (d56ab5c) Head (41c7df7) Diff
    main 322 kB 322 kB -174 B (-0.1%)

  • @firebase/storage

    Type Base (d56ab5c) Head (41c7df7) Diff
    browser 63.7 kB 63.6 kB -64 B (-0.1%)
    esm2017 54.9 kB 54.9 kB -33 B (-0.1%)
    main 55.4 kB 55.4 kB -33 B (-0.1%)
    module 63.7 kB 63.6 kB -64 B (-0.1%)

  • @firebase/storage-compat

    Type Base (d56ab5c) Head (41c7df7) Diff
    browser 10.3 kB 10.2 kB -52 B (-0.5%)
    main 29.3 kB 29.2 kB -64 B (-0.2%)
    module 10.3 kB 10.2 kB -52 B (-0.5%)

  • @firebase/storage-exp

    Type Base (d56ab5c) Head (41c7df7) Diff
    browser 51.6 kB 51.6 kB +19 B (+0.0%)
    main 53.0 kB 53.0 kB +19 B (+0.0%)
    module 51.6 kB 51.6 kB +19 B (+0.0%)

  • firebase

    Type Base (d56ab5c) Head (41c7df7) Diff
    firebase-database.js 187 kB 187 kB -3 B (-0.0%)
    firebase-storage.js 41.6 kB 41.5 kB -58 B (-0.1%)
    firebase.js 894 kB 894 kB -61 B (-0.0%)

Test Logs

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jul 9, 2021 •
edited
Loading

Size Analysis Report

Affected Products

No changes between base commit (d56ab5c) and head commit (41c7df7).

schmidt-sebastian
schmidt-sebastian approved these changes Jul 13, 2021
View reviewed changes
@Feiyang1 Feiyang1 changed the title Update Firestore type names [WIP] Update Firestore type names Jul 19, 2021
@Feiyang1 Feiyang1 changed the title [WIP] Update Firestore type names Update Firestore type names Jul 22, 2021
Feiyang1
Feiyang1 commented Jul 22, 2021
View reviewed changes
@Feiyang1 Feiyang1 assigned egilmorez and unassigned Feiyang1 Jul 22, 2021
egilmorez
egilmorez reviewed Jul 23, 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 for putting me on the review Feiyang!

One meta question: is there a logical way to separate files with publicly-viewable comments from internal-only files? If so, that might be a good way to narrow down the size/scope of these kinds of PRs.

@Feiyang1
Copy link
Member Author

Thanks for putting me on the review Feiyang!

One meta question: is there a logical way to separate files with publicly-viewable comments from internal-only files? If so, that might be a good way to narrow down the size/scope of these kinds of PRs.

In simpler packages like app, analytics, remote-config, all public APIs are defined in api.ts and public-types.ts, but it's difficult to enforce the same convention for auth, firestore, database that are more complex.

The best way to identify public docs is looking for the @public annotation. I wonder if we can make a GHA that highlights these locations in PRs. I have seen similar things for highlighting lint errors, so it should be doable. I will investigate.

hsubox76
hsubox76 approved these changes Jul 23, 2021
View reviewed changes
egilmorez
egilmorez approved these changes Jul 24, 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.

LG with nits, thanks!

@Feiyang1 Feiyang1 merged commit b62b6df into master Jul 27, 2021
@Feiyang1 Feiyang1 deleted the fei-type-names branch July 27, 2021 17:22
@Feiyang1 Feiyang1 mentioned this pull request Jul 27, 2021
Closed
@firebase firebase locked and limited conversation to collaborators Aug 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@egilmorez egilmorez egilmorez approved these changes

@hsubox76 hsubox76 hsubox76 approved these changes

@schmidt-sebastian schmidt-sebastian schmidt-sebastian approved these changes

@hiranya911 hiranya911 Awaiting requested review from hiranya911

Assignees

@egilmorez egilmorez

Labels
v9
Projects
None yet
Milestone
v9 GA
Development

Successfully merging this pull request may close these issues.
5 participants
@Feiyang1 @google-oss-bot @egilmorez @hsubox76 @schmidt-sebastian