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

New way to config Firestore SDK Cache. #7015

Merged
merged 25 commits into from
Mar 15, 2023
Merged

New way to config Firestore SDK Cache. #7015

merged 25 commits into from
Mar 15, 2023

Conversation

wu-hui
Copy link
Contributor

@wu-hui wu-hui commented Feb 8, 2023

@changeset-bot
Copy link

changeset-bot bot commented Feb 8, 2023

🦋 Changeset detected

Latest commit: e6b8250

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

This PR includes changesets to release 3 packages
Name Type
@firebase/firestore Minor
firebase Patch
@firebase/firestore-compat 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

Copy link
Contributor

@MarkDuckworth MarkDuckworth left a comment

Choose a reason for hiding this comment

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

I like the new interface! I added a few comments for your consideration.

@MarkDuckworth MarkDuckworth assigned wu-hui and unassigned MarkDuckworth Feb 8, 2023
@wu-hui wu-hui force-pushed the wuandy/NewCacheConfig branch 3 times, most recently from 6c83a97 to 6535efa Compare February 21, 2023 18:31
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Feb 21, 2023

Size Report 1

Affected Products

  • @firebase/firestore

    TypeBase (a2505bd)Merge (ffcd1c2)Diff
    browser274 kB278 kB+3.90 kB (+1.4%)
    esm5341 kB346 kB+4.48 kB (+1.3%)
    main549 kB554 kB+4.44 kB (+0.8%)
    module274 kB278 kB+3.90 kB (+1.4%)
    react-native274 kB278 kB+3.90 kB (+1.4%)
  • @firebase/firestore-lite

    TypeBase (a2505bd)Merge (ffcd1c2)Diff
    browser87.4 kB87.7 kB+268 B (+0.3%)
    esm5105 kB106 kB+268 B (+0.3%)
    main149 kB149 kB+31 B (+0.0%)
    module87.4 kB87.7 kB+268 B (+0.3%)
    react-native87.6 kB87.9 kB+268 B (+0.3%)
  • bundle

    12 size changes

    TypeBase (a2505bd)Merge (ffcd1c2)Diff
    firestore (Persistence)282 kB283 kB+1.72 kB (+0.6%)
    firestore (Query Cursors)220 kB222 kB+1.50 kB (+0.7%)
    firestore (Query)218 kB219 kB+1.50 kB (+0.7%)
    firestore (Read data once)205 kB207 kB+1.43 kB (+0.7%)
    firestore (Realtime updates)207 kB209 kB+1.43 kB (+0.7%)
    firestore (Transaction)189 kB190 kB+1.46 kB (+0.8%)
    firestore (Write data)188 kB190 kB+1.45 kB (+0.8%)
    firestore-lite (Query Cursors)81.4 kB81.6 kB+180 B (+0.2%)
    firestore-lite (Query)77.6 kB77.7 kB+180 B (+0.2%)
    firestore-lite (Read data once)59.8 kB59.9 kB+69 B (+0.1%)
    firestore-lite (Transaction)84.5 kB84.7 kB+202 B (+0.2%)
    firestore-lite (Write data)69.3 kB69.5 kB+177 B (+0.3%)

  • firebase

    TypeBase (a2505bd)Merge (ffcd1c2)Diff
    firebase-compat.js747 kB750 kB+2.30 kB (+0.3%)
    firebase-firestore-compat.js321 kB323 kB+2.30 kB (+0.7%)
    firebase-firestore-lite.js94.2 kB94.4 kB+268 B (+0.3%)
    firebase-firestore.js323 kB327 kB+3.92 kB (+1.2%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/QiImZhogLW.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Feb 21, 2023

Size Analysis Report 1

This report is too large (790,994 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/Z1dB7EdbmV.html

Copy link
Contributor

@MarkDuckworth MarkDuckworth left a comment

Choose a reason for hiding this comment

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

LGTM. A few comments given for your consideration.

packages/firestore/src/api/database.ts Outdated Show resolved Hide resolved
packages/firestore/src/api/cache_config.ts Outdated Show resolved Hide resolved
packages/firestore/src/api/cache_config.ts Outdated Show resolved Hide resolved
packages/firestore/src/api/cache_config.ts Outdated Show resolved Hide resolved
packages/firestore/src/api/cache_config.ts Outdated Show resolved Hide resolved
packages/firestore/src/api/cache_config.ts Outdated Show resolved Hide resolved
packages/firestore/src/api/cache_config.ts Outdated Show resolved Hide resolved
packages/firestore/src/api/cache_config.ts Outdated Show resolved Hide resolved
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 run yarn docgen devsite to generate new files in docs-devsite? There's a message in https://github.com/firebase/firebase-js-sdk/actions/runs/4345091205/jobs/7589318276 but it's not very easy to find, sorry.

}

/**
* @deprecated This function will be removed in a future major release. Instead, set
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure but I think this tag needs to go at the bottom of the comment block. You can double check when you run yarn docs devsite.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in the latest commit.

@wu-hui wu-hui assigned hsubox76 and unassigned markarndt Mar 6, 2023
@wu-hui wu-hui closed this Mar 7, 2023
@wu-hui wu-hui reopened this Mar 7, 2023
@wu-hui wu-hui requested a review from egilmorez as a code owner March 8, 2023 17:04
@wu-hui wu-hui closed this Mar 8, 2023
@wu-hui wu-hui reopened this Mar 8, 2023
@wu-hui wu-hui merged commit 60a730e into master Mar 15, 2023
@wu-hui wu-hui deleted the wuandy/NewCacheConfig branch March 15, 2023 16:49
renkelvin pushed a commit that referenced this pull request Mar 21, 2023
* New way to config Firestore SDK Cache.

* fixups

* Fix some test failures

* Fixing dts file

* Add public comments

* API report

* Create brown-beers-tease.md

* warning messages and more tests

* Addressing comments

* Update API reports

* Fix provider test failure for node w/o indexeddb

* Fix node memory test

* rename cache to localCache

* Update API reports

* Tech writer review

* yarn docgen

* auth change

* Update API reports

* docgen devsite

* Fix changeset

* Another auth change

* more devsite fix

* Update API reports

* Rename indexeddb to persistent

---------

Co-authored-by: wu-hui <[email protected]>
dwyfrequency pushed a commit that referenced this pull request Mar 23, 2023
* New way to config Firestore SDK Cache.

* fixups

* Fix some test failures

* Fixing dts file

* Add public comments

* API report

* Create brown-beers-tease.md

* warning messages and more tests

* Addressing comments

* Update API reports

* Fix provider test failure for node w/o indexeddb

* Fix node memory test

* rename cache to localCache

* Update API reports

* Tech writer review

* yarn docgen

* auth change

* Update API reports

* docgen devsite

* Fix changeset

* Another auth change

* more devsite fix

* Update API reports

* Rename indexeddb to persistent

---------

Co-authored-by: wu-hui <[email protected]>
@google-oss-bot google-oss-bot mentioned this pull request Mar 29, 2023
@firebase firebase locked and limited conversation to collaborators Apr 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants