Skip to content

Commit

Permalink
Fix multi-tab snapshot listener metadata sync issue (#8339)
Browse files Browse the repository at this point in the history
* Update sync_engine_impl.ts

* add spec test

* update comment

* typo

* update spec test

* add changeset
  • Loading branch information
milaGGL authored Jun 28, 2024
1 parent 226fe8a commit ecadbe3
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 1 deletion.
6 changes: 6 additions & 0 deletions .changeset/flat-scissors-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@firebase/firestore': patch
'firebase': patch
---

Fix persistence multi-tab snapshot listener metadata sync issue.
8 changes: 7 additions & 1 deletion packages/firestore/src/core/sync_engine_impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,13 @@ export async function syncEngineEmitNewSnapsAndNotifyLocalStore(
// secondary clients to update query state.
if (viewSnapshot || remoteEvent) {
if (syncEngineImpl.isPrimaryClient) {
const isCurrent = viewSnapshot && !viewSnapshot.fromCache;
// Query state is set to `current` if:
// - There is a view change and it is up-to-date, or,
// - There is a global snapshot, the Target is current, and no changes to be resolved
const isCurrent = viewSnapshot
? !viewSnapshot.fromCache
: remoteEvent?.targetChanges.get(queryView.targetId)?.current;

syncEngineImpl.sharedClientState.updateQueryState(
queryView.targetId,
isCurrent ? 'current' : 'not-current'
Expand Down
36 changes: 36 additions & 0 deletions packages/firestore/test/unit/specs/listen_spec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1895,4 +1895,40 @@ describeSpec('Listens:', [], () => {
.restoreListen(query1, 'resume-token-1000', /* expectedCount= */ 1);
}
);

specTest(
'Global snapshots would not alter query state if there is no changes',
['multi-client'],
() => {
const query1 = query('collection');
const docA = doc('collection/a', 1000, { key: 'a' });
return (
client(0)
.becomeVisible()
.expectPrimaryState(true)
// Populate the cache first
.userListens(query1)
.watchAcksFull(query1, 1000, docA)
.expectEvents(query1, { added: [docA] })
.userUnlistens(query1)
.watchRemoves(query1)
// Listen to the query in the primary client
.userListens(query1, { resumeToken: 'resume-token-1000' })
.expectEvents(query1, {
added: [docA],
fromCache: true
})
.watchAcksFull(query1, 2000, docA)
.expectEvents(query1, { fromCache: false })
// Reproduces: https://github.com/firebase/firebase-js-sdk/issues/8314
// Watch could send a global snapshot from time to time. If there are no view changes,
// the query should not be marked as "not-current" as the Target is up to date.
.watchSnapshots(3000, [], 'resume-token-3000')
// Listen to the query in the secondary tab. The snapshot is up to date.
.client(1)
.userListens(query1)
.expectEvents(query1, { added: [docA], fromCache: false })
);
}
);
});

0 comments on commit ecadbe3

Please sign in to comment.