diff --git a/clients/web/src/test/core/nullableUnion.test.ts b/clients/web/src/test/core/nullableUnion.test.ts index f56abbdd..915e7e8e 100644 --- a/clients/web/src/test/core/nullableUnion.test.ts +++ b/clients/web/src/test/core/nullableUnion.test.ts @@ -144,6 +144,39 @@ describe("normalizeNullableUnion", () => { }); }); + // JSON Schema's `enum` is untyped, so a typeless branch only implies strings + // when every member is one. Guessing otherwise would hand a number to a + // renderer that declared the option list `string[]`. + it("does not infer string for a typeless non-string enum branch", () => { + const schema = { anyOf: [{ enum: [1, 2] }, { type: "null" as const }] }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); + + it("does not infer string for a typeless mixed enum branch", () => { + const schema = { anyOf: [{ enum: ["a", 2] }, { type: "null" as const }] }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); + + it("does not infer string for a typeless empty enum branch", () => { + const schema = { anyOf: [{ enum: [] }, { type: "null" as const }] }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); + + // An explicit `type` is authoritative — the enum members are the server's + // problem at that point, not an inference this function is making. + it("still collapses a non-string enum branch that declares its type", () => { + expect( + normalizeNullableUnion({ + anyOf: [{ type: "number", enum: [1, 2] }, { type: "null" }], + }), + ).toEqual({ + type: "number", + enum: [1, 2], + anyOf: undefined, + nullable: true, + }); + }); + it("infers string for a typeless enum branch", () => { expect( normalizeNullableUnion({ diff --git a/clients/web/src/utils/jsonUtils.test.ts b/clients/web/src/utils/jsonUtils.test.ts index 57a61132..4a69409c 100644 --- a/clients/web/src/utils/jsonUtils.test.ts +++ b/clients/web/src/utils/jsonUtils.test.ts @@ -221,6 +221,42 @@ describe("collectSchemaDefaults", () => { it("returns an empty object when the schema has no properties", () => { expect(collectSchemaDefaults({ type: "object" })).toEqual({}); }); + + // The form normalizes a nullable union before rendering, so this has to as + // well: a nested object's `properties` live on the union's surviving branch, + // and a default only visible there would be displayed but never submitted. + it("collects defaults hoisted out of a nullable object union", () => { + const schema: InspectorFormSchema = { + type: "object", + properties: { + profile: { + anyOf: [ + { + type: "object", + properties: { nick: { type: "string", default: "ada" } }, + }, + { type: "null" }, + ], + }, + }, + }; + expect(collectSchemaDefaults(schema)).toEqual({ + profile: { nick: "ada" }, + }); + }); + + it("keeps a default declared on the union wrapper itself", () => { + const schema: InspectorFormSchema = { + type: "object", + properties: { + direction: { + default: "envio", + anyOf: [{ type: "string", enum: ["envio"] }, { type: "null" }], + }, + }, + }; + expect(collectSchemaDefaults(schema)).toEqual({ direction: "envio" }); + }); }); describe("hasMissingRequiredFields", () => { @@ -247,4 +283,44 @@ describe("hasMissingRequiredFields", () => { false, ); }); + + // JSON Schema `required` constrains presence, not content — so an explicit + // `null` satisfies a required field whose schema admits null. Since #1928 the + // user can produce exactly that by clearing a nullable enum, and treating it + // as missing would disable submit on a value the schema calls valid. + it("accepts an explicit null for a required field that admits null", () => { + const nullable: InspectorFormSchema = { + type: "object", + properties: { + direction: { + anyOf: [{ type: "string", enum: ["envio"] }, { type: "null" }], + }, + }, + required: ["direction"], + }; + expect(hasMissingRequiredFields(nullable, { direction: null })).toBe(false); + expect(hasMissingRequiredFields(nullable, {})).toBe(true); + }); + + it("accepts an explicit null for a required type: [T, null] field", () => { + const nullable: InspectorFormSchema = { + type: "object", + properties: { note: { type: ["string", "null"] } }, + required: ["note"], + }; + expect(hasMissingRequiredFields(nullable, { note: null })).toBe(false); + }); + + it("still rejects null for a required field whose schema is not nullable", () => { + expect(hasMissingRequiredFields(schema, { name: null })).toBe(true); + }); + + it("rejects null for a required field the schema does not describe", () => { + const undescribed: InspectorFormSchema = { + type: "object", + properties: {}, + required: ["ghost"], + }; + expect(hasMissingRequiredFields(undescribed, { ghost: null })).toBe(true); + }); }); diff --git a/clients/web/src/utils/jsonUtils.ts b/clients/web/src/utils/jsonUtils.ts index 98dd905a..2cdb2331 100644 --- a/clients/web/src/utils/jsonUtils.ts +++ b/clients/web/src/utils/jsonUtils.ts @@ -1,3 +1,5 @@ +import { normalizeNullableUnion } from "@inspector/core/json/nullableUnion.js"; + export type JsonValue = | string | number @@ -116,7 +118,12 @@ export function collectSchemaDefaults( ): Record { const properties = schema.properties ?? {}; const result: Record = {}; - for (const [fieldName, fieldSchema] of Object.entries(properties)) { + for (const [fieldName, rawSchema] of Object.entries(properties)) { + // Collapse a nullable union first, for the same reason `SchemaForm` does: + // a nested object's `properties` live on the union's surviving branch, so + // without this the form would *display* a hoisted default that never + // reached the seeded values — the field would submit empty (#1928). + const fieldSchema = normalizeNullableUnion(rawSchema); if (fieldSchema.default !== undefined) { result[fieldName] = fieldSchema.default; } else if (fieldSchema.type === "object" && fieldSchema.properties) { @@ -133,15 +140,29 @@ export function collectSchemaDefaults( * Whether any of the schema's required top-level fields is missing a value in * `values` (absent, null, or empty string). Used to gate a form's submit * action until required fields are supplied. + * + * `null` counts as missing only for a field that does not admit it. JSON + * Schema's `required` constrains *presence*, not content, so a required field + * whose schema is a nullable union is satisfied by an explicit `null` — and + * since #1928 the user can produce exactly that by clearing a nullable enum. + * Treating it as missing would leave the form's submit permanently disabled on + * a value the schema calls valid. */ export function hasMissingRequiredFields( schema: InspectorFormSchema, values: Record, ): boolean { const required = schema.required ?? []; + const properties = schema.properties ?? {}; return required.some((field) => { const value = values[field]; - return value === undefined || value === null || value === ""; + if (value === null) { + const fieldSchema = properties[field]; + return fieldSchema === undefined + ? true + : normalizeNullableUnion(fieldSchema).nullable !== true; + } + return value === undefined || value === ""; }); } diff --git a/core/json/nullableUnion.ts b/core/json/nullableUnion.ts index 4e81b791..2adfdd51 100644 --- a/core/json/nullableUnion.ts +++ b/core/json/nullableUnion.ts @@ -25,7 +25,8 @@ const RENDERABLE_TYPES = [ "object", ] as const; -type RenderableType = (typeof RENDERABLE_TYPES)[number]; +/** A `type` name the collapse is willing to produce. */ +export type RenderableType = (typeof RENDERABLE_TYPES)[number]; function isRenderableType(type: unknown): type is RenderableType { return RENDERABLE_TYPES.includes(type as RenderableType); @@ -51,6 +52,71 @@ function toBranch(value: unknown): Record | null { return value as Record; } +/** + * Whether a branch's `enum` is one a *typeless* branch can be read as a string + * enum. + * + * JSON Schema's `enum` is untyped — `[1, 2]`, `[true, false]`, and `[null]` are + * all legal — so a bare `{ enum: [...] }` does **not** imply strings. Guessing + * `"string"` for a numeric enum would hand non-strings to a renderer that has + * declared them `string[]`: the web `Select` would receive numbers, and the TUI + * would `String(...)` them and submit `"1"` where the server expects `1`. So + * only an all-string enum earns the inference; anything else stays on the + * fallback path, which renders the value honestly as JSON. + */ +function isStringEnum(value: unknown): boolean { + return ( + Array.isArray(value) && + value.length > 0 && + value.every((member) => typeof member === "string") + ); +} + +/** + * The result of a collapse that actually matched. Spelled out rather than + * reusing `T` because the collapse genuinely changes two of `T`'s fields: + * `type` becomes a single renderable name (never an array, never `"null"`), + * and `anyOf` is cleared. Returning `T` for these would let a caller whose `T` + * types `type` as an array go on treating it as one after it has become a + * string, or dereference an `anyOf` that is now `undefined`. + * + * Note this cannot make the *hoist* sound: an `anyOf` branch is `unknown`, so + * any keyword lifted off it is whatever the server sent, however `T` declares + * it. {@link isStringEnum} validates the one keyword the renderers dereference + * as a typed array (`enum`); the rest reach widgets that read them defensively. + */ +export type NormalizedNullableUnion = Omit< + T, + "type" | "anyOf" +> & { + type?: RenderableType; + anyOf?: undefined; + nullable?: boolean; +}; + +/** + * Build the collapsed schema. + * + * The assertion is needed because the spread merges an unresolved generic `T` + * with an `unknown` branch, and `NormalizedNullableUnion`'s `Omit` stays + * deferred while `T` is open — so TS cannot verify the two line up even though + * every field is set right here. Isolated in this one function so the exported + * signature carries the contract and nothing else has to assert it. + */ +function collapsed( + schema: T, + branch: Record | undefined, + type: RenderableType, +): NormalizedNullableUnion { + return { + ...schema, + ...branch, + type, + anyOf: undefined, + nullable: true, + } as NormalizedNullableUnion; +} + /** * Collapse a nullable union — what Zod's `.nullish()` / `.nullable()` and * FastMCP's optional arguments emit — into `{ type: , nullable: true }`. @@ -76,7 +142,7 @@ function toBranch(value: unknown): Record | null { */ export function normalizeNullableUnion( schema: T, -): T { +): T | NormalizedNullableUnion { if (schema.anyOf?.length === 2) { const branches = schema.anyOf.map(toBranch); const nullBranch = branches.find((entry) => entry?.type === "null"); @@ -85,21 +151,13 @@ export function normalizeNullableUnion( ); if (nullBranch && branch) { - // A bare `{ enum: [...] }` branch carries no `type`; JSON Schema allows - // that, and every value such a schema admits here is a string. - const type = branch.type ?? (branch.enum ? "string" : undefined); + // A branch may carry an `enum` and no `type`; JSON Schema allows that, and + // an all-string enum is unambiguously a string field. See isStringEnum for + // why a non-string enum deliberately does not get the same treatment. + const type = + branch.type ?? (isStringEnum(branch.enum) ? "string" : undefined); if (isRenderableType(type)) { - // The result is `schema` with `type` narrowed to a value it already - // admitted, the branch's keywords merged in, and `nullable` added. TS - // cannot express "a spread of T and a subset of T is still a T", and a - // non-generic return would force every caller to cast instead. - return { - ...schema, - ...branch, - type, - anyOf: undefined, - nullable: true, - } as T; + return collapsed(schema, branch, type); } } } @@ -111,8 +169,7 @@ export function normalizeNullableUnion( ) { const type = schema.type.find((member) => member !== "null"); if (isRenderableType(type)) { - // Same reasoning as above. - return { ...schema, type, nullable: true } as T; + return collapsed(schema, undefined, type); } }