From 07e5dd2a20d01574a10fcef198ef367379a15744 Mon Sep 17 00:00:00 2001 From: Frederik Wallner Date: Mon, 14 Sep 2026 10:34:15 +0000 Subject: [PATCH 1/4] fix(firestore)!: decode missing keys for Optional and server-stamped fields `Firestore.Optional` database variants now use `Schema.OptionFromOptionalNullOr`, so a document without the field decodes to `Option.none()` again instead of failing with `SchemaError: Missing key` under Effect v4. `Option.none()` is still written as `null`. BREAKING CHANGE: the encoded type of `Optional` fields moves from `NullishOr` to `optional(NullOr)`, making the key optional in `Model.Encoded`; annotations using `Schema.OptionFromNullishOr<...>` must switch to `Schema.OptionFromOptionalNullOr<...>`. Also: - `OptionalDeletable.update` decodes a missing key and encodes `Option.none()` as an omitted key rather than `undefined`. - `DateTimeInsert`/`DateTimeUpdate`/`ServerDateTime` insert/update variants accept an omitted key for the server timestamp, so `createdAt: undefined` is no longer required. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UTvaTdYRhJ8MdaKxQ19miS --- example/app/src/routes/firestore.tsx | 2 - packages/effect-firebase/AGENTS.md | 28 +-- packages/effect-firebase/MIGRATION.md | 30 ++- .../src/lib/firestore/model/datetime.spec.ts | 93 ++++++++++ .../src/lib/firestore/model/datetime.ts | 26 ++- .../src/lib/firestore/model/optional.spec.ts | 171 +++++++++++++++++- .../src/lib/firestore/model/optional.ts | 46 +++-- .../lib/firestore/model/repository.spec.ts | 17 +- 8 files changed, 350 insertions(+), 63 deletions(-) diff --git a/example/app/src/routes/firestore.tsx b/example/app/src/routes/firestore.tsx index 2ac33a8..4d051ae 100644 --- a/example/app/src/routes/firestore.tsx +++ b/example/app/src/routes/firestore.tsx @@ -87,8 +87,6 @@ function PostForm({ title: value.title, content: value.content, author: AuthorId.make('1'), - createdAt: undefined, - updatedAt: undefined, checked: false, optional: Option.none(), list: [], diff --git a/packages/effect-firebase/AGENTS.md b/packages/effect-firebase/AGENTS.md index adf2719..e6c6da5 100644 --- a/packages/effect-firebase/AGENTS.md +++ b/packages/effect-firebase/AGENTS.md @@ -91,20 +91,20 @@ Variants: `PostModel` (alias `.select`, what reads decode to), `.insert`, Field helpers (all under `Firestore.` unless noted): -| Helper | Notes | -| -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | -| `Model.GeneratedByDb(s)` / `Model.GeneratedByApp(s)` | From `effect/unstable/schema`. DB-generated ids vs app-generated ids. | -| `DateTimeInsert`, `DateTimeUpdate` | Auto server timestamps. App type is `DateTime.Utc`. | -| `DateTime`, `ServerDateTime` | Plain timestamp; `ServerDateTime` writes server time when given `undefined`. | -| `WithServerTimestamp(field)` | Lets insert/update accept `Firestore.serverTimestamp()` explicitly. | -| `Reference(id, path)`, `ReferenceOptional(id, path)` | Typed reference exposed as branded id. | -| `ReferenceAsInstance(id, path)`, `ReferencePath(path)` | Expose `FirestoreSchema.Reference` instance / full path string. | -| `AnyIdReference`, `AnyPathReference` | Untyped references. | -| `Optional(s)`, `OptionalNull(s)`, `OptionalDeletable(s)` | `Option` in app. `Optional` accepts null/undefined, `OptionalNull` only null, `OptionalDeletable` supports `Firestore.delete()` in update. | -| `Array(s)`, `WithArrayFields(field)` | `Firestore.arrayUnion([...])` / `arrayRemove([...])` in update. | -| `Number`, `WithIncrementField(field)` | `Firestore.increment(n)` in update. | -| `GeoPoint` | `FirestoreSchema.GeoPoint` instance in app, `{ latitude, longitude }` in JSON. | -| `Model.Field({ select, insert, update, json, ... })` | Fully custom per-variant schemas (from `effect/unstable/schema`). | +| Helper | Notes | +| -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `Model.GeneratedByDb(s)` / `Model.GeneratedByApp(s)` | From `effect/unstable/schema`. DB-generated ids vs app-generated ids. | +| `DateTimeInsert`, `DateTimeUpdate` | Auto server timestamps. App type is `DateTime.Utc`. | +| `DateTime`, `ServerDateTime` | Plain timestamp; `ServerDateTime` writes server time when the key is omitted or `undefined`. | +| `WithServerTimestamp(field)` | Lets insert/update accept `Firestore.serverTimestamp()` explicitly. | +| `Reference(id, path)`, `ReferenceOptional(id, path)` | Typed reference exposed as branded id. | +| `ReferenceAsInstance(id, path)`, `ReferencePath(path)` | Expose `FirestoreSchema.Reference` instance / full path string. | +| `AnyIdReference`, `AnyPathReference` | Untyped references. | +| `Optional(s)`, `OptionalNull(s)`, `OptionalDeletable(s)` | `Option` in app. `Optional` reads a missing key/null/undefined and writes `null`; `OptionalNull` only null; `OptionalDeletable` omits the key and supports `Firestore.delete()` in update. | +| `Array(s)`, `WithArrayFields(field)` | `Firestore.arrayUnion([...])` / `arrayRemove([...])` in update. | +| `Number`, `WithIncrementField(field)` | `Firestore.increment(n)` in update. | +| `GeoPoint` | `FirestoreSchema.GeoPoint` instance in app, `{ latitude, longitude }` in JSON. | +| `Model.Field({ select, insert, update, json, ... })` | Fully custom per-variant schemas (from `effect/unstable/schema`). | ## Create a repository diff --git a/packages/effect-firebase/MIGRATION.md b/packages/effect-firebase/MIGRATION.md index ca2514f..71906cb 100644 --- a/packages/effect-firebase/MIGRATION.md +++ b/packages/effect-firebase/MIGRATION.md @@ -191,10 +191,32 @@ const Custom = Model.Field({ select: A, insert: B, update: C, json: D }); `Model.ServerDateTime` was wrapped in `VariantSchema.Overrideable` and required `Model.Override(value)` to write an explicit timestamp. In v1.0 `Firestore.ServerDateTime` is a plain field: pass a `DateTime.Utc` to write -that instant, or `undefined` to write the server timestamp. For fields where -you want to opt into the server timestamp explicitly, wrap any timestamp -field in `Firestore.WithServerTimestamp(...)` and pass -`Firestore.serverTimestamp()`. +that instant, or omit the key (or pass `undefined`) to write the server +timestamp. The same applies to the `insert`/`update` variants of +`DateTimeInsert` and `DateTimeUpdate`, so `repo.add({ title })` no longer +needs `createdAt: undefined`. For fields where you want to opt into the +server timestamp explicitly, wrap any timestamp field in +`Firestore.WithServerTimestamp(...)` and pass `Firestore.serverTimestamp()`. + +**`Optional` encoded type.** The database variants (`select`/`insert`/ +`update`) of `Firestore.Optional(s)` decode a missing key, `null` and +`undefined` to `Option.none()`, as they did in v0.x (Effect v4 no longer +tolerates a missing key on a required property, so v1.0 betas before beta.7 +failed reads of documents without the field with `SchemaError: Missing key`). +`Option.none()` is still encoded as `null`, so write payloads are unchanged. +The encoded field type moved from `Schema.NullishOr` to +`Schema.optional(Schema.NullOr)`, which makes the key optional in +`Model.Encoded` / `Model.insert.Encoded` / `Model.update.Encoded`. Code that +spelled these fields out as `Schema.OptionFromNullishOr<...>` must use +`Schema.OptionFromOptionalNullOr<...>` instead. `OptionalNull` and +`ReferenceOptional` are unchanged and still require the key to be present. + +**`OptionalDeletable` update variant.** `Option.none()` in an `update` +payload is now encoded as a missing key instead of `undefined` (which the +Firebase SDKs reject as a value), and a document without the key decodes +through `Model.update`. A merge update whose only field is `Option.none()` +therefore fails with `FirestoreError` code `invalid-argument` (empty +payload); use `Option.some(Firestore.delete())` to remove the field. ### 5. Update `FirestoreField` import (converter usage) diff --git a/packages/effect-firebase/src/lib/firestore/model/datetime.spec.ts b/packages/effect-firebase/src/lib/firestore/model/datetime.spec.ts index fb16083..ec2df9c 100644 --- a/packages/effect-firebase/src/lib/firestore/model/datetime.spec.ts +++ b/packages/effect-firebase/src/lib/firestore/model/datetime.spec.ts @@ -5,6 +5,7 @@ import { DateTime, DateTimeInsert, DateTimeUpdate, + ServerDateTime, WithServerTimestamp, } from './datetime.js'; import { Timestamp } from '../schema/timestamp.js'; @@ -130,6 +131,13 @@ describe('Model.DateTimeInsert', () => { }); describe('insert variant', () => { + it('should encode a missing key to ServerTimestamp', () => { + const encode = Schema.encodeSync(TestModel.insert); + const result = encode({}); + + expect(result.createdAt).toBeInstanceOf(FirestoreSchema.ServerTimestamp); + }); + it('should encode undefined to ServerTimestamp', () => { const encode = Schema.encodeSync(TestModel.insert); const result = encode({ createdAt: undefined }); @@ -137,6 +145,19 @@ describe('Model.DateTimeInsert', () => { expect(result.createdAt).toBeInstanceOf(FirestoreSchema.ServerTimestamp); }); + it('should decode Timestamp to DateTime.Utc', () => { + const decode = Schema.decodeUnknownSync(TestModel.insert); + const result = decode({ createdAt: Timestamp.fromMillis(1705315800000) }); + + expect(EffectDateTime.isDateTime(result.createdAt)).toBe(true); + }); + + it('should reject decoding a ServerTimestamp', () => { + const decode = Schema.decodeUnknownSync(TestModel.insert); + + expect(() => decode({ createdAt: serverTimestamp() })).toThrow(); + }); + it('should encode a DateTime.Utc value to Timestamp', () => { const encode = Schema.encodeSync(TestModel.insert); const millis = 1705315800000; @@ -293,5 +314,77 @@ describe('Model.DateTimeUpdate', () => { expect(result.updatedAt).toBeInstanceOf(FirestoreSchema.ServerTimestamp); }); + + it('should encode a missing key to ServerTimestamp', () => { + const encode = Schema.encodeSync(TestModel.update); + const result = encode({}); + + expect(result.updatedAt).toBeInstanceOf(FirestoreSchema.ServerTimestamp); + }); + + it('should encode a DateTime.Utc value to Timestamp', () => { + const encode = Schema.encodeSync(TestModel.update); + const result = encode({ + updatedAt: EffectDateTime.makeUnsafe(1705315800000), + }); + + expect(result.updatedAt).toEqual({ seconds: 1705315800, nanoseconds: 0 }); + }); + }); + + describe('insert variant', () => { + it('should encode a missing key to ServerTimestamp', () => { + const encode = Schema.encodeSync(TestModel.insert); + const result = encode({}); + + expect(result.updatedAt).toBeInstanceOf(FirestoreSchema.ServerTimestamp); + }); + }); +}); + +describe('Model.ServerDateTime', () => { + class TestModel extends Model.Class('TestModel')({ + id: Model.GeneratedByDb(Schema.String), + seenAt: ServerDateTime, + }) {} + + describe('get variant', () => { + it('should decode Timestamp to DateTime.Utc', () => { + const decode = Schema.decodeUnknownSync(TestModel); + const result = decode({ + id: 'post-1', + seenAt: Timestamp.fromMillis(1705315800000), + }); + + expect(EffectDateTime.isDateTime(result.seenAt)).toBe(true); + }); + }); + + describe.each([ + ['insert', () => TestModel.insert], + ['update', () => TestModel.update], + ] as const)('%s variant', (_, variant) => { + it('should encode a missing key to ServerTimestamp', () => { + const encode = Schema.encodeUnknownSync(variant()); + const result = encode({}); + + expect(result.seenAt).toBeInstanceOf(FirestoreSchema.ServerTimestamp); + }); + + it('should encode undefined to ServerTimestamp', () => { + const encode = Schema.encodeUnknownSync(variant()); + const result = encode({ seenAt: undefined }); + + expect(result.seenAt).toBeInstanceOf(FirestoreSchema.ServerTimestamp); + }); + + it('should encode a DateTime.Utc value to Timestamp', () => { + const encode = Schema.encodeUnknownSync(variant()); + const result = encode({ + seenAt: EffectDateTime.makeUnsafe(1705315800000), + }); + + expect(result.seenAt).toEqual({ seconds: 1705315800, nanoseconds: 0 }); + }); }); }); diff --git a/packages/effect-firebase/src/lib/firestore/model/datetime.ts b/packages/effect-firebase/src/lib/firestore/model/datetime.ts index dfe574b..0915bad 100644 --- a/packages/effect-firebase/src/lib/firestore/model/datetime.ts +++ b/packages/effect-firebase/src/lib/firestore/model/datetime.ts @@ -1,6 +1,7 @@ import { DateTime as EffectDateTime, Effect, + Option, Schema, SchemaGetter, SchemaIssue, @@ -23,15 +24,18 @@ export const DateTime: DateTime = Model.Field({ }); /** - * Schema for add/update variants that: + * Schema for the insert/update variants that: * - Decodes: Timestamp → DateTime.Utc (ServerTimestamp decode fails) - * - Encodes: DateTime.Utc → Timestamp, undefined → ServerTimestamp + * - Encodes: DateTime.Utc → Timestamp, missing key or undefined → ServerTimestamp + * + * The key is optional on the decoded side, so callers can omit the field to + * request the server timestamp. */ const ServerDateTimeSchema = Schema.Union([ FirestoreSchema.TimestampInstance, FirestoreSchema.ServerTimestampInstance, ]).pipe( - Schema.decodeTo(Schema.UndefinedOr(Schema.DateTimeUtc), { + Schema.decodeTo(Schema.optional(Schema.DateTimeUtc), { decode: SchemaGetter.transformOrFail( (input: FirestoreSchema.Timestamp | FirestoreSchema.ServerTimestamp) => { if (input instanceof FirestoreSchema.Timestamp) { @@ -47,13 +51,17 @@ const ServerDateTimeSchema = Schema.Union([ ); }, ), - encode: SchemaGetter.transform( + encode: SchemaGetter.transformOptional( ( - dt: EffectDateTime.Utc | undefined, - ): FirestoreSchema.Timestamp | FirestoreSchema.ServerTimestamp => - dt !== undefined - ? FirestoreSchema.Timestamp.fromDateTime(dt) - : new FirestoreSchema.ServerTimestamp(), + dt: Option.Option, + ): Option.Option< + FirestoreSchema.Timestamp | FirestoreSchema.ServerTimestamp + > => + Option.some( + Option.isSome(dt) && dt.value !== undefined + ? FirestoreSchema.Timestamp.fromDateTime(dt.value) + : new FirestoreSchema.ServerTimestamp(), + ), ), }), ); diff --git a/packages/effect-firebase/src/lib/firestore/model/optional.spec.ts b/packages/effect-firebase/src/lib/firestore/model/optional.spec.ts index 199abcd..a9c67c0 100644 --- a/packages/effect-firebase/src/lib/firestore/model/optional.spec.ts +++ b/packages/effect-firebase/src/lib/firestore/model/optional.spec.ts @@ -1,8 +1,10 @@ -import { Option, Schema } from 'effect'; +import { DateTime as EffectDateTime, Option, Schema } from 'effect'; import { describe, expect, it } from 'vitest'; import { Model } from 'effect/unstable/schema'; import { Optional, OptionalNull, OptionalDeletable } from './optional.js'; +import { DateTime } from './datetime.js'; +import { Reference } from './reference.js'; import { Delete, delete as deleteField } from '../fields/delete.js'; describe('Optional', () => { @@ -11,30 +13,61 @@ describe('Optional', () => { bio: Optional(Schema.String), }) {} - describe('get variant', () => { + describe.each([ + ['select', () => TestModel], + ['insert', () => TestModel.insert], + ['update', () => TestModel.update], + ] as const)('%s variant', (_, variant) => { it('should decode value to Option.some', () => { - const decode = Schema.decodeUnknownSync(TestModel); + const decode = Schema.decodeUnknownSync(variant()); const result = decode({ name: 'John', bio: 'Developer' }); expect(Option.isSome(result.bio)).toBe(true); expect(Option.getOrNull(result.bio)).toBe('Developer'); }); + it('should decode missing key to Option.none', () => { + const decode = Schema.decodeUnknownSync(variant()); + const result = decode({ name: 'John' }); + + expect(Option.isNone(result.bio)).toBe(true); + }); + it('should decode null to Option.none', () => { - const decode = Schema.decodeUnknownSync(TestModel); + const decode = Schema.decodeUnknownSync(variant()); const result = decode({ name: 'John', bio: null }); expect(Option.isNone(result.bio)).toBe(true); }); it('should decode undefined to Option.none', () => { - const decode = Schema.decodeUnknownSync(TestModel); + const decode = Schema.decodeUnknownSync(variant()); const result = decode({ name: 'John', bio: undefined }); expect(Option.isNone(result.bio)).toBe(true); }); }); + describe.each([ + ['insert', () => TestModel.insert], + ['update', () => TestModel.update], + ] as const)('encoding %s variant', (_, variant) => { + it('should encode Option.none as null', () => { + const encode = Schema.encodeUnknownSync(variant()); + const result = encode({ name: 'John', bio: Option.none() }); + + expect(result).toHaveProperty('bio'); + expect(result.bio).toBeNull(); + }); + + it('should encode Option.some with the value', () => { + const encode = Schema.encodeUnknownSync(variant()); + const result = encode({ name: 'John', bio: Option.some('Developer') }); + + expect(result.bio).toBe('Developer'); + }); + }); + describe('encoding get variant', () => { it('should encode Option.none as null', () => { const encode = Schema.encodeSync(TestModel); @@ -42,6 +75,7 @@ describe('Optional', () => { new TestModel({ name: 'John', bio: Option.none() }), ); + expect(result).toHaveProperty('bio'); expect(result.bio).toBeNull(); }); @@ -55,6 +89,29 @@ describe('Optional', () => { }); }); + describe('encoded types', () => { + it('should allow omitting the key in the database Encoded types', () => { + const select: typeof TestModel.Encoded = { name: 'John' }; + const insert: typeof TestModel.insert.Encoded = { name: 'John' }; + const update: typeof TestModel.update.Encoded = { name: 'John' }; + + expect(select).toEqual({ name: 'John' }); + expect(insert).toEqual({ name: 'John' }); + expect(update).toEqual({ name: 'John' }); + }); + + it('should still accept null and the value in the Encoded types', () => { + const withNull: typeof TestModel.Encoded = { name: 'John', bio: null }; + const withValue: typeof TestModel.Encoded = { + name: 'John', + bio: 'Developer', + }; + + expect(withNull.bio).toBeNull(); + expect(withValue.bio).toBe('Developer'); + }); + }); + describe('json variant', () => { it('should decode present value to Option.some', () => { const decode = Schema.decodeUnknownSync(TestModel.json); @@ -69,6 +126,79 @@ describe('Optional', () => { expect(Option.isNone(result.bio)).toBe(true); }); + + it('should reject null', () => { + const decode = Schema.decodeUnknownSync(TestModel.json); + + expect(() => decode({ name: 'John', bio: null })).toThrow(); + }); + + it('should encode Option.none as a missing key', () => { + const encode = Schema.encodeSync(TestModel.json); + const result = encode({ name: 'John', bio: Option.none() }); + + expect(result).not.toHaveProperty('bio'); + }); + }); + + describe.each([ + ['jsonCreate', () => TestModel.jsonCreate], + ['jsonUpdate', () => TestModel.jsonUpdate], + ] as const)('%s variant', (_, variant) => { + it('should decode missing key and null to Option.none', () => { + const decode = Schema.decodeUnknownSync(variant()); + + expect(Option.isNone(decode({ name: 'John' }).bio)).toBe(true); + expect(Option.isNone(decode({ name: 'John', bio: null }).bio)).toBe(true); + }); + + it('should encode Option.none as a missing key', () => { + const encode = Schema.encodeUnknownSync(variant()); + const result = encode({ name: 'John', bio: Option.none() }); + + expect(result).not.toHaveProperty('bio'); + }); + }); + + describe('multi-variant fields', () => { + const AuthorId = Schema.String.pipe(Schema.brand('AuthorId')); + + class DateModel extends Model.Class('DateModel')({ + publishedAt: Optional(DateTime), + author: Optional(Reference(AuthorId, 'authors')), + }) {} + + it('should decode missing keys to Option.none for all database variants', () => { + const select = Schema.decodeUnknownSync(DateModel)({}); + const insert = Schema.decodeUnknownSync(DateModel.insert)({}); + const update = Schema.decodeUnknownSync(DateModel.update)({}); + + expect(Option.isNone(select.publishedAt)).toBe(true); + expect(Option.isNone(select.author)).toBe(true); + expect(Option.isNone(insert.publishedAt)).toBe(true); + expect(Option.isNone(insert.author)).toBe(true); + expect(Option.isNone(update.publishedAt)).toBe(true); + expect(Option.isNone(update.author)).toBe(true); + }); + + it('should type the decoded fields as Option', () => { + const encoded: typeof DateModel.Encoded = {}; + const model = Schema.decodeUnknownSync(DateModel)(encoded); + const publishedAt: Option.Option = model.publishedAt; + const author: Option.Option = model.author; + + expect(Option.isNone(publishedAt)).toBe(true); + expect(Option.isNone(author)).toBe(true); + }); + + it('should encode Option.none as null', () => { + const result = Schema.encodeSync(DateModel)( + new DateModel({ publishedAt: Option.none(), author: Option.none() }), + ); + + expect(result.publishedAt).toBeNull(); + expect(result.author).toBeNull(); + }); }); }); @@ -182,6 +312,28 @@ describe('OptionalDeletable', () => { expect(Option.isNone(result.bio)).toBe(true); }); + + it('should decode missing key to Option.none', () => { + const decode = Schema.decodeUnknownSync(TestModel.update); + const result = decode({ name: 'John' }); + + expect(Option.isNone(result.bio)).toBe(true); + }); + + it('should reject null', () => { + const decode = Schema.decodeUnknownSync(TestModel.update); + + expect(() => decode({ name: 'John', bio: null })).toThrow(); + }); + }); + + describe('get variant', () => { + it('should decode missing key to Option.none', () => { + const decode = Schema.decodeUnknownSync(TestModel); + const result = decode({ name: 'John' }); + + expect(Option.isNone(result.bio)).toBe(true); + }); }); describe('encoding get variant', () => { @@ -212,7 +364,14 @@ describe('OptionalDeletable', () => { bio: Option.some(deleteField()), }); - expect(result.bio).toBeDefined(); + expect(result.bio).toBeInstanceOf(Delete); + }); + + it('should encode Option.none as a missing key', () => { + const encode = Schema.encodeSync(TestModel.update); + const result = encode({ name: 'John', bio: Option.none() }); + + expect(result).not.toHaveProperty('bio'); }); }); diff --git a/packages/effect-firebase/src/lib/firestore/model/optional.ts b/packages/effect-firebase/src/lib/firestore/model/optional.ts index 785c7dd..befe97e 100644 --- a/packages/effect-firebase/src/lib/firestore/model/optional.ts +++ b/packages/effect-firebase/src/lib/firestore/model/optional.ts @@ -49,13 +49,15 @@ export const OptionalNull: < /** * Convert a field to one that is optional for all variants. * - * For the database variants, it will accept `null` or `undefined` values. - * For the JSON variants, it will also accept missing keys. + * For the database variants, a missing key, `null` and `undefined` all decode + * to `Option.none()`, and the key is optional in the `Encoded` type. + * `Option.none()` is encoded as `null`, so write payloads always carry the key. + * For the JSON variants, `Option.none()` is encoded as a missing key. */ export type Optional = VariantSchema.Field<{ - readonly select: Schema.OptionFromNullishOr; - readonly insert: Schema.OptionFromNullishOr; - readonly update: Schema.OptionFromNullishOr; + readonly select: Schema.OptionFromOptionalNullOr; + readonly insert: Schema.OptionFromOptionalNullOr; + readonly update: Schema.OptionFromOptionalNullOr; readonly json: Schema.OptionFromOptional; readonly jsonCreate: Schema.OptionFromOptionalNullOr; readonly jsonUpdate: Schema.OptionFromOptionalNullOr; @@ -64,8 +66,10 @@ export type Optional = VariantSchema.Field<{ /** * Convert a field to one that is optional for all variants. * - * For the database variants, it will accept `null` or `undefined` values. - * For the JSON variants, it will also accept missing keys. + * For the database variants, a missing key, `null` and `undefined` all decode + * to `Option.none()`, and the key is optional in the `Encoded` type. + * `Option.none()` is encoded as `null`, so write payloads always carry the key. + * For the JSON variants, `Option.none()` is encoded as a missing key. */ export const Optional: | Schema.Top>( self: Field, @@ -74,18 +78,16 @@ export const Optional: | Schema.Top>( : Field extends VariantSchema.Field ? VariantSchema.Field<{ readonly [K in keyof S]: S[K] extends Schema.Top - ? K extends Model.VariantsDatabase - ? Schema.OptionFromNullishOr - : Schema.OptionFromOptionalNullOr + ? Schema.OptionFromOptionalNullOr : never; }> : never = Model.fieldEvolve({ select: (s: Schema.Top) => - Schema.OptionFromNullishOr(s, { onNoneEncoding: null }), + Schema.OptionFromOptionalNullOr(s, { onNoneEncoding: null }), insert: (s: Schema.Top) => - Schema.OptionFromNullishOr(s, { onNoneEncoding: null }), + Schema.OptionFromOptionalNullOr(s, { onNoneEncoding: null }), update: (s: Schema.Top) => - Schema.OptionFromNullishOr(s, { onNoneEncoding: null }), + Schema.OptionFromOptionalNullOr(s, { onNoneEncoding: null }), json: Schema.OptionFromOptional, jsonCreate: Schema.OptionFromOptionalNullOr, jsonUpdate: Schema.OptionFromOptionalNullOr, @@ -94,13 +96,15 @@ export const Optional: | Schema.Top>( /** * Convert a field to one that is optional for all variants and can be deleted. * - * For the database variants, it will accept `undefined` or `Delete` values. - * For the JSON variants, it will also accept missing keys. + * For the database variants, a missing key or `undefined` decodes to + * `Option.none()`, which is encoded as a missing key. The `update` variant + * additionally accepts a `Delete` sentinel. + * The `jsonCreate` and `jsonUpdate` variants additionally accept `null`. */ export type OptionalDeletable = VariantSchema.Field<{ readonly select: Schema.OptionFromOptional; readonly insert: Schema.OptionFromOptional; - readonly update: Schema.OptionFromUndefinedOr< + readonly update: Schema.OptionFromOptional< Schema.Union >; readonly json: Schema.OptionFromOptional; @@ -117,18 +121,20 @@ export const OptionalDeletable: < : Field extends VariantSchema.Field ? VariantSchema.Field<{ readonly [K in keyof S]: S[K] extends Schema.Top - ? K extends Model.VariantsDatabase - ? Schema.OptionFromUndefinedOr - : Schema.OptionFromUndefinedOr< + ? K extends 'update' + ? Schema.OptionFromOptional< Schema.Union > + : K extends 'jsonCreate' | 'jsonUpdate' + ? Schema.OptionFromOptionalNullOr + : Schema.OptionFromOptional : never; }> : never = Model.fieldEvolve({ select: Schema.OptionFromOptional, insert: Schema.OptionFromOptional, update: (s: Schema.Top) => - Schema.OptionFromUndefinedOr(Schema.Union([s, DeleteInstance])), + Schema.OptionFromOptional(Schema.Union([s, DeleteInstance])), json: Schema.OptionFromOptional, jsonCreate: Schema.OptionFromOptionalNullOr, jsonUpdate: Schema.OptionFromOptionalNullOr, diff --git a/packages/effect-firebase/src/lib/firestore/model/repository.spec.ts b/packages/effect-firebase/src/lib/firestore/model/repository.spec.ts index 881f5a7..0f78d36 100644 --- a/packages/effect-firebase/src/lib/firestore/model/repository.spec.ts +++ b/packages/effect-firebase/src/lib/firestore/model/repository.spec.ts @@ -232,14 +232,13 @@ describe('Repository', () => { data: { title: 'Hello', createdAt: undefined, updatedAt: undefined }, }); - const insertMissingInsertOnlyField = repo.set( - PostId.make('post-1'), - // @ts-expect-error the default insert variant requires createdAt. - { data: { title: 'Hello', updatedAt: undefined } }, - ); + // Server-stamped fields may be omitted from the insert variant. + const insertWithoutStampedFields = repo.set(PostId.make('post-1'), { + data: { title: 'Hello' }, + }); expect(updateWithInsertOnlyField).toBeDefined(); - expect(insertMissingInsertOnlyField).toBeDefined(); + expect(insertWithoutStampedFields).toBeDefined(); }); }); @@ -513,7 +512,7 @@ describe('Repository', () => { await Effect.runPromise( repo.update( PostId.make('post-1'), - { profile: Option.none() }, + { title: 'Hello', profile: Option.none() }, { merge: true }, ), ); @@ -526,7 +525,9 @@ describe('Repository', () => { ); expect(updateMock.mock.calls).toHaveLength(2); - expect(payloadOf(updateMock)).toEqual({ profile: undefined }); + // Option.none() leaves the field untouched: the key is omitted rather + // than written as undefined. + expect(payloadOf(updateMock)).toEqual({ title: 'Hello' }); expect( ( updateMock.mock.calls[1] as unknown as [ From f915273db47fafccfe5916688840ab92d174421b Mon Sep 17 00:00:00 2001 From: Frederik Wallner Date: Mon, 14 Sep 2026 12:33:32 +0000 Subject: [PATCH 2/4] test(firestore): cover decoding defaults on spread struct fields Regression test for models built from a struct whose field carries a decoding default, and a MIGRATION note for the Effect v4 replacement of `Schema.optionalWith(s, { default })`. Closes #29 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UTvaTdYRhJ8MdaKxQ19miS --- packages/effect-firebase/MIGRATION.md | 25 +++++++----- .../lib/firestore/model/repository.spec.ts | 38 +++++++++++++++++++ 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/packages/effect-firebase/MIGRATION.md b/packages/effect-firebase/MIGRATION.md index 71906cb..a29b2b6 100644 --- a/packages/effect-firebase/MIGRATION.md +++ b/packages/effect-firebase/MIGRATION.md @@ -401,16 +401,21 @@ the mock's simulated states live. Additional Effect v4 breaking changes you may encounter in your own code: -| v3 | v4 | -| ------------------------------------------------- | -------------------------------------------------------------- | -| `Effect.catchAll(f)` | `Effect.catch(f)` | -| `Effect.catchAllDefect(f)` | `Effect.catchDefect(f)` | -| `Effect.catchAllCause(f)` | `Effect.catchCause(f)` | -| `Schema.Union(a, b, ...)` | `Schema.Union([a, b, ...])` | -| `struct.pick('field')` | `struct.mapFields(Struct.pick(['field']))` | -| `ParseResult.ArrayFormatter.formatErrorSync(e)` | `e.message` (use `Schema.isSchemaError(e)` to narrow) | -| `import { ParseError } from 'effect/ParseResult'` | `import { Schema } from 'effect'` → use `Schema.isSchemaError` | -| `Context.Tag('id')()` | `Context.Service()('id')` | +| v3 | v4 | +| ------------------------------------------------- | --------------------------------------------------------------------------- | +| `Effect.catchAll(f)` | `Effect.catch(f)` | +| `Effect.catchAllDefect(f)` | `Effect.catchDefect(f)` | +| `Effect.catchAllCause(f)` | `Effect.catchCause(f)` | +| `Schema.Union(a, b, ...)` | `Schema.Union([a, b, ...])` | +| `Schema.optionalWith(s, { default: () => v })` | `Schema.optionalKey(s).pipe(Schema.withDecodingDefault(Effect.succeed(v)))` | +| `struct.pick('field')` | `struct.mapFields(Struct.pick(['field']))` | +| `ParseResult.ArrayFormatter.formatErrorSync(e)` | `e.message` (use `Schema.isSchemaError(e)` to narrow) | +| `import { ParseError } from 'effect/ParseResult'` | `import { Schema } from 'effect'` → use `Schema.isSchemaError` | +| `Context.Tag('id')()` | `Context.Service()('id')` | + +Fields spread from a struct with a decoding default (the +`Schema.optionalWith(..., { default })` pattern) work in `Model.Class` again; +v0.x rejected them with `Unsupported schema`. For a complete list of Effect v4 breaking changes beyond what's covered here, see the [Effect migration guide](https://effect.website/docs/migration-guide). diff --git a/packages/effect-firebase/src/lib/firestore/model/repository.spec.ts b/packages/effect-firebase/src/lib/firestore/model/repository.spec.ts index 0f78d36..8fdaafe 100644 --- a/packages/effect-firebase/src/lib/firestore/model/repository.spec.ts +++ b/packages/effect-firebase/src/lib/firestore/model/repository.spec.ts @@ -260,6 +260,44 @@ describe('Repository', () => { }); }); + it('applies decoding defaults from spread struct fields', async () => { + // Regression for fields spread from a struct that carries a decoding + // default: the model must accept them and fill the default on read. + const WithDefault = Schema.Struct({ + status: Schema.optionalKey(Schema.String).pipe( + Schema.withDecodingDefault(Effect.succeed('draft')), + ), + }); + class DefaultedModel extends Model.Class( + 'DefaultedModel', + )({ + id: Model.GeneratedByDb(PostId), + ...WithDefault.fields, + }) {} + const getMock = vi.fn(() => + Effect.succeed(Option.some(snap('post-1', {}))), + ); + const repo = await Effect.runPromise( + makeRepository(DefaultedModel, { + collectionPath: 'posts', + idField: 'id', + spanPrefix: 'test', + }).pipe(Effect.provide(makeLayer({ get: getMock }))), + ); + const result = await Effect.runPromise( + repo.getById(PostId.make('post-1')), + ); + + expect(Option.getOrThrow(result)).toMatchObject({ + id: 'post-1', + status: 'draft', + }); + expect(Schema.encodeSync(DefaultedModel.insert)({})).toEqual({}); + expect(Schema.decodeUnknownSync(DefaultedModel.insert)({}).status).toBe( + 'draft', + ); + }); + it('returns None when the document does not exist', async () => { const getMock = vi.fn(() => Effect.succeed(Option.none())); const repo = await Effect.runPromise(makeRepo({ get: getMock })); From 5107ad3a93bd73275a5f582a1cd3668bbf67117d Mon Sep 17 00:00:00 2001 From: Frederik Wallner Date: Mon, 14 Sep 2026 12:40:24 +0000 Subject: [PATCH 3/4] fix(firestore): omit dotted-path leaves whose schema drops the key The per-path leaf encoder read the encoded property back unconditionally, so a nested `OptionalDeletable` given `Option.none()` produced a key with an `undefined` value that the Firebase SDKs reject. The encoder now reports key presence and the update loop skips omitted leaves. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UTvaTdYRhJ8MdaKxQ19miS --- .../lib/firestore/model/repository.spec.ts | 44 +++++++++++++++++++ .../src/lib/firestore/model/repository.ts | 20 ++++++--- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/packages/effect-firebase/src/lib/firestore/model/repository.spec.ts b/packages/effect-firebase/src/lib/firestore/model/repository.spec.ts index 8fdaafe..6cbf341 100644 --- a/packages/effect-firebase/src/lib/firestore/model/repository.spec.ts +++ b/packages/effect-firebase/src/lib/firestore/model/repository.spec.ts @@ -566,6 +566,7 @@ describe('Repository', () => { // Option.none() leaves the field untouched: the key is omitted rather // than written as undefined. expect(payloadOf(updateMock)).toEqual({ title: 'Hello' }); + expect(payloadOf(updateMock)).not.toHaveProperty('profile'); expect( ( updateMock.mock.calls[1] as unknown as [ @@ -576,6 +577,49 @@ describe('Repository', () => { ).toEqual({ profile: deleteField() }); }); + it('omits a nested OptionalDeletable given Option.none()', async () => { + class ProfileModel extends Model.Class('ProfileModel')({ + id: Model.GeneratedByDb(PostId), + title: Schema.String, + profile: Model.Struct({ bio: OptionalDeletable(Schema.String) }), + }) {} + const updateMock = vi.fn(() => Effect.succeed(undefined)); + const repo = await Effect.runPromise( + makeRepository(ProfileModel, { + collectionPath: 'posts', + idField: 'id', + spanPrefix: 'test', + }).pipe(Effect.provide(makeLayer({ update: updateMock }))), + ); + await Effect.runPromise( + repo.update(PostId.make('post-1'), { + title: 'Hello', + 'profile.bio': Option.none(), + }), + ); + await Effect.runPromise( + repo.update( + PostId.make('post-1'), + { title: 'Hello', profile: { bio: Option.none() } }, + { merge: true }, + ), + ); + await Effect.runPromise( + repo.update(PostId.make('post-1'), { + 'profile.bio': Option.some(deleteField()), + }), + ); + + const payloads = updateMock.mock.calls.map( + (call) => (call as unknown as [string, Record])[1], + ); + expect(payloads[0]).toEqual({ title: 'Hello' }); + expect(payloads[0]).not.toHaveProperty('profile.bio'); + expect(payloads[1]).toEqual({ title: 'Hello' }); + expect(payloads[1]).not.toHaveProperty('profile.bio'); + expect(payloads[2]).toEqual({ 'profile.bio': deleteField() }); + }); + it('drops empty objects, failing invalid-argument if nothing is left', async () => { const updateMock = vi.fn(() => Effect.succeed(undefined)); const repo = await Effect.runPromise( diff --git a/packages/effect-firebase/src/lib/firestore/model/repository.ts b/packages/effect-firebase/src/lib/firestore/model/repository.ts index 2fd8509..9b149a9 100644 --- a/packages/effect-firebase/src/lib/firestore/model/repository.ts +++ b/packages/effect-firebase/src/lib/firestore/model/repository.ts @@ -464,9 +464,12 @@ export const makeRepository = < // that does not resolve stays in the struct payload, where the strict // encoder rejects it by name. Encoders are cached per path because // Schema.encodeUnknownEffect compiles on construction. + // A leaf whose schema omits the key on encode (for example an + // `OptionalDeletable` given `Option.none()`) resolves to `None`, so the + // caller can leave it out of the payload rather than write `undefined`. type LeafEncoder = ( value: unknown, - ) => Effect.Effect; + ) => Effect.Effect, Schema.SchemaError, unknown>; const leafEncoders = new Map(); const leafEncoder = (path: string): Option.Option => { const cached = leafEncoders.get(path); @@ -477,10 +480,12 @@ export const makeRepository = < Fetch.strictEncoding, ); const encoder: LeafEncoder = (value) => - Effect.map( - encodeLeaf({ [path]: value }), - (encoded) => (encoded as Record)[path], - ); + Effect.map(encodeLeaf({ [path]: value }), (encoded) => { + const record = encoded as Record; + return Object.hasOwn(record, path) + ? Option.some(record[path]) + : Option.none(); + }); leafEncoders.set(path, encoder); return encoder; }); @@ -508,7 +513,10 @@ export const makeRepository = < const { [options.idField as string]: encodedId, ...payload } = (yield* encodeUpdateFields(fields)) as Record; for (const [key, value, encode] of paths) { - payload[key] = yield* encode(value); + const encoded = yield* encode(value); + if (Option.isSome(encoded)) { + payload[key] = encoded.value; + } } // Firestore rejects an empty update with a message about argument From 9faec71af7b768d34b41b492fdd1d82e37d6ef11 Mon Sep 17 00:00:00 2001 From: Frederik Wallner Date: Mon, 14 Sep 2026 12:51:19 +0000 Subject: [PATCH 4/4] docs(firestore): recommend Schema.optionalKey for plain optional fields Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UTvaTdYRhJ8MdaKxQ19miS --- packages/effect-firebase/AGENTS.md | 7 +++++++ packages/effect-firebase/MIGRATION.md | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/packages/effect-firebase/AGENTS.md b/packages/effect-firebase/AGENTS.md index e6c6da5..5f67778 100644 --- a/packages/effect-firebase/AGENTS.md +++ b/packages/effect-firebase/AGENTS.md @@ -77,6 +77,7 @@ export class PostModel extends Model.Class('PostModel')({ likes: Firestore.Number, // accepts Firestore.increment(n) in update tags: Firestore.Array(Schema.String), // accepts arrayUnion/arrayRemove in update summary: Firestore.OptionalDeletable(Schema.String), // Option in app; Firestore.delete() removes it + subtitle: Schema.optionalKey(Schema.String), // plain `string | absent`; prefer over Schema.optional checked: Schema.Boolean.pipe( Schema.withDecodingDefault(Effect.succeed(false)), ), @@ -450,6 +451,12 @@ root: https://github.com/fwal/effect-firebase/blob/main/REACT.md. nested partial; a plain `{ a: { b: v } }` replaces the whole `a` map. Undeclared keys fail with `SchemaError`; `update` never silently drops them. +11. Optional fields: use `Firestore.Optional*` when the app wants an + `Option`, and `Schema.optionalKey(s)` for a plain `T | absent` field. + Avoid `Schema.optional(s)`: it lets `{ field: undefined }` through the + schema, and the Firebase SDKs reject `undefined` as a value at write + time. `optionalKey` fails with `SchemaError` instead. For a default, + `Schema.optionalKey(s).pipe(Schema.withDecodingDefault(Effect.succeed(v)))`. ## Where to look diff --git a/packages/effect-firebase/MIGRATION.md b/packages/effect-firebase/MIGRATION.md index a29b2b6..436ef44 100644 --- a/packages/effect-firebase/MIGRATION.md +++ b/packages/effect-firebase/MIGRATION.md @@ -211,6 +211,13 @@ spelled these fields out as `Schema.OptionFromNullishOr<...>` must use `Schema.OptionFromOptionalNullOr<...>` instead. `OptionalNull` and `ReferenceOptional` are unchanged and still require the key to be present. +**Plain optional fields.** For a field that should be `T | absent` in the +app (no `Option`), declare it with `Schema.optionalKey(s)` rather than +`Schema.optional(s)`. `Schema.optional` also admits `undefined` as a value, +which passes the schema but is rejected by the Firebase SDKs on write; +`optionalKey` rejects it with a `SchemaError` up front. Firestore documents +never contain `undefined`, so nothing is lost on read. + **`OptionalDeletable` update variant.** `Option.none()` in an `update` payload is now encoded as a missing key instead of `undefined` (which the Firebase SDKs reject as a value), and a document without the key decodes