Skip to content

Commit b66908d

Browse files
authored
Allow aggregations to work with multi-tab. (#7212)
* Allow aggregations to work with multi-tab. Fixes #7198. A proper long-term solution for multi-tab should be implemented. * Create .changeset/three-months-lie.md * Improve the change log entry. * Skip the relevant test. * Skip the relevant test.
1 parent 8c44d58 commit b66908d

File tree

3 files changed

+16
-17
lines changed

3 files changed

+16
-17
lines changed

.changeset/three-months-lie.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@firebase/firestore": patch
3+
---
4+
5+
Fix a bug that sometimes prevented aggregations from being run when muli-tab persistence was enabled.

packages/firestore/src/core/firestore_client.ts

+5-15
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ import { newSerializer } from '../platform/serializer';
4242
import { newTextEncoder } from '../platform/text_serializer';
4343
import { Datastore, invokeRunAggregationQueryRpc } from '../remote/datastore';
4444
import {
45-
canUseNetwork,
4645
RemoteStore,
4746
remoteStoreDisableNetwork,
4847
remoteStoreEnableNetwork,
@@ -536,20 +535,11 @@ export function firestoreClientRunAggregateQuery(
536535
// to the implementation in firestoreClientGetDocumentsViaSnapshotListener
537536
// above
538537
try {
539-
const remoteStore = await getRemoteStore(client);
540-
if (!canUseNetwork(remoteStore)) {
541-
deferred.reject(
542-
new FirestoreError(
543-
Code.UNAVAILABLE,
544-
'Failed to get aggregate result because the client is offline.'
545-
)
546-
);
547-
} else {
548-
const datastore = await getDatastore(client);
549-
deferred.resolve(
550-
invokeRunAggregationQueryRpc(datastore, query, aggregates)
551-
);
552-
}
538+
// TODO(b/277628384): check `canUseNetwork()` and handle multi-tab.
539+
const datastore = await getDatastore(client);
540+
deferred.resolve(
541+
invokeRunAggregationQueryRpc(datastore, query, aggregates)
542+
);
553543
} catch (e) {
554544
deferred.reject(e as Error);
555545
}

packages/firestore/test/integration/api/aggregation.test.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,9 @@ apiDescribe('Count queries', (persistence: boolean) => {
115115
});
116116
});
117117

118-
it('getCountFromServer fails if user is offline', () => {
118+
// TODO(b/277628384): Re-enable this test once b/277628384 is fixed.
119+
// eslint-disable-next-line no-restricted-properties
120+
it.skip('getCountFromServer fails if user is offline', () => {
119121
return withEmptyTestCollection(persistence, async (coll, firestore) => {
120122
await disableNetwork(firestore);
121123
await expect(getCountFromServer(coll)).to.be.eventually.rejectedWith(
@@ -293,7 +295,9 @@ apiDescribe('Aggregation queries', (persistence: boolean) => {
293295
});
294296
});
295297

296-
it('getAggregateFromServer fails if user is offline', () => {
298+
// TODO(b/277628384): Re-enable this test once b/277628384 is fixed.
299+
// eslint-disable-next-line no-restricted-properties
300+
it.skip('getAggregateFromServer fails if user is offline', () => {
297301
return withEmptyTestCollection(persistence, async (coll, firestore) => {
298302
await disableNetwork(firestore);
299303
await expect(getCountFromServer(coll)).to.be.eventually.rejectedWith(

0 commit comments

Comments
 (0)