[Feat] Firestore onSnapshot support for bundles for feature branch. #8896

DellaBitta · 2025-04-04T16:55:40Z

Discussion

Pull request to add onSnapshot listeners support for bundles to the Firestore SSR serialization feature branch.

This change adds a series of onSnapshot overloads that match the existing onSnapshot variants (observer, individual callback functions, etc) but accepts a bundle instead of a QuerySnapshot or a DocumentSnapshot.

The toJSON methods of QuerySnapshot and DocumentSnapshot have also been updated to explicitly name the fields in the object return type.

Fixed an issue in the bundle builder where the bundleName for DocumentSnapshots didn't match the ResourcePath of the document, which is needed when reconstituting the DocumentReference in onSnapshot.

Finally, I cleaned up the text wrapping for the onSnapshot reference doc comments so they take up fewer lines of source code.

Testing

Added integration tests to ensure that onSnapshot functions invoked with a bundle properly call next with the snapshot data initially and when the document is updated in the service, and error if the bundle is invalid. Made tests that exercise the method signatures that take an Observer as well.

API Changes

Numerous onSnapshot overloads that now accept a bundle a parameter. These will be part of a formal API proposal once the full feature branch implementation is closer to completion.

github-actions · 2025-04-04T16:55:56Z

Vertex AI Mock Responses Check ⚠️

A newer major version of the mock responses for Vertex AI unit tests is available. update_vertexai_responses.sh should be updated to clone the latest version of the responses: v8.0

google-oss-bot · 2025-04-04T19:12:28Z

Size Report ¹

Affected Products

@firebase/firestore
Type Base (b85747b) Merge (9ad6c4a) Diff
browser 387 kB 389 kB +1.95 kB (+0.5%)
main 599 kB 602 kB +3.35 kB (+0.6%)
module 387 kB 389 kB +1.95 kB (+0.5%)
react-native 388 kB 390 kB +1.95 kB (+0.5%)

`bundle`

Type	Base (`b85747b`)	Merge (`9ad6c4a`)	Diff
firestore (Query Cursors)	255 kB	255 kB	+100 B (+0.0%)
firestore (Query)	253 kB	262 kB	+9.37 kB (+3.7%)
firestore (Read data once)	244 kB	244 kB	+44 B (+0.0%)
firestore (Read Write w Persistence)	332 kB	332 kB	+44 B (+0.0%)
firestore (Realtime updates)	247 kB	256 kB	+9.37 kB (+3.8%)
firestore (Transaction)	221 kB	221 kB	+44 B (+0.0%)

firebase
Type Base (b85747b) Merge (9ad6c4a) Diff
firebase-compat.js 799 kB 801 kB +1.60 kB (+0.2%)
firebase-firestore-compat.js 345 kB 346 kB +1.60 kB (+0.5%)
firebase-firestore.js 446 kB 448 kB +1.93 kB (+0.4%)

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/3xTflPfA2w.html

google-oss-bot · 2025-04-04T19:24:04Z

Size Analysis Report ¹

This report is too large (92,564 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

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

changeset-bot · 2025-04-07T14:47:32Z

⚠️ No Changeset found

Latest commit: fc8d568

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

MarkDuckworth

Looks good. I think the only thing that needs to be fixed is checking for unsubscribed before creating a snapshot listener for a DocumentReference

packages/firestore/src/api/reference_impl.ts

MarkDuckworth · 2025-04-11T19:57:54Z

packages/firestore/src/api/reference_impl.ts

+          ) => void,
+          error: args[curArg++] as (error: FirestoreError) => void,
+          complete: args[curArg++] as () => void
+        },


nit: it feels like you could reduce some duplicate code by creating these observer objects higher in the function. It may only reduce two calls to onSnapshot[Query|Document]SnapshotBundle

Yeah, that's what I had originally but it ended up being more lines of code due to the type definiton and then configuring the object with optional parameters. I can revist it though, if you'd like.

Edit: revisited, and done!

packages/firestore/src/api/reference_impl.ts

json => snapshotJSON. Common observer definition default SnapshotOption object unsubscribe check on onSnapshotDocumentSnapshotBundle

MarkDuckworth

LGTM

MarkDuckworth · 2025-04-14T20:18:59Z

packages/firestore/src/api/reference_impl.ts

