diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx index 0110bf00..bc6b75c9 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx @@ -2,6 +2,7 @@ import { useState } from "react"; import { describe, it, expect, vi } from "vitest"; import userEvent from "@testing-library/user-event"; import type { InspectorFormSchema } from "../../../utils/jsonUtils"; +import { toFormSchema } from "../../../utils/jsonUtils"; import { fireEvent, renderWithMantine, @@ -737,7 +738,13 @@ describe("SchemaForm", () => { expect(lastCall.config).toEqual([1, 2]); }); - it("falls back to passing raw string to onChange when JSON is invalid in JsonInput", async () => { + // The JSON field used to store unparseable text back as the *value*, which + // the next render re-stringified — so each keystroke added a layer of + // escaping (`[` → `"["` → `"\"[\""`). That compounding escape is #1928's + // original symptom, and it lived here rather than in the dispatch. It matters + // more since #2007, whose fix deliberately routes object unions to this + // editor: a fallback nobody can type into is not a fallback. + it("reports no value, not raw text, while the JSON is mid-edit", async () => { const user = userEvent.setup(); const onChange = vi.fn(); const schema: InspectorFormSchema = { @@ -751,9 +758,59 @@ describe("SchemaForm", () => { ); const jsonInput = screen.getByLabelText(/Config/) as HTMLTextAreaElement; await user.type(jsonInput, "x"); - expect(onChange).toHaveBeenCalled(); - const lastCall = onChange.mock.calls[onChange.mock.calls.length - 1][0]; - expect(typeof lastCall.config).toBe("string"); + expect(onChange).toHaveBeenLastCalledWith({ config: undefined }); + }); + + it("lets an array literal be typed one character at a time", async () => { + const user = userEvent.setup(); + const schema: InspectorFormSchema = { + type: "object", + properties: { + config: { type: "array", title: "Config" }, + }, + }; + // Drive the real controlled loop: each onChange feeds straight back in as + // `values`, which is what turned the old handler's raw-text write into a + // compounding re-escape. + function Harness() { + const [values, setValues] = useState>({}); + return ( + + ); + } + renderWithMantine(); + + const jsonInput = screen.getByLabelText(/Config/) as HTMLTextAreaElement; + // `[` is a userEvent keyboard descriptor, so it is escaped as `[[`. + await user.type(jsonInput, '[[1,"a"]'); + + // The box shows exactly what was typed — no injected quotes or backslashes. + expect(jsonInput.value).toBe('[1,"a"]'); + }); + + it("keeps an in-progress draft visible instead of rewriting it", async () => { + const user = userEvent.setup(); + const schema: InspectorFormSchema = { + type: "object", + properties: { + config: { type: "array", title: "Config" }, + }, + }; + function Harness() { + const [values, setValues] = useState>({}); + return ( + + ); + } + renderWithMantine(); + + const jsonInput = screen.getByLabelText(/Config/) as HTMLTextAreaElement; + await user.type(jsonInput, "[[1,"); + // Unparseable so far, and it must survive the re-render untouched. + expect(jsonInput.value).toBe("[1,"); + + await user.type(jsonInput, "2]"); + expect(jsonInput.value).toBe("[1,2]"); }); it("uses default values when value is undefined", () => { @@ -891,18 +948,23 @@ describe("SchemaForm nullable unions", () => { it("renders a Select for a type: [string, null] enum, without a null option", async () => { const user = userEvent.setup(); const onChange = vi.fn(); - const schema: InspectorFormSchema = { + // Built through `toFormSchema`, the same narrowing boundary every + // production call site uses, rather than cast into `InspectorFormSchema`. + // A `null` member is valid JSON Schema but outside that type's `string[]` + // `enum`, and this fixture *is* wire data — so the honest way to introduce + // it is the wire→form narrow, not a double cast that erases the mismatch. + const schema = toFormSchema({ type: "object", properties: { direction: { title: "Direction", type: ["string", "null"], - enum: ["envio", "recebimento", null] as unknown as string[], + enum: ["envio", "recebimento", null], }, }, - }; + }); renderWithMantine( - , + , ); await user.click(screen.getByRole("textbox", { name: "Direction" })); const options = await screen.findAllByRole("option", { hidden: true }); diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index 9cba4bd9..fc64c218 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -146,6 +146,88 @@ function toNumericValue(raw: string | number): number | undefined { return Number.isSafeInteger(Math.trunc(parsed)) ? parsed : undefined; } +/** Parse editor text, reporting `undefined` for anything that is not JSON yet. */ +function parseJsonDraft(text: string): unknown { + if (text.trim() === "") { + return undefined; + } + try { + return JSON.parse(text); + } catch { + return undefined; + } +} + +/** Structural equality for the draft/value re-sync, via canonical JSON. */ +function isSameJson(a: unknown, b: unknown): boolean { + if (a === undefined || b === undefined) { + return a === b; + } + return serializeJson(a) === serializeJson(b); +} + +interface SchemaJsonFieldProps { + label: string; + description?: string; + withAsterisk: boolean; + disabled: boolean; + value: unknown; + onChange: (value: unknown) => void; +} + +/** + * A `JsonInput` that keeps the text the user is typing, not just the value it + * currently parses to. + * + * This field used to drive the box straight off the parent's value and, when + * the text failed `JSON.parse`, store the **raw text** back as the value — which + * the next render re-`JSON.stringify`d, adding a layer of escaping per + * keystroke. Typing `[` gave `"["`, then `"\"[\""`, and the field was unusable + * within a few characters. That compounding escape is the original #1928 + * symptom, and it lived in this fallback rather than in the dispatch. + * + * Routing nullable fields to real widgets removed the common way of *landing* + * here, but it did not fix the editor — and #2007's fix deliberately sends a + * union of object shapes to it, so the editor itself has to be typeable. Hence + * the same split `SchemaNumberInput` uses: the raw text is the source of truth + * for what is *displayed*, while the parent only ever sees parsed JSON. + * + * While the text is mid-edit and does not parse, the parent is told + * `undefined` rather than handed the text. An inspector must not report a value + * it cannot send, and `undefined` is how "no value supplied" is spelled + * everywhere else in this form. + */ +function SchemaJsonField({ + value, + onChange, + ...inputProps +}: SchemaJsonFieldProps) { + const [draft, setDraft] = useState(() => + value === undefined ? "" : serializeJson(value), + ); + + // Re-sync only when the parent's value genuinely diverges from what the draft + // parses to, which leaves an external reset (a cleared form, a loaded + // example) working while an in-progress `[` — whose parse is `undefined`, + // matching the `undefined` we just emitted — is left alone. + useValueChange(value, (next) => { + if (!isSameJson(parseJsonDraft(draft), next)) { + setDraft(next === undefined ? "" : serializeJson(next)); + } + }); + + return ( + { + setDraft(text); + onChange(parseJsonDraft(text)); + }} + /> + ); +} + interface SchemaNumberInputProps { label: string; description?: string; @@ -440,20 +522,16 @@ export function SchemaForm({ // fallback: JsonInput for complex schemas return ( - { - try { - handleFieldChange(fieldName, JSON.parse(val)); - } catch { - handleFieldChange(fieldName, val); - } - }} + value={rawValue} + onChange={(val) => handleFieldChange(fieldName, val)} /> ); } diff --git a/clients/web/src/test/core/nullableUnion.test.ts b/clients/web/src/test/core/nullableUnion.test.ts index 2bf7a1c9..73dba382 100644 --- a/clients/web/src/test/core/nullableUnion.test.ts +++ b/clients/web/src/test/core/nullableUnion.test.ts @@ -277,14 +277,52 @@ describe("normalizeNullableUnion", () => { }); }); - it("drops an enum that held nothing but null", () => { - // No value a dropdown could offer, so the field is better served by its - // plain widget than by an empty select. + // Declining is the point: emitting `enum: undefined` would turn a schema that + // permits *only* `null` into a plain string field accepting arbitrary text, + // inviting values the schema forbids. Left uncollapsed it renders through the + // JSON editor, which represents it honestly. + it("declines to collapse an enum that held nothing but null", () => { + const schema = { type: ["string", "null"], enum: [null] }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); + + it("declines the same way on the anyOf encoding", () => { + const schema = { + anyOf: [{ type: "string", enum: [null] }, { type: "null" as const }], + }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); + + // The parallel `enumNames` is filtered by the same indices. Both renderers + // discard labels outright on a length mismatch, so stripping one without the + // other would silently lose every label, not just the dropped one's. + it("keeps enumNames aligned when stripping the null member", () => { expect( - normalizeNullableUnion({ type: ["string", "null"], enum: [null] }), + normalizeNullableUnion({ + type: ["string", "null"], + enum: ["a", null, "b"], + enumNames: ["Alpha", "None", "Beta"], + }), ).toEqual({ type: "string", - enum: undefined, + enum: ["a", "b"], + enumNames: ["Alpha", "Beta"], + anyOf: undefined, + nullable: true, + }); + }); + + it("leaves a mismatched enumNames alone rather than inventing an alignment", () => { + expect( + normalizeNullableUnion({ + type: ["string", "null"], + enum: ["a", null], + enumNames: ["Alpha"], + }), + ).toEqual({ + type: "string", + enum: ["a"], + enumNames: ["Alpha"], anyOf: undefined, nullable: true, }); diff --git a/core/json/nullableUnion.ts b/core/json/nullableUnion.ts index edcee0c4..1b5c9f9f 100644 --- a/core/json/nullableUnion.ts +++ b/core/json/nullableUnion.ts @@ -102,27 +102,64 @@ export type NormalizedNullableUnion = Omit< }; /** - * Drop a `null` sentinel from an `enum`, since the collapse has already moved - * that fact onto `nullable`. + * What {@link stripNullEnumMembers} concluded about an `enum`. + * + * `"only-null"` is a distinct outcome rather than "an empty list" because it + * has to **stop the collapse**, not just empty a field. See below. + */ +type EnumStrip = + | { kind: "unchanged" } + | { kind: "filtered"; members: unknown[]; names?: unknown } + | { kind: "only-null" }; + +/** + * Drop `null` members from an `enum`, keeping the parallel `enumNames` aligned. * * The `type: [T, "null"]` encoding keeps its keywords at the top level, so a * nullable enum written that way carries the null *inside* the list: - * `{ type: ["string", "null"], enum: ["envio", "recebimento", null] }`. Leaving - * it there breaks both renderers in different ways — the web dispatcher would - * hand `null` to Mantine as option data, and the TUI's all-strings check would - * reject the whole enum and fall back to a plain text field. Neither is the - * dropdown the schema is asking for. + * `{ type: ["string", "null"], enum: ["envio", "recebimento", null] }`. The + * collapse has already moved that fact onto `nullable`, so leaving the sentinel + * in the list breaks both renderers in different ways — the web dispatcher + * would hand `null` to Mantine as option data, and the TUI's all-strings check + * would reject the whole enum and fall back to a plain text field. * - * Returns `undefined` when nothing but `null` was in the list: an enum of only - * `null` permits no value a dropdown could offer, so the field is better served - * by its plain widget than by an empty select. + * **`enumNames` is filtered by the same indices, not left alone.** It is a + * positional parallel array, and both renderers discard labels outright when + * the two lengths disagree — so stripping one without the other silently loses + * every label rather than just the dropped one's. + * + * **An enum of nothing but `null` reports `"only-null"` so the caller declines + * to collapse at all.** Emitting `enum: undefined` instead would turn a schema + * permitting *only* `null` into a plain string field that accepts arbitrary + * text — trading a cosmetic problem for a correctness one, since the form would + * then invite values the schema forbids. Left uncollapsed it renders through + * the JSON editor, which represents it honestly. */ -function stripNullFromEnum(value: unknown): unknown[] | undefined { - if (!Array.isArray(value)) { - return undefined; +function stripNullEnumMembers(members: unknown, names: unknown): EnumStrip { + if (!Array.isArray(members)) { + return { kind: "unchanged" }; } - const members = value.filter((member) => member !== null); - return members.length > 0 ? members : undefined; + const kept = members + .map((member, index) => ({ member, index })) + .filter((entry) => entry.member !== null); + if (kept.length === members.length) { + return { kind: "unchanged" }; + } + if (kept.length === 0) { + return { kind: "only-null" }; + } + // Only realign names that were positionally parallel to begin with; a + // mismatched list is already ignored by both renderers, so re-indexing it + // would invent an alignment the server never declared. + const aligned = + Array.isArray(names) && names.length === members.length + ? kept.map((entry) => names[entry.index]) + : names; + return { + kind: "filtered", + members: kept.map((entry) => entry.member), + names: aligned, + }; } /** @@ -133,23 +170,32 @@ function stripNullFromEnum(value: unknown): unknown[] | undefined { * 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. + * + * Returns `null` when the merged schema turns out to permit only `null`, which + * is not collapsible; see {@link stripNullEnumMembers}. */ function collapsed( schema: T, branch: Record | undefined, type: RenderableType, -): NormalizedNullableUnion { +): NormalizedNullableUnion | null { const merged = { ...schema, ...branch }; + // Read after the merge so the branch's list wins when it has one. + const strip = stripNullEnumMembers( + merged.enum, + (merged as { enumNames?: unknown }).enumNames, + ); + if (strip.kind === "only-null") { + return null; + } return { ...merged, type, anyOf: undefined, nullable: true, - // `enum` is read after the spread so the branch's list wins when it has - // one, and the null sentinel is stripped either way. - ...(merged.enum === undefined - ? {} - : { enum: stripNullFromEnum(merged.enum) }), + ...(strip.kind === "filtered" + ? { enum: strip.members, enumNames: strip.names } + : {}), } as NormalizedNullableUnion; } @@ -233,7 +279,12 @@ export function normalizeNullableUnion( const type = branch.type ?? (isStringEnum(branch.enum) ? "string" : undefined); if (isRenderableType(type)) { - return collapsed(schema, branch, type); + // `null` when the merged enum permits only `null`, which is not + // collapsible — fall through and return the schema by identity. + const result = collapsed(schema, branch, type); + if (result !== null) { + return result; + } } } } @@ -245,7 +296,10 @@ export function normalizeNullableUnion( ) { const type = schema.type.find((member) => member !== "null"); if (isRenderableType(type)) { - return collapsed(schema, undefined, type); + const result = collapsed(schema, undefined, type); + if (result !== null) { + return result; + } } }