diff --git a/lib/interface/schema.d.ts b/lib/interface/schema.d.ts index 7501a67..e01db86 100644 --- a/lib/interface/schema.d.ts +++ b/lib/interface/schema.d.ts @@ -307,16 +307,34 @@ export declare const parseOrThrow: (schema: TSchema, * **`strictNullChecks` is now enabled, so the collapse no longer occurs.** * Verified by inferring `z.object({u: z.union([...])})` and observing that the * required key is now correctly reported as missing (`TS2741`) where it - * previously type-checked clean. The wrapper is therefore no longer necessary - * for its original purpose, and is retained only so that removing it — which - * changes the errors reported at its two call sites — is a deliberate change - * with its own tests rather than a side effect of a compiler-flag change. - * - * The trade-off it carries is error granularity: a failure reports one issue at - * the key's path rather than the inner schema's per-member detail, which is why - * the message is a required argument rather than a generic default. That - * trade-off is now a cost without a corresponding benefit, so prefer the inner - * schema directly for new code, and see the note above before adding a call. + * previously type-checked clean. Both former call sites have been migrated and + * their keys remain inferred as required without the wrapper. + * + * ## Use zod's native `error` parameter instead + * + * The replacement is not "delete the wrapper" — doing only that degrades the + * reported message. A failed `z.union` reports `Invalid input`, because zod + * cannot know which member the caller intended, and a failed `z.string()` + * inside a `.nullable()` reports `expected string`, which is true of the inner + * schema but false of the field. Both send a caller looking for the wrong fix. + * + * Declare the message on the schema itself, which preserves the wording *and* + * the correct inference, and drops the `z.custom` indirection: + * + * ```ts + * // instead of requiredKey(z.union([...]), 'Expected …') + * z.union([...], {error: 'Expected …'}) + * // instead of requiredKey(z.string().nullable(), 'Expected … or null') + * z.string({error: 'Expected … or null'}).nullable() + * ``` + * + * This is **retained rather than removed** because it is a published export and + * dropping it would be a breaking change for any consumer that imported it. + * + * @deprecated Declare the message on the schema with zod's native `error` + * parameter — `z.union([...], {error})` or `z.string({error}).nullable()` — + * which reports the same message, infers the key as required, and avoids + * wrapping the schema in `z.custom`. * * @template TSchema The inner schema, which performs the actual validation. * @param {TSchema} schema - Schema describing the accepted values. diff --git a/lib/interface/schema.js b/lib/interface/schema.js index ca41f88..df6fc7a 100644 --- a/lib/interface/schema.js +++ b/lib/interface/schema.js @@ -138,16 +138,34 @@ export const parseOrThrow = (schema, value, label) => { * **`strictNullChecks` is now enabled, so the collapse no longer occurs.** * Verified by inferring `z.object({u: z.union([...])})` and observing that the * required key is now correctly reported as missing (`TS2741`) where it - * previously type-checked clean. The wrapper is therefore no longer necessary - * for its original purpose, and is retained only so that removing it — which - * changes the errors reported at its two call sites — is a deliberate change - * with its own tests rather than a side effect of a compiler-flag change. - * - * The trade-off it carries is error granularity: a failure reports one issue at - * the key's path rather than the inner schema's per-member detail, which is why - * the message is a required argument rather than a generic default. That - * trade-off is now a cost without a corresponding benefit, so prefer the inner - * schema directly for new code, and see the note above before adding a call. + * previously type-checked clean. Both former call sites have been migrated and + * their keys remain inferred as required without the wrapper. + * + * ## Use zod's native `error` parameter instead + * + * The replacement is not "delete the wrapper" — doing only that degrades the + * reported message. A failed `z.union` reports `Invalid input`, because zod + * cannot know which member the caller intended, and a failed `z.string()` + * inside a `.nullable()` reports `expected string`, which is true of the inner + * schema but false of the field. Both send a caller looking for the wrong fix. + * + * Declare the message on the schema itself, which preserves the wording *and* + * the correct inference, and drops the `z.custom` indirection: + * + * ```ts + * // instead of requiredKey(z.union([...]), 'Expected …') + * z.union([...], {error: 'Expected …'}) + * // instead of requiredKey(z.string().nullable(), 'Expected … or null') + * z.string({error: 'Expected … or null'}).nullable() + * ``` + * + * This is **retained rather than removed** because it is a published export and + * dropping it would be a breaking change for any consumer that imported it. + * + * @deprecated Declare the message on the schema with zod's native `error` + * parameter — `z.union([...], {error})` or `z.string({error}).nullable()` — + * which reports the same message, infers the key as required, and avoids + * wrapping the schema in `z.custom`. * * @template TSchema The inner schema, which performs the actual validation. * @param {TSchema} schema - Schema describing the accepted values. diff --git a/lib/model/Block.d.ts b/lib/model/Block.d.ts index 79a6d7b..ab85e81 100644 --- a/lib/model/Block.d.ts +++ b/lib/model/Block.d.ts @@ -85,7 +85,7 @@ export declare namespace Block { */ const Schema: z.ZodObject<{ type: z.ZodEnum; - value: z.ZodCustom | unknown[], string | number | Record | unknown[]>; + value: z.ZodUnion, z.ZodArray]>; label: z.ZodString; width: z.ZodOptional; height: z.ZodOptional; diff --git a/lib/model/Block.js b/lib/model/Block.js index 81dea91..258e579 100644 --- a/lib/model/Block.js +++ b/lib/model/Block.js @@ -3,7 +3,7 @@ * Copyright Furcata. All Rights Reserved. */ import { z } from "zod"; -import { counter, parseOrThrow, parseResult, requiredKey, } from "../interface/schema.js"; +import { counter, parseOrThrow, parseResult, } from "../interface/schema.js"; /** * Namespace for content-block primitives used to compose rich-media sections * within {@link EventData.Interface} and other structured content documents. @@ -42,7 +42,7 @@ export var Block; * schema so it is the single source of truth for both the validation and the * inferred output type of the `value` field below. */ - const blockValueSchema = z.union([z.string(), z.number(), z.record(z.string(), z.unknown()), z.array(z.unknown())]); + const blockValueSchema = z.union([z.string(), z.number(), z.record(z.string(), z.unknown()), z.array(z.unknown())], { error: 'Expected a string, number, object or array block value' }); /** * Runtime schema producing {@link Interface}. * @@ -67,16 +67,12 @@ export var Block; * {@link Interface.type}, and that correspondence is the renderer's to * enforce rather than this schema's. * - * Wrapped in `requiredKey` because this key is required and its schema is a - * `z.union`, which zod inferred as an optional key under the - * `strictNullChecks: false` setting this package previously used. Without - * the wrapper {@link parse} would have returned a type claiming `value` may - * be absent when at runtime it never is. `strictNullChecks` is now enabled - * and the inference is correct without the wrapper, which is retained here - * only so that removing it is a deliberate change with its own tests rather - * than a side effect of a compiler-flag change; see `requiredKey`. + * Carries an explicit `error` message on the union itself, because zod + * reports a failed union as the unhelpful `Invalid input` — it cannot know + * which member the caller intended. Naming the accepted shapes is the + * difference between a caller seeing what to send and seeing nothing. */ - value: requiredKey(blockValueSchema, 'Expected a string, number, object or array block value'), + value: blockValueSchema, /** * See {@link Interface.label}. Required but permitted to be empty, because * a block with no caption is a legitimate authoring choice. diff --git a/lib/model/EventData.d.ts b/lib/model/EventData.d.ts index 5840bcc..43aa1f0 100644 --- a/lib/model/EventData.d.ts +++ b/lib/model/EventData.d.ts @@ -200,7 +200,7 @@ export declare namespace EventData { uid: z.ZodOptional>; blocks: z.ZodOptional; - value: z.ZodCustom | unknown[], string | number | Record | unknown[]>; + value: z.ZodUnion, z.ZodArray]>; label: z.ZodString; width: z.ZodOptional; height: z.ZodOptional; diff --git a/lib/model/Idempotency.d.ts b/lib/model/Idempotency.d.ts index b097219..3812fa4 100644 --- a/lib/model/Idempotency.d.ts +++ b/lib/model/Idempotency.d.ts @@ -154,7 +154,7 @@ export declare namespace Idempotency { progress: z.ZodOptional>; response: z.ZodOptional; + body: z.ZodNullable; truncated: z.ZodBoolean; }, z.core.$loose>>; lockExpires: z.ZodOptional>>; diff --git a/lib/model/Idempotency.js b/lib/model/Idempotency.js index 8e98b0a..5f2348a 100644 --- a/lib/model/Idempotency.js +++ b/lib/model/Idempotency.js @@ -4,7 +4,7 @@ */ import { z } from 'zod'; import { baseFirestoreShape } from '../interface/base_db.js'; -import { counter, nonEmptyString, parseOrThrow, parseResult, requiredKey, timestampLike, token, } from '../interface/schema.js'; +import { counter, nonEmptyString, parseOrThrow, parseResult, timestampLike, token, } from '../interface/schema.js'; /** * Namespace for idempotency records: the durable claim that makes a * client-initiated request execute at most once. @@ -52,14 +52,9 @@ export var Idempotency; /** * Schema for {@link Response}. * - * {@link Response.body} is required **and** nullable. It is wrapped in - * `requiredKey` because zod inferred a required `z.nullable` key as optional - * under the `strictNullChecks: false` setting this package previously used. - * That setting is now enabled and the inference is correct without the - * wrapper, which is retained pending a deliberate removal; see that helper. - * `null` here means the original response genuinely had no body, which is a - * different claim from the field being absent, so the distinction has to - * survive. + * {@link Response.body} is required **and** nullable. `null` here means the + * original response genuinely had no body, which is a different claim from + * the field being absent, so the distinction has to survive. */ const responseSchema = z.looseObject({ /** @@ -68,9 +63,12 @@ export var Idempotency; */ status: z.int().min(100).max(599), /** - * See {@link Response.body}. + * See {@link Response.body}. The `error` message is declared on the schema + * so a rejection names `null` as accepted; zod's default would report + * `expected string`, which is true of the inner schema but false of the + * field, and would send a caller looking for the wrong fix. */ - body: requiredKey(z.string().nullable(), 'Expected a response body string or null'), + body: z.string({ error: 'Expected a response body string or null' }).nullable(), /** * See {@link Response.truncated}. */ diff --git a/src/interface/schema.ts b/src/interface/schema.ts index 6e7da81..d3b9472 100644 --- a/src/interface/schema.ts +++ b/src/interface/schema.ts @@ -374,16 +374,34 @@ export const parseOrThrow = ( * **`strictNullChecks` is now enabled, so the collapse no longer occurs.** * Verified by inferring `z.object({u: z.union([...])})` and observing that the * required key is now correctly reported as missing (`TS2741`) where it - * previously type-checked clean. The wrapper is therefore no longer necessary - * for its original purpose, and is retained only so that removing it — which - * changes the errors reported at its two call sites — is a deliberate change - * with its own tests rather than a side effect of a compiler-flag change. - * - * The trade-off it carries is error granularity: a failure reports one issue at - * the key's path rather than the inner schema's per-member detail, which is why - * the message is a required argument rather than a generic default. That - * trade-off is now a cost without a corresponding benefit, so prefer the inner - * schema directly for new code, and see the note above before adding a call. + * previously type-checked clean. Both former call sites have been migrated and + * their keys remain inferred as required without the wrapper. + * + * ## Use zod's native `error` parameter instead + * + * The replacement is not "delete the wrapper" — doing only that degrades the + * reported message. A failed `z.union` reports `Invalid input`, because zod + * cannot know which member the caller intended, and a failed `z.string()` + * inside a `.nullable()` reports `expected string`, which is true of the inner + * schema but false of the field. Both send a caller looking for the wrong fix. + * + * Declare the message on the schema itself, which preserves the wording *and* + * the correct inference, and drops the `z.custom` indirection: + * + * ```ts + * // instead of requiredKey(z.union([...]), 'Expected …') + * z.union([...], {error: 'Expected …'}) + * // instead of requiredKey(z.string().nullable(), 'Expected … or null') + * z.string({error: 'Expected … or null'}).nullable() + * ``` + * + * This is **retained rather than removed** because it is a published export and + * dropping it would be a breaking change for any consumer that imported it. + * + * @deprecated Declare the message on the schema with zod's native `error` + * parameter — `z.union([...], {error})` or `z.string({error}).nullable()` — + * which reports the same message, infers the key as required, and avoids + * wrapping the schema in `z.custom`. * * @template TSchema The inner schema, which performs the actual validation. * @param {TSchema} schema - Schema describing the accepted values. diff --git a/src/model/Block.ts b/src/model/Block.ts index 89af95b..3b32a02 100644 --- a/src/model/Block.ts +++ b/src/model/Block.ts @@ -11,7 +11,6 @@ import { ParseResult, parseOrThrow, parseResult, - requiredKey, } from "../interface/schema.js"; /** @@ -87,7 +86,7 @@ export namespace Block { * schema so it is the single source of truth for both the validation and the * inferred output type of the `value` field below. */ - const blockValueSchema = z.union([z.string(), z.number(), z.record(z.string(), z.unknown()), z.array(z.unknown())]); + const blockValueSchema = z.union([z.string(), z.number(), z.record(z.string(), z.unknown()), z.array(z.unknown())], {error: 'Expected a string, number, object or array block value'}); /** * Runtime schema producing {@link Interface}. @@ -113,16 +112,12 @@ export namespace Block { * {@link Interface.type}, and that correspondence is the renderer's to * enforce rather than this schema's. * - * Wrapped in `requiredKey` because this key is required and its schema is a - * `z.union`, which zod inferred as an optional key under the - * `strictNullChecks: false` setting this package previously used. Without - * the wrapper {@link parse} would have returned a type claiming `value` may - * be absent when at runtime it never is. `strictNullChecks` is now enabled - * and the inference is correct without the wrapper, which is retained here - * only so that removing it is a deliberate change with its own tests rather - * than a side effect of a compiler-flag change; see `requiredKey`. + * Carries an explicit `error` message on the union itself, because zod + * reports a failed union as the unhelpful `Invalid input` — it cannot know + * which member the caller intended. Naming the accepted shapes is the + * difference between a caller seeing what to send and seeing nothing. */ - value: requiredKey(blockValueSchema, 'Expected a string, number, object or array block value'), + value: blockValueSchema, /** * See {@link Interface.label}. Required but permitted to be empty, because * a block with no caption is a legitimate authoring choice. diff --git a/src/model/Idempotency.ts b/src/model/Idempotency.ts index d8dc776..d55e295 100644 --- a/src/model/Idempotency.ts +++ b/src/model/Idempotency.ts @@ -11,7 +11,6 @@ import { ParseResult, parseOrThrow, parseResult, - requiredKey, timestampLike, TimestampLike, token, @@ -150,14 +149,9 @@ export namespace Idempotency { /** * Schema for {@link Response}. * - * {@link Response.body} is required **and** nullable. It is wrapped in - * `requiredKey` because zod inferred a required `z.nullable` key as optional - * under the `strictNullChecks: false` setting this package previously used. - * That setting is now enabled and the inference is correct without the - * wrapper, which is retained pending a deliberate removal; see that helper. - * `null` here means the original response genuinely had no body, which is a - * different claim from the field being absent, so the distinction has to - * survive. + * {@link Response.body} is required **and** nullable. `null` here means the + * original response genuinely had no body, which is a different claim from + * the field being absent, so the distinction has to survive. */ const responseSchema = z.looseObject({ /** @@ -166,9 +160,12 @@ export namespace Idempotency { */ status: z.int().min(100).max(599), /** - * See {@link Response.body}. + * See {@link Response.body}. The `error` message is declared on the schema + * so a rejection names `null` as accepted; zod's default would report + * `expected string`, which is true of the inner schema but false of the + * field, and would send a caller looking for the wrong fix. */ - body: requiredKey(z.string().nullable(), 'Expected a response body string or null'), + body: z.string({error: 'Expected a response body string or null'}).nullable(), /** * See {@link Response.truncated}. */ diff --git a/test/model/Block.test.ts b/test/model/Block.test.ts index ec53e25..ce98ff2 100644 --- a/test/model/Block.test.ts +++ b/test/model/Block.test.ts @@ -320,6 +320,23 @@ describe('Block.Schema', () => { }); }); + describe('rejection message for the value union', () => { + it('should name the accepted shapes rather than reporting a bare invalid input', () => { + const result = Block.safeParse({ ...validBlock(), value: true }); + expect(result.success).toBe(false); + const issue = result.issues?.find((entry) => entry.path === 'value'); + expect(issue?.message).toBe('Expected a string, number, object or array block value'); + }); + + it('should name the accepted shapes when the value is absent entirely', () => { + const { value: _omitted, ...withoutValue } = validBlock(); + const result = Block.safeParse(withoutValue); + expect(result.success).toBe(false); + const issue = result.issues?.find((entry) => entry.path === 'value'); + expect(issue?.message).toBe('Expected a string, number, object or array block value'); + }); + }); + describe('throwing form', () => { it('should throw a ParseError naming the shape', () => { expect(() => Block.parse({ type: Block.Type.text })).toThrow(ParseError); diff --git a/test/model/Idempotency.test.ts b/test/model/Idempotency.test.ts index 8b3b682..612e5cf 100644 --- a/test/model/Idempotency.test.ts +++ b/test/model/Idempotency.test.ts @@ -126,6 +126,14 @@ describe('Idempotency.Schema', () => { }); describe('the stored response', () => { + it('should name null as accepted when the body is rejected', () => { + const record = {...validRecord(), response: {status: 200, body: 42, truncated: false}}; + const result = Idempotency.safeParse(record); + expect(result.success).toBe(false); + const issue = result.issues?.find((entry) => entry.path === 'response.body'); + expect(issue?.message).toBe('Expected a response body string or null'); + }); + it('should accept a null body, which means the original response had none', () => { const record = {...validRecord(), response: {status: 204, body: null, truncated: false}}; const parsed = Idempotency.parse(record);