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

Fix multi-tab snapshot listener metadata sync issue #8339

Merged
merged 6 commits into from
Jun 28, 2024
Merged

Fix multi-tab snapshot listener metadata sync issue #8339

merged 6 commits into from
Jun 28, 2024

Conversation

milaGGL
Copy link
Contributor

@milaGGL milaGGL commented Jun 26, 2024 •
edited
Loading

Fix #8314.

The previous fix #8247 introduced out-of-sync metadata issue for multi-tab users. When viewSnapshot is not defined, that means there is no change, so it should not be seen as "not-current".

@milaGGL milaGGL requested review from a team as code owners June 26, 2024 18:14
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jun 26, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 8ed81a9

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 Patch
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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jun 26, 2024 •
edited
Loading

Size Report 1

Affected Products

  • @firebase/firestore

    TypeBase (e7260e2)Merge (0489347)Diff
    browser378 kB378 kB+92 B (+0.0%)
    esm5363 kB363 kB+92 B (+0.0%)
    main582 kB582 kB+147 B (+0.0%)
    module378 kB378 kB+92 B (+0.0%)
    react-native378 kB378 kB+92 B (+0.0%)

  • bundle

    TypeBase (e7260e2)Merge (0489347)Diff
    firestore (CSI Auto Indexing Disable and Delete)268 kB268 kB+92 B (+0.0%)
    firestore (CSI Auto Indexing Enable)268 kB268 kB+92 B (+0.0%)
    firestore (Persistence)303 kB303 kB+92 B (+0.0%)
    firestore (Query Cursors)240 kB240 kB+92 B (+0.0%)
    firestore (Query)238 kB238 kB+92 B (+0.0%)
    firestore (Read data once)226 kB226 kB+92 B (+0.0%)
    firestore (Read Write w Persistence)322 kB322 kB+92 B (+0.0%)
    firestore (Realtime updates)228 kB228 kB+92 B (+0.0%)
    firestore (Transaction)205 kB205 kB+92 B (+0.0%)
    firestore (Write data)205 kB205 kB+92 B (+0.0%)

  • firebase

    TypeBase (e7260e2)Merge (0489347)Diff
    firebase-compat.js786 kB786 kB+101 B (+0.0%)
    firebase-firestore-compat.js341 kB341 kB+101 B (+0.0%)
    firebase-firestore.js437 kB437 kB+92 B (+0.0%)

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jun 26, 2024 •
edited
Loading

Size Analysis Report 1

Affected Products

  • @firebase/firestore

    • addDoc

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size124 kB124 kB+92 B (+0.1%)
      size-with-ext-deps195 kB196 kB+92 B (+0.0%)

    • deleteDoc

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size115 kB115 kB+92 B (+0.1%)
      size-with-ext-deps186 kB186 kB+92 B (+0.0%)

    • disableNetwork

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size103 kB103 kB+92 B (+0.1%)
      size-with-ext-deps175 kB175 kB+92 B (+0.1%)

    • enableIndexedDbPersistence

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size186 kB186 kB+92 B (+0.0%)
      size-with-ext-deps258 kB258 kB+92 B (+0.0%)

    • enableMultiTabIndexedDbPersistence

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size222 kB222 kB+92 B (+0.0%)
      size-with-ext-deps294 kB294 kB+92 B (+0.0%)

    • enableNetwork

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size103 kB103 kB+92 B (+0.1%)
      size-with-ext-deps175 kB175 kB+92 B (+0.1%)

    • executeWrite

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size114 kB114 kB+92 B (+0.1%)
      size-with-ext-deps186 kB186 kB+92 B (+0.0%)

    • getAggregateFromServer

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size110 kB110 kB+92 B (+0.1%)
      size-with-ext-deps182 kB182 kB+92 B (+0.1%)

    • getCountFromServer

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size110 kB111 kB+92 B (+0.1%)
      size-with-ext-deps182 kB182 kB+92 B (+0.1%)

    • getDoc

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size145 kB145 kB+92 B (+0.1%)
      size-with-ext-deps216 kB216 kB+92 B (+0.0%)

    • getDocFromServer

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size145 kB145 kB+92 B (+0.1%)
      size-with-ext-deps216 kB216 kB+92 B (+0.0%)

    • getDocs

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size146 kB146 kB+92 B (+0.1%)
      size-with-ext-deps218 kB218 kB+92 B (+0.0%)

    • getDocsFromServer

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size146 kB146 kB+92 B (+0.1%)
      size-with-ext-deps218 kB218 kB+92 B (+0.0%)

    • loadBundle

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size113 kB113 kB+92 B (+0.1%)
      size-with-ext-deps184 kB184 kB+92 B (+0.0%)

    • memoryLocalCache

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size100 kB100 kB+92 B (+0.1%)
      size-with-ext-deps172 kB172 kB+92 B (+0.1%)

    • onSnapshot

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size147 kB147 kB+92 B (+0.1%)
      size-with-ext-deps219 kB219 kB+92 B (+0.0%)

    • onSnapshotsInSync

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size136 kB136 kB+92 B (+0.1%)
      size-with-ext-deps208 kB208 kB+92 B (+0.0%)

    • persistentLocalCache

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size184 kB184 kB+92 B (+0.1%)
      size-with-ext-deps255 kB255 kB+92 B (+0.0%)

    • persistentMultipleTabManager

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size219 kB219 kB+92 B (+0.0%)
      size-with-ext-deps291 kB291 kB+92 B (+0.0%)

    • persistentSingleTabManager

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size183 kB183 kB+92 B (+0.1%)
      size-with-ext-deps255 kB255 kB+92 B (+0.0%)

    • runTransaction

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size124 kB124 kB+92 B (+0.1%)
      size-with-ext-deps195 kB195 kB+92 B (+0.0%)

    • setDoc

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size123 kB123 kB+92 B (+0.1%)
      size-with-ext-deps195 kB195 kB+92 B (+0.0%)

    • updateDoc

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size123 kB124 kB+92 B (+0.1%)
      size-with-ext-deps195 kB195 kB+92 B (+0.0%)

    • waitForPendingWrites

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size104 kB104 kB+92 B (+0.1%)
      size-with-ext-deps175 kB175 kB+92 B (+0.1%)

    • writeBatch

      Size

      TypeBase (e7260e2)Merge (0489347)Diff
      size125 kB126 kB+92 B (+0.1%)
      size-with-ext-deps197 kB197 kB+92 B (+0.0%)

Test Logs

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

wu-hui
wu-hui approved these changes Jun 28, 2024
View reviewed changes
Copy link
Contributor

@wu-hui wu-hui 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 the investigation! Your should be proud of your work.

@wu-hui wu-hui assigned milaGGL and unassigned wu-hui Jun 28, 2024
@dlarocque
Copy link
Contributor

dlarocque commented Jun 28, 2024 •
edited
Loading

Do we want a changeset for this so that we can mention it in our release notes?

@milaGGL milaGGL requested a review from a team as a code owner June 28, 2024 16:13
@milaGGL milaGGL merged commit ecadbe3 into master Jun 28, 2024
42 checks passed
@milaGGL milaGGL deleted the mila/fix-multi-tab-snapshot-listener-metadata-sync-issue branch June 28, 2024 19:09
@google-oss-bot google-oss-bot mentioned this pull request Jul 2, 2024
Merged
tom-andersen pushed a commit that referenced this pull request Jul 24, 2024
@milaGGL @tom-andersen
Fix multi-tab snapshot listener metadata sync issue (#8339)
61fcec3 
* Update sync_engine_impl.ts

* add spec test

* update comment

* typo

* update spec test

* add changeset
@surajit03 surajit03 mentioned this pull request Jul 25, 2024
Open
@Abellache-Rabah Abellache-Rabah mentioned this pull request Jul 25, 2024
Open
@admin-token-bot admin-token-bot mentioned this pull request Jul 25, 2024
Closed
@therealbenpai therealbenpai mentioned this pull request Jul 25, 2024
Closed
@kushalsdesk kushalsdesk mentioned this pull request Jul 25, 2024
Open
@Beve-hub Beve-hub mentioned this pull request Jul 25, 2024
Open
@kenn7575 kenn7575 mentioned this pull request Jul 25, 2024
Closed
@ciscoviit ciscoviit mentioned this pull request Jul 25, 2024
Merged
@lydmoon lydmoon mentioned this pull request Jul 25, 2024
Open
@sswietoniowski sswietoniowski mentioned this pull request Jul 25, 2024
Merged
@DevJPCosta DevJPCosta mentioned this pull request Jul 25, 2024
Open
@leoo1992 leoo1992 mentioned this pull request Jul 25, 2024
Merged
@22-sharmi 22-sharmi mentioned this pull request Jul 25, 2024
Open
@iky95 iky95 mentioned this pull request Jul 25, 2024
Closed
@arnonrdp arnonrdp mentioned this pull request Jul 25, 2024
Closed
@sachin2967 sachin2967 mentioned this pull request Jul 26, 2024
Open
@Samsonroyal Samsonroyal mentioned this pull request Jul 26, 2024
Open
@sonlast sonlast mentioned this pull request Jul 27, 2024
Open
@NEONKID NEONKID mentioned this pull request Jul 28, 2024
Open
@firebase firebase locked and limited conversation to collaborators Jul 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dlarocque dlarocque dlarocque approved these changes

@wu-hui wu-hui wu-hui approved these changes

Assignees

@milaGGL milaGGL

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BUG: using multi-tab persistence, the queries in the secondary tabs don't report metadata.fromCache=false
4 participants
@milaGGL @google-oss-bot @dlarocque @wu-hui