fix: address Copilot review round 30 — seed a read-only const for callers that do not
Rendering every `const` read-only assumed the caller had already seeded it, which SchemaForm never did itself: `InlineElicitationRequest` passes its values straight through and gates submit on hasMissingRequiredFields, so a required `const` — a root union's discriminator among them — was displayed and never supplied, leaving Submit disabled with no way to fix it. The form now reports the fixed values upward once per entity, in an effect (the parent's `onChange` cannot be called during our render), adding only names absent from `values` — so a caller that already seeds sees no call at all and the report cannot loop. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
c913168c59
commit
67e7a4ca1a
2 changed files with 62 additions and 0 deletions
|
|
@ -7,6 +7,7 @@ import {
|
||||||
fireEvent,
|
fireEvent,
|
||||||
renderWithMantine,
|
renderWithMantine,
|
||||||
screen,
|
screen,
|
||||||
|
waitFor,
|
||||||
} from "../../../test/renderWithMantine";
|
} from "../../../test/renderWithMantine";
|
||||||
import { SchemaForm } from "./SchemaForm";
|
import { SchemaForm } from "./SchemaForm";
|
||||||
|
|
||||||
|
|
@ -2264,6 +2265,32 @@ describe("SchemaForm multiline strings (#2042)", () => {
|
||||||
expect(screen.getByRole("textbox", { name: /Alpha/ })).toBeTruthy();
|
expect(screen.getByRole("textbox", { name: /Alpha/ })).toBeTruthy();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("reports the branch's fixed values upward when mounted with none", async () => {
|
||||||
|
// A read-only `const` the caller never seeded would otherwise be
|
||||||
|
// displayed and, if required, keep submit disabled forever.
|
||||||
|
const onChange = vi.fn();
|
||||||
|
renderWithMantine(
|
||||||
|
<SchemaForm schema={UNION_SCHEMA} values={{}} onChange={onChange} />,
|
||||||
|
);
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(onChange).toHaveBeenCalledWith({ kind: "email" }),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reports nothing when the caller already seeded them", async () => {
|
||||||
|
const onChange = vi.fn();
|
||||||
|
renderWithMantine(
|
||||||
|
<SchemaForm
|
||||||
|
schema={UNION_SCHEMA}
|
||||||
|
values={{ kind: "email" }}
|
||||||
|
onChange={onChange}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
// Nothing is missing, so the form does not touch the caller's values.
|
||||||
|
await Promise.resolve();
|
||||||
|
expect(onChange).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it("renders no picker for a single-branch union but still shows its fields", () => {
|
it("renders no picker for a single-branch union but still shows its fields", () => {
|
||||||
const schema: InspectorFormSchema = {
|
const schema: InspectorFormSchema = {
|
||||||
type: "object",
|
type: "object",
|
||||||
|
|
|
||||||
|
|
@ -706,6 +706,41 @@ export function SchemaForm({
|
||||||
[],
|
[],
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// Report the schema's own fixed values upward once per entity, for a caller
|
||||||
|
// that mounts the form with nothing seeded.
|
||||||
|
//
|
||||||
|
// A `const` field is rendered read-only, so the user cannot supply it — and a
|
||||||
|
// required one then leaves submit disabled forever, on a value that was never
|
||||||
|
// in doubt. Most callers seed through `collectSchemaDefaults` before mounting;
|
||||||
|
// this covers the ones that do not, rather than making every caller
|
||||||
|
// responsible for a value the form is already displaying.
|
||||||
|
//
|
||||||
|
// An effect, not a render-time update: `onChange` belongs to the parent, and
|
||||||
|
// calling it during our render would update another component mid-render.
|
||||||
|
// Only names absent from `values` are added, so a caller that has already
|
||||||
|
// seeded them sees no call at all, and re-running cannot loop.
|
||||||
|
const latestSeed = useRef({ schema: effectiveSchema, values, onChange });
|
||||||
|
// Written in an effect, not during render — the same shape the validity
|
||||||
|
// reporter below uses, and what `react-hooks/refs` requires.
|
||||||
|
useEffect(() => {
|
||||||
|
latestSeed.current = { schema: effectiveSchema, values, onChange };
|
||||||
|
});
|
||||||
|
useEffect(() => {
|
||||||
|
const {
|
||||||
|
schema: current,
|
||||||
|
values: held,
|
||||||
|
onChange: report,
|
||||||
|
} = latestSeed.current;
|
||||||
|
const missing = Object.entries(collectSchemaDefaults(current)).filter(
|
||||||
|
([name]) => !Object.hasOwn(held, name),
|
||||||
|
);
|
||||||
|
if (missing.length > 0) {
|
||||||
|
report({ ...held, ...Object.fromEntries(missing) });
|
||||||
|
}
|
||||||
|
// Keyed by the entity and branch being edited: a different one has
|
||||||
|
// different fixed values, and the same one needs this only once.
|
||||||
|
}, [draftKey]);
|
||||||
|
|
||||||
// Read through a ref so the callback's identity is not a dependency. It has
|
// Read through a ref so the callback's identity is not a dependency. It has
|
||||||
// to be one or the other, and a *stable* dependency is what this needs: a
|
// to be one or the other, and a *stable* dependency is what this needs: a
|
||||||
// nested form is handed a fresh closure every render, and re-running the
|
// nested form is handed a fresh closure every render, and re-running the
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue