Skip to content

feat(firestore)!: add collection group queries - #88

Merged
fwal merged 5 commits into
mainfrom
fwal/serene-tesla-4o470d
Sep 18, 2026
Merged

fwal merged 5 commits into
mainfrom
fwal/serene-tesla-4o470d

Conversation

@fwal

@fwal fwal commented Sep 18, 2026

Copy link
Copy Markdown
Owner

Adds collection group queries across the stack.

  • FirestoreService.queryGroup / streamQueryGroup (admin, client, mock, noopLayer, MockFirestoreService).
  • Every repository gets a group view with the same four query methods, run over the collection group named by the last segment of collectionPath.
  • New makeRepository option pathField (string fields only) fills a model field with each document's full path on read.
  • validateCollectionId / collectionIdOf exported from core and shared by all backends; malformed IDs fail with a typed FirestoreError on both effect and stream.
  • Mock: state for a group resolves by collection ID.
  • Docs: core AGENTS/README/MIGRATION, mock README, root README.

Breaking: FirestoreServiceShape has two new required members. Hand-written layers must implement them; see MIGRATION.md step 8.

Verified: nx run-many -t build test lint green for all 8 projects. effect-firebase:typecheck fails identically on main (pre-existing tsconfig.spec.json issue).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Xo119tdUdoz5hd3eZ6QuzP


Generated by Claude Code

Add `queryGroup` and `streamQueryGroup` to `FirestoreServiceShape`, so a
query can span every collection with a given ID at any depth
(`posts/{p}/comments` and `users/{u}/comments` both belong to the
`comments` group).

- core: every repository gains a `group` view exposing the same four query
  methods (`query`, `queryStream`, `getByQuery`, `getByQueryStream`) over
  the collection group named by the last segment of `collectionPath`.
  New optional `pathField` (constrained to string fields) fills a model
  field with each document's full path on every read. The query methods
  are built once by a shared helper for both views.
- core: `validateCollectionId` / `collectionIdOf` exported and shared by
  the repository factory and every backend.
- admin/client: build the base query from `collectionGroup(id)` with the
  constraint handling shared with single-collection queries. The ID is
  validated up front so both the effect and the stream fail with a typed
  `FirestoreError` (`invalid-argument`) instead of a defect. The client
  dies inside a transaction, matching `query`.
- mock: `docsInCollectionGroup` matches documents whose parent collection
  segment equals the ID; simulated state resolves by collection ID.
  `MockFirestoreService` gains the two throwing defaults.
- noopLayer, docs (core AGENTS/README/MIGRATION, mock README, root
  README) and tests for all four packages.

BREAKING CHANGE: `FirestoreServiceShape` has two new required members,
`queryGroup` and `streamQueryGroup`. Hand-written `FirestoreService`
layers must implement them; `noopLayer` and `MockFirestoreService`
already do. See MIGRATION.md step 8.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xo119tdUdoz5hd3eZ6QuzP
@github-actions github-actions Bot added 📖 docs Improvements or additions to documentation 📦 admin 📦 client 📦 core 📦 mock labels Sep 18, 2026
@fwal fwal self-assigned this Sep 18, 2026
@fwal
fwal marked this pull request as ready for review September 18, 2026 11:48
@fwal fwal added this to the 1.0 milestone Sep 18, 2026
@greptile-apps

greptile-apps Bot commented Sep 18, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the latest cursor-arity fix resolves the remaining previous finding without introducing a new actionable defect.

Summary

The PR adds collection-group querying throughout the Firestore abstraction and repository API.

  • Adds effect and streaming collection-group queries to the Admin, Client, Mock, noop, and custom-service surfaces.
  • Adds repository group query methods and optional full-document-path decoding through pathField.
  • Centralizes collection-ID validation and expands collection-group cursor behavior in the mock.
  • Documents the new API and its breaking custom-layer requirements.
  • The latest revision correctly fixes explicit document-name cursor arity in the mock.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  R[Repository group view] --> S[FirestoreService queryGroup / streamQueryGroup]
  S --> A[Admin collectionGroup]
  S --> C[Client collectionGroup]
  S --> M[Mock collection-group selector]
  M --> V[Collection ID and cursor validation]
  A --> D[Snapshots with full paths]
  C --> D
  M --> D
  D --> P[Model decoding with optional pathField]
Loading

Reviews (5) · Last reviewed commit: "fix(mock): count an explicit __name__ or..."

Comment thread packages/mock/src/lib/firestore/layer.ts
`__name__` ordering and the implicit document-name tiebreaker compared
`ref.id`, so two group documents with the same ID under different parents
compared equal and paged differently from the real backends. Compare the
full path instead, and accept a `__name__` cursor as either a bare ID
(single-collection query, expanded against the document's parent) or a
full path (collection group query), matching Firestore.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xo119tdUdoz5hd3eZ6QuzP
Comment thread packages/mock/src/lib/firestore/query-filter.ts
Both SDKs require a full document path for a `__name__` cursor in a
collection group query; the mock accepted a bare ID and expanded it per
snapshot, so a pagination test could pass against the mock and fail on
Firestore. `queryGroup` and `streamQueryGroup` now fail with
`invalid-argument` for a cursor that is not a document path (even, non-empty
segments), mirroring the SDK error.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xo119tdUdoz5hd3eZ6QuzP
Comment thread packages/mock/src/lib/firestore/query-filter.ts
A collection group cursor carrying more values than the query orders by
(explicit orderBys plus the implicit document-name tiebreaker) indexed
past the end of the orderBy list while validating, throwing a TypeError
instead of the typed invalid-argument failure the SDKs return.

Validate the cursor length up front, and read the orderBy entry
defensively so an out-of-range index can never dereference undefined.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xo119tdUdoz5hd3eZ6QuzP
Comment thread packages/mock/src/lib/firestore/query-filter.ts Outdated
Firestore only appends the implicit document-name ordering when the query
does not already order by __name__, so a query ordering by document ID
accepts exactly one cursor value per orderBy. The cursor length check
reserved an extra position regardless, letting a group query pass in the
mock that both SDKs reject.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xo119tdUdoz5hd3eZ6QuzP
@fwal
fwal merged commit 9144ae7 into main Sep 18, 2026
6 checks passed
@fwal
fwal deleted the fwal/serene-tesla-4o470d branch September 18, 2026 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📦 admin 📦 client 📦 core 📖 docs Improvements or additions to documentation 📦 mock

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant