Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 139 additions & 0 deletions clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1648,3 +1648,142 @@ describe("JSON editor escaping (#1853, #1856, #1885)", () => {
expect(jsonInput.value).not.toContain("\\");
});
});

// A schema `default` is what a field *opens* with, not a value re-imposed on
// every unsendable draft. Unsendable text reports `undefined` by design, and
// `resolveValue` used to turn that straight back into the default — a change
// from the previous value, so the draft/value re-sync rewrote the box and
// reverted the keystroke.
//
// The two halves differed in how reachable they were, which the tests below
// mirror deliberately:
//
// - **The number field reverted in the app.** Numbers compare by value, so
// clearing a defaulted box (value `3` → resolved default `30`) always fired
// the re-sync and refilled itself.
// - **The JSON field was latent.** `collectSchemaDefaults` assigns
// `fieldSchema.default` itself, so a field seeded from the schema holds the
// *same reference* the substitution returns and nothing fires. It needs a
// structurally-equal but distinct value object to become observable — which
// is what parsed wire/deep-link values and rebuilt nested-object defaults
// produce, and what these tests construct.
describe("SchemaForm defaulted fields (#2026)", () => {
function DefaultHarness({
schema,
initial = {},
}: {
schema: InspectorFormSchema;
initial?: Record<string, unknown>;
}) {
const [values, setValues] = useState<Record<string, unknown>>(initial);
return <SchemaForm schema={schema} values={values} onChange={setValues} />;
}

const arrayWithDefault: InspectorFormSchema = {
type: "object",
properties: { tags: { type: "array", title: "Tags", default: ["a"] } },
};

it("opens the JSON editor on the schema default", () => {
renderWithMantine(<DefaultHarness schema={arrayWithDefault} />);
expect((screen.getByLabelText(/Tags/) as HTMLTextAreaElement).value).toBe(
'[\n "a"\n]',
);
});

it("keeps a keystroke typed into a defaulted JSON field", async () => {
const user = userEvent.setup();
// Seeded with a value that is equal to the schema default but is not the
// same object — how values parsed off the wire or out of a deep link
// arrive. Sharing the reference (what `collectSchemaDefaults` produces) is
// what keeps this latent rather than what makes it safe.
renderWithMantine(
<DefaultHarness schema={arrayWithDefault} initial={{ tags: ["a"] }} />,
);

const jsonInput = screen.getByLabelText(/Tags/) as HTMLTextAreaElement;
await user.click(jsonInput);
await user.keyboard("{End}x");

// The invalid draft is the user's, not the default reasserting itself.
expect(jsonInput.value).toBe('[\n "a"\n]x');
});

it("lets a defaulted JSON field be edited to a new value", async () => {
const user = userEvent.setup();
// Distinct-object seed again, for the reason above.
renderWithMantine(
<DefaultHarness schema={arrayWithDefault} initial={{ tags: ["a"] }} />,
);

const jsonInput = screen.getByLabelText(/Tags/) as HTMLTextAreaElement;
await user.clear(jsonInput);
await user.type(jsonInput, '[["b"]');

expect(jsonInput.value).toBe('["b"]');
expect(screen.queryByText(/Not valid JSON/)).not.toBeInTheDocument();
});

it("lets a defaulted number field be emptied", async () => {
const user = userEvent.setup();
const schema: InspectorFormSchema = {
type: "object",
properties: { n: { type: "integer", title: "N", default: 30 } },
};
renderWithMantine(<DefaultHarness schema={schema} initial={{ n: 30 }} />);

const n = screen.getByLabelText(/N/) as HTMLInputElement;
expect(n.value).toBe("30");

await user.click(n);
await user.keyboard("{End}{Backspace}{Backspace}");

// The box stays empty instead of refilling itself with the default.
expect(n.value).toBe("");
});

// An explicit `null` is a value, not an absent one, so a non-null default
// must not be substituted for it. Reachable for a nullable number field the
// user cleared, or one the server sent as null.
it("shows an explicit null as empty, not as the default", () => {
const schema: InspectorFormSchema = {
type: "object",
properties: {
n: {
title: "N",
anyOf: [{ type: "integer" }, { type: "null" }],
default: 30,
},
},
};
renderWithMantine(<DefaultHarness schema={schema} initial={{ n: null }} />);

expect((screen.getByLabelText(/^N$/) as HTMLInputElement).value).toBe("");
});

it("still re-syncs a defaulted field when the value changes externally", async () => {
function ExternalHarness() {
const [values, setValues] = useState<Record<string, unknown>>({
tags: ["a"],
});
return (
<>
<SchemaForm
schema={arrayWithDefault}
values={values}
onChange={setValues}
/>
<button type="button" onClick={() => setValues({ tags: ["z"] })}>
load example
</button>
</>
);
}
const user = userEvent.setup();
renderWithMantine(<ExternalHarness />);

const jsonInput = screen.getByLabelText(/Tags/) as HTMLTextAreaElement;
await user.click(screen.getByRole("button", { name: "load example" }));
expect(jsonInput.value).toBe('[\n "z"\n]');
});
});
59 changes: 53 additions & 6 deletions clients/web/src/components/groups/SchemaForm/SchemaForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,14 @@ interface SchemaJsonFieldProps {
description?: string;
withAsterisk: boolean;
disabled: boolean;
/**
* The field's own entry in the form's values — **not** run through
* `resolveValue`. A draft-holding field is handed the raw value and the
* schema default separately; see `defaultValue` for why.
*/
value: unknown;
/** The schema `default`, used to seed the draft only. See `defaultValue`. */
defaultValue: unknown;
onChange: (value: unknown) => void;
onValidityChange: DraftValidityReporter;
}
Expand Down Expand Up @@ -244,18 +251,39 @@ interface SchemaJsonFieldProps {
function SchemaJsonField({
fieldName,
value,
defaultValue,
onChange,
onValidityChange,
...inputProps
}: SchemaJsonFieldProps) {
const [draft, setDraft] = useState(() =>
value === undefined ? "" : serializeJson(value),
);
const [draft, setDraft] = useState(() => {
const initial = value === undefined ? defaultValue : value;
return initial === undefined ? "" : serializeJson(initial);
});

// 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.
//
// The value compared here is the field's *raw* entry, never `resolveValue`'s
// default substitution (#2026). Unsendable text reports `undefined`, and a
// defaulted field resolves that straight back to its default — a change from
// the previous value, so comparing against the resolved one would rewrite the
// draft to the default and revert the keystroke. Almost every edit passes
// through an unsendable state, so a defaulted array or object argument would
// be uneditable.
//
// Would be, not was: this is latent under today's seeding, where
// `collectSchemaDefaults` assigns `fieldSchema.default` itself, so the
// substitution returns the *same reference* and `useValueChange` never fires.
// It goes live as soon as the value is a structurally-equal but distinct
// object — parsed from the wire or a deep link, or a nested-object default,
// which `collectSchemaDefaults` rebuilds per call. `SchemaNumberInput` has no
// such accidental protection, since a number compares by value.
//
// Either way the default belongs to what the field *opens* with, seeded
// above, not to what is re-imposed while it is being typed into.
useValueChange(value, (next) => {
if (!isSameJson(parseJsonDraft(draft), next)) {
setDraft(next === undefined ? "" : serializeJson(next));
Expand Down Expand Up @@ -309,7 +337,10 @@ interface SchemaNumberInputProps {
description?: string;
withAsterisk: boolean;
disabled: boolean;
/** Raw, not `resolveValue`-substituted — see `SchemaJsonFieldProps.value`. */
value: number | undefined;
/** The schema `default`, used to seed the draft only (#2026). */
defaultValue: number | undefined;
min?: number;
max?: number;
allowDecimal: boolean;
Expand Down Expand Up @@ -343,12 +374,24 @@ interface SchemaNumberInputProps {
function SchemaNumberInput({
fieldName,
value,
defaultValue,
onChange,
onValidityChange,
...inputProps
}: SchemaNumberInputProps) {
const [draft, setDraft] = useState<string | number>(value ?? "");
// `undefined` means "no value supplied", which is what the default is for.
// An explicit `null` is a value — a nullable number the user cleared, or one
// the server sent — so it must not be overwritten by a non-null default; it
// displays as the empty box `resolveValue` produced before. Hence the
// `undefined` test rather than `??`, matching `SchemaJsonField`.
const [draft, setDraft] = useState<string | number>(
(value === undefined ? defaultValue : value) ?? "",
);

// Compared against the raw value, never the default-substituted one, for the
// reason spelled out on `SchemaJsonField` (#2026): clearing this box reports
// no value, which a defaulted field resolved straight back to its default —
// so the box refilled itself and the argument could not be emptied.
useValueChange(value, (next) => {
if (!Object.is(toNumericValue(draft), next)) {
setDraft(next ?? "");
Expand Down Expand Up @@ -584,7 +627,9 @@ export function SchemaForm({
description={description}
withAsterisk={isRequired}
disabled={disabled}
value={rawValue as number | undefined}
// Raw and default kept apart, not pre-resolved (#2026).
value={values[fieldName] as number | undefined}
defaultValue={getDefaultValue(fieldSchema) as number | undefined}
min={fieldSchema.minimum}
max={fieldSchema.maximum}
// An `integer` field rejects the decimal point outright rather than
Expand Down Expand Up @@ -692,7 +737,9 @@ export function SchemaForm({
description={description}
withAsterisk={isRequired}
disabled={disabled}
value={rawValue}
// Raw and default kept apart, not pre-resolved (#2026).
value={values[fieldName]}
defaultValue={getDefaultValue(fieldSchema)}
onChange={(val) => handleFieldChange(fieldName, val)}
onValidityChange={reportFieldValidity}
/>
Expand Down