Skip to content

Commit cf39fe6

Browse files
build(firestore): stop zone-wrapping functions that schedule no work (#3842)
Query and reference builders (where, orderBy, query, collection, doc and the rest), field values (increment, deleteField, vector, sum), the snapshot and query comparators, and setLogLevel only build or compare values. In @angular/fire/firestore and its lite entry point, the zone wrapper did nothing for them except print a false injection-context warning and, in zone.js apps, trigger an unneeded change detection pass when called inside an injection context but outside the Angular zone. No export or type changes. getFirestore, initializeFirestore, connectFirestoreEmulator, writeBatch and the cache index functions stay wrapped: they register listeners or start the Firestore client. New specs record every task each unwrapped function schedules, for all 38 null entries, and fail if a null entry has no sample call. Fixes #3841
1 parent 34d0d84 commit cf39fe6

6 files changed

Lines changed: 333 additions & 126 deletions

File tree

‎src/firestore/firebase.ts‎

Lines changed: 0 additions & 50 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/firestore/lite/firebase.ts‎

Lines changed: 0 additions & 50 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/firestore/lite/lite.spec.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
import { TestBed } from '@angular/core/testing';
22
import { FirebaseApp, getApp, initializeApp, provideFirebaseApp } from '@angular/fire/app';
3+
import * as angularFire from '@angular/fire/firestore/lite';
34
import { Firestore, connectFirestoreEmulator, getFirestore, provideFirestore } from '@angular/fire/firestore/lite';
5+
import { deleteApp, initializeApp as initializeSdkApp } from 'firebase/app';
6+
import * as sdk from 'firebase/firestore/lite';
47
import { COMMON_CONFIG, firestoreEmulatorPort } from '../../test-config';
58
import { rando } from '../../utils';
9+
import { UNWRAPPED_FIRESTORE_FUNCTIONS, expectExactlyTheseUnwrapped, expectSynchronousAndTaskFree } from '../zone-wrap-spec-support';
610

711
describe('Firestore-lite', () => {
812
let app: FirebaseApp;
@@ -37,3 +41,68 @@ describe('Firestore-lite', () => {
3741
});
3842

3943
});
44+
45+
// Lite is a separate bundle with its own function objects, so the full SDK's test does not cover it.
46+
describe('Firestore Lite functions re-exported without the zone wrapper', () => {
47+
48+
it('each one returns synchronously and schedules no work', async () => {
49+
const sdkApp = initializeSdkApp(COMMON_CONFIG, rando());
50+
const unusedSdkApp = initializeSdkApp(COMMON_CONFIG, rando());
51+
try {
52+
const firestore = sdk.getFirestore(sdkApp);
53+
sdk.connectFirestoreEmulator(firestore, 'localhost', firestoreEmulatorPort);
54+
// The reads below start firestore's client, so calls taking an instance use one nothing has started.
55+
const unusedFirestore = sdk.getFirestore(unusedSdkApp);
56+
const items = sdk.collection(firestore, 'unwrapped-lite');
57+
const itemRef = sdk.doc(items, rando());
58+
await sdk.setDoc(itemRef, { a: 1 });
59+
const snapshot = await sdk.getDoc(itemRef);
60+
const aggregate = await sdk.getAggregate(items, { total: sdk.sum('a') });
61+
const isOne = sdk.where('a', '==', 1);
62+
const isPositive = sdk.where('a', '>', 0);
63+
const itemsQuery = sdk.query(items, isOne);
64+
await expectSynchronousAndTaskFree(sdk, UNWRAPPED_FIRESTORE_FUNCTIONS, {
65+
where: sdkFunction => sdkFunction('a', '==', 1),
66+
orderBy: sdkFunction => sdkFunction('a', 'desc'),
67+
limit: sdkFunction => sdkFunction(1),
68+
limitToLast: sdkFunction => sdkFunction(1),
69+
startAt: sdkFunction => sdkFunction(1),
70+
startAfter: sdkFunction => sdkFunction(1),
71+
endAt: sdkFunction => sdkFunction(1),
72+
endBefore: sdkFunction => sdkFunction(1),
73+
and: sdkFunction => sdkFunction(isOne, isPositive),
74+
or: sdkFunction => sdkFunction(isOne, isPositive),
75+
query: sdkFunction => sdkFunction(items, sdk.or(isOne, isPositive), sdk.orderBy('a'), sdk.limit(1)),
76+
collection: sdkFunction => sdkFunction(unusedFirestore, 'unwrapped-lite'),
77+
collectionGroup: sdkFunction => sdkFunction(unusedFirestore, 'unwrapped-lite'),
78+
doc: sdkFunction => sdkFunction(items),
79+
documentId: sdkFunction => sdkFunction(),
80+
increment: sdkFunction => sdkFunction(1),
81+
deleteField: sdkFunction => sdkFunction(),
82+
vector: sdkFunction => sdkFunction([1, 2, 3]),
83+
sum: sdkFunction => sdkFunction('a'),
84+
arrayRemove: sdkFunction => sdkFunction(1),
85+
arrayUnion: sdkFunction => sdkFunction(1),
86+
serverTimestamp: sdkFunction => sdkFunction(),
87+
maximum: sdkFunction => sdkFunction(1),
88+
minimum: sdkFunction => sdkFunction(1),
89+
average: sdkFunction => sdkFunction('a'),
90+
count: sdkFunction => sdkFunction(),
91+
refEqual: sdkFunction => sdkFunction(itemRef, itemRef),
92+
queryEqual: sdkFunction => sdkFunction(itemsQuery, itemsQuery),
93+
snapshotEqual: sdkFunction => sdkFunction(snapshot, snapshot),
94+
aggregateFieldEqual: sdkFunction => sdkFunction(sdk.sum('a'), sdk.sum('a')),
95+
aggregateQuerySnapshotEqual: sdkFunction => sdkFunction(aggregate, aggregate),
96+
// 'info' is the level Firestore's logger starts at, so the call changes nothing for later specs.
97+
setLogLevel: sdkFunction => sdkFunction('info'),
98+
});
99+
} finally {
100+
await deleteApp(sdkApp);
101+
await deleteApp(unusedSdkApp);
102+
}
103+
});
104+
105+
it('reach callers as the SDK\'s own functions, and no others do', () => {
106+
expectExactlyTheseUnwrapped(angularFire, sdk, UNWRAPPED_FIRESTORE_FUNCTIONS);
107+
});
108+
});

0 commit comments

Comments
 (0)