Skip to content

fix(firestore): reject undeclared update keys, support typed dotted field paths - #83

Merged
fwal merged 4 commits into
mainfrom
fwal/fix-nested-updates
Sep 10, 2026
Merged

fix(firestore): reject undeclared update keys, support typed dotted field paths#83
fwal merged 4 commits into
mainfrom
fwal/fix-nested-updates

Conversation

@fwal

@fwal fwal commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Fixes #63

Repository.update encoded through the model's update struct, which strips excess keys. A Firestore field path like 'metaData.deleted' was silently dropped and the SDK rejected the empty payload with an unrelated error.

  • All repository request encoders use onExcessProperty: 'error': undeclared keys in add/set/update fail with SchemaError naming the key.
  • Empty update payload fails with FirestoreError code invalid-argument.
  • update accepts dotted field paths, typed via UpdateData<T>. Each path resolves to its leaf schema in Model.update (through Struct, Model.Struct, Class, Record, optional, OptionalDeletable) and encodes on its own, so nested sentinels and transforms work.
yield* repo.update(id, { 'metaData.deleted': true });
yield* repo.update(id, { 'stats.likes': Firestore.increment(1) });

Behavior change: junk keys previously dropped now error.

🤖 Generated with Claude Code

…d field paths

Repository.update encoded through the model's update struct, which strips
excess keys, so a Firestore field path like 'metaData.deleted' vanished
and the SDK rejected the empty payload with an unrelated error (#63).

- Encode all repository request payloads with onExcessProperty: 'error':
  an undeclared key fails with a SchemaError naming it.
- Fail an empty update payload with FirestoreError invalid-argument.
- Accept dotted field paths in update, typed via UpdateData<T>. Each path
  resolves to its leaf schema in Model.update (through Struct, Model.Struct,
  Class, Record, optional, OptionalDeletable) and encodes on its own, so
  nested sentinels and transforms work.

Fixes #63

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions github-actions Bot added 📖 docs Improvements or additions to documentation 🐛 fix Something is broken or doesn't work properly 📦 core labels Sep 9, 2026
@fwal fwal added this to the 1.0 milestone Sep 9, 2026
@fwal fwal self-assigned this Sep 9, 2026
@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown

Greptile Summary

The PR makes repository request encoding strict and adds typed dotted-field updates with leaf-schema encoding and optional merge flattening.

  • Undeclared request keys now fail with a descriptive SchemaError.
  • Nested updates support typed dotted paths, transforms, sentinels, optional fields, records, and bounded recursive schemas.
  • Empty updates fail early with an invalid-argument FirestoreError.
  • Migration and repository documentation explain the behavior change and nested-update semantics.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; both previously reported issues are fixed by the current migration guidance and constrained Record-key validation.

Important Files Changed

Filename Overview
packages/effect-firebase/src/lib/firestore/model/update-path.ts Adds bounded typed field-path resolution, constrained Record-key validation, and merge-payload flattening; the previously reported Record validation issue is fixed.
packages/effect-firebase/src/lib/firestore/model/repository.ts Routes dotted update keys through leaf encoders, strictly validates ordinary fields, rejects empty payloads, and supports merge flattening.
packages/effect-firebase/src/lib/firestore/model/fetch.ts Applies strict excess-property checking consistently to repository request encoders.
packages/effect-firebase/MIGRATION.md Documents the strict-encoding breaking change, dotted paths, whole-map replacement, and merge-mode migration guidance.
packages/effect-firebase/src/lib/firestore/model/update-path.spec.ts Covers nested schema traversal, constrained Record keys, recursive depth limits, merge flattening, and type-level path behavior.
packages/effect-firebase/src/lib/firestore/model/repository.spec.ts Exercises strict request validation, dotted leaf encoding, sentinels, Record paths, merge semantics, and empty-update handling.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Repository.update] --> B{merge enabled?}
  B -->|Yes| C[Flatten nested plain objects]
  B -->|No| D[Keep supplied keys]
  C --> E{Dotted field path?}
  D --> E
  E -->|Resolvable| F[Encode with leaf schema]
  E -->|Not resolvable| G[Strict struct encoding]
  F --> H[Build Firestore payload]
  G --> H
  H --> I{Payload empty?}
  I -->|Yes| J[Fail invalid-argument]
  I -->|No| K[Firestore update]
Loading

Reviews (4): Last reviewed commit: "feat(firestore): add { merge: true } to ..." | Re-trigger Greptile

Comment thread packages/effect-firebase/src/lib/firestore/model/fetch.ts
Comment thread packages/effect-firebase/src/lib/firestore/model/update-path.ts Outdated
fwal and others added 2 commits September 9, 2026 21:55
…encoding in MIGRATION

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…r recursive schemas

Walk SchemaAST (Objects, Union, Suspend, Declaration, encoding links) instead
of the Schema wrapper API so Schema.suspend maps resolve. Cap paths at
MAX_FIELD_PATH_DEPTH (5) at both type and runtime level: without a cap,
UpdateData on a recursive type alias is a circular mapped type (TS2615).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
greptile-apps[bot]
greptile-apps Bot previously approved these changes Sep 10, 2026
Flattens a deep-partial payload into dotted field paths before the write,
so nested objects merge into the stored map instead of replacing it. Leaves
(arrays, class instances, sentinels, Option.none) are written whole;
Option.some is merged into; empty objects contribute nothing.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@greptile-apps
greptile-apps Bot dismissed their stale review September 10, 2026 06:50

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

@fwal
fwal merged commit 6c3bbbd into main Sep 10, 2026
6 checks passed
@fwal
fwal deleted the fwal/fix-nested-updates branch September 10, 2026 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📦 core 📖 docs Improvements or additions to documentation 🐛 fix Something is broken or doesn't work properly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Typed nested-path (dotted) updates through the repository

1 participant