+ *
+ * @internal
+ */
+function normalizeSnapshotJsonFields(snapshotJson: object): {


I might have used a type predicate for this. However, since you are also checking values, this implementation is also okay, IMO.

DellaBitta added 2 commits April 4, 2025 10:24

WIP, QuerySnapshot works in protoype function

990c066

Bundle snapshot loader part of overload

54c89fc

Compilation complete

593e3cd

Overloaded onSnapshot signature impl.

8a181e1

DellaBitta added 16 commits April 7, 2025 14:56

cleanup observer marshalling

7588ffd

DocumentSnapshot. bundleSource & name fix

10bf743

onNext overloads and toJSON type

f88748f

itest for Snapshot created by a DocSnap bundle

19990fc

Three DocumentSnap iTests

099e53e

QuerySnapshot integration tests

e33c4f9

Remove extra test, added by mistake.

a5ffd29

Update database.test.ts

740b723

Wait for writes in QuerySnapshot multiple events

c039ae2

debug logging

2f27429

Timing protection QuerySnapshot tests.

8fc6c33

format

4bc9bdd

docs

8238cce

Use EventsAccumulator

697b6d4

onSnapshot Observer and Error testing

df3cb20

Internal function formal comments

a35575a

DellaBitta marked this pull request as ready for review April 11, 2025 17:44

DellaBitta requested review from a team as code owners April 11, 2025 17:44

DellaBitta changed the title ~~[Feat] WIP - Firestore onSnapshot support for bundles~~ [Feat] Firestore onSnapshot support for bundles for feature branch. Apr 11, 2025

DellaBitta requested a review from MarkDuckworth April 11, 2025 17:44

MarkDuckworth reviewed Apr 11, 2025

View reviewed changes

DellaBitta added 9 commits April 12, 2025 16:18

PR fixes

afc7f44

json => snapshotJSON. Common observer definition default SnapshotOption object unsubscribe check on onSnapshotDocumentSnapshotBundle

Fixes for review.

68d49a6

removed typo.

078d0c4

Debug output for CI.

c4dab65

more debug info

3ef49bb

...

72e970c

...

378d1fc

Alt impl for setting keys.

9b2f0fe

Remove debug output

fc8d568

MarkDuckworth approved these changes Apr 14, 2025

View reviewed changes

DellaBitta merged commit e34353d into ddb-firestore-result-serialization Apr 15, 2025
72 of 105 checks passed

DellaBitta deleted the ddb-onsnapshot-docbundle branch April 15, 2025 00:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feat] Firestore onSnapshot support for bundles for feature branch. #8896

[Feat] Firestore onSnapshot support for bundles for feature branch. #8896

DellaBitta commented Apr 4, 2025 •

edited

Loading

github-actions bot commented Apr 4, 2025

google-oss-bot commented Apr 4, 2025 •

edited

Loading

`@firebase/firestore`

`bundle`

`firebase`

google-oss-bot commented Apr 4, 2025 •

edited

Loading

changeset-bot bot commented Apr 7, 2025 •

edited

Loading

MarkDuckworth left a comment

MarkDuckworth Apr 11, 2025

DellaBitta Apr 11, 2025

MarkDuckworth left a comment

MarkDuckworth Apr 14, 2025

[Feat] Firestore onSnapshot support for bundles for feature branch. #8896

[Feat] Firestore onSnapshot support for bundles for feature branch. #8896

Conversation

DellaBitta commented Apr 4, 2025 • edited Loading

Discussion

Testing

API Changes

github-actions bot commented Apr 4, 2025

Vertex AI Mock Responses Check ⚠️

google-oss-bot commented Apr 4, 2025 • edited Loading

Size Report 1

Affected Products

@firebase/firestore

bundle

firebase

Test Logs

google-oss-bot commented Apr 4, 2025 • edited Loading

Size Analysis Report 1

Test Logs

changeset-bot bot commented Apr 7, 2025 • edited Loading

⚠️ No Changeset found

MarkDuckworth left a comment

Choose a reason for hiding this comment

MarkDuckworth Apr 11, 2025

Choose a reason for hiding this comment

DellaBitta Apr 11, 2025

Choose a reason for hiding this comment

MarkDuckworth left a comment

Choose a reason for hiding this comment

MarkDuckworth Apr 14, 2025

Choose a reason for hiding this comment

DellaBitta commented Apr 4, 2025 •

edited

Loading

google-oss-bot commented Apr 4, 2025 •

edited

Loading

Size Report ¹

`@firebase/firestore`

`bundle`

`firebase`

google-oss-bot commented Apr 4, 2025 •

edited

Loading

Size Analysis Report ¹

changeset-bot bot commented Apr 7, 2025 •

edited

Loading