mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
refactor(ui): port the create key form off antd Form onto react-hook-form (#37442)
* refactor(ui): port the create key form off antd Form onto react-hook-form antd hands onFinish exactly the fields mounted at submit time, so a collapsed section contributes nothing to the request while the values typed into it survive for re-expansion. react-hook-form reaches only one of those two behaviours per shouldUnregister setting, so the store is kept intact and projected down to the mounted set through an explicit mount registry. MountedFormField carries the rest of the Form.Item contract the payload depends on: defaults taken from each field's own declaration rather than a blanket empty value, and help text that replaces the rule message instead of sitting beside it. The 60-case submit differential runs unedited against the port, joined by cases for the writers outside the submit path, mounted-set validation, Enter to submit, and switch coercion. * docs(ui): state the mounted projection's static-name limit at its export The registry counts by name and the projection emits flat keys, so a Form.List row and its per-row sub-fields, whose names are generated at runtime, are never in the mounted set and go missing from the payload. That is silent and it is correct for every static field around it, so the contract belongs where the next consumer reads it. * refactor(ui): cut the mounted field's explanatory comments to the contract limit The mechanism the projection uses and the reason a helped field hides its rule message are both derivable from the code, so they belong in the pull request rather than in two places. What survives is the one thing no reader can derive: that a runtime-generated name is silently absent from the payload. * refactor(ui): drop the doc comment from MountedFormField The static-name constraint it described moves to the PR description, where it is not a second place to keep in sync with the code.
This commit is contained in:
parent
61029814ab
commit
963c7fb0d4
5 changed files with 1458 additions and 1063 deletions
|
|
@ -2332,9 +2332,6 @@
|
|||
"local/filename-pascal-case": {
|
||||
"count": 1
|
||||
},
|
||||
"local/no-complex-jsx-arrow": {
|
||||
"count": 2
|
||||
},
|
||||
"max-lines": {
|
||||
"count": 1
|
||||
},
|
||||
|
|
|
|||
|
|
@ -0,0 +1,146 @@
|
|||
"use client";
|
||||
|
||||
import * as React from "react";
|
||||
import {
|
||||
Controller,
|
||||
type Control,
|
||||
type ControllerProps,
|
||||
type RegisterOptions,
|
||||
type UseFormGetValues,
|
||||
} from "react-hook-form";
|
||||
|
||||
import { Field, FieldDescription, FieldError, FieldLabel } from "@/components/shared/form/field";
|
||||
|
||||
export type MountedFormValues = Record<string, unknown>;
|
||||
|
||||
export interface MountRegistry {
|
||||
readonly register: (name: string) => () => void;
|
||||
readonly mountedNames: () => readonly string[];
|
||||
}
|
||||
|
||||
export interface MountedFormContextValue {
|
||||
readonly control: Control<MountedFormValues>;
|
||||
readonly registry: MountRegistry;
|
||||
}
|
||||
|
||||
const missingProvider = (): never => {
|
||||
throw new Error("MountedFormField requires a MountedFormProvider ancestor");
|
||||
};
|
||||
|
||||
const MountedFormContext = React.createContext<MountedFormContextValue>({
|
||||
get control(): Control<MountedFormValues> {
|
||||
return missingProvider();
|
||||
},
|
||||
registry: {
|
||||
register: missingProvider,
|
||||
mountedNames: missingProvider,
|
||||
},
|
||||
});
|
||||
|
||||
export const MountedFormProvider = MountedFormContext.Provider;
|
||||
|
||||
export const useMountRegistry = (): MountRegistry => {
|
||||
const counts = React.useRef<Map<string, number>>(new Map());
|
||||
return React.useMemo(
|
||||
() => ({
|
||||
register: (name: string) => {
|
||||
counts.current.set(name, (counts.current.get(name) ?? 0) + 1);
|
||||
return () => {
|
||||
const remaining = (counts.current.get(name) ?? 0) - 1;
|
||||
if (remaining > 0) {
|
||||
counts.current.set(name, remaining);
|
||||
} else {
|
||||
counts.current.delete(name);
|
||||
}
|
||||
};
|
||||
},
|
||||
mountedNames: () => Array.from(counts.current.keys()),
|
||||
}),
|
||||
[],
|
||||
);
|
||||
};
|
||||
|
||||
export const projectMountedValues = (
|
||||
registry: MountRegistry,
|
||||
getValues: UseFormGetValues<MountedFormValues>,
|
||||
): MountedFormValues => {
|
||||
const names = [...registry.mountedNames()];
|
||||
const values = getValues(names);
|
||||
return Object.fromEntries(names.map((name, index) => [name, values[index]]));
|
||||
};
|
||||
|
||||
export type MountedFieldControlProps = {
|
||||
readonly id: string;
|
||||
readonly name: string;
|
||||
readonly value: unknown;
|
||||
readonly onChange: (...event: unknown[]) => void;
|
||||
readonly onBlur: () => void;
|
||||
readonly "aria-required": "true" | undefined;
|
||||
readonly "aria-invalid": "true" | undefined;
|
||||
readonly "aria-describedby": string | undefined;
|
||||
};
|
||||
|
||||
export interface MountedFormFieldProps {
|
||||
readonly name: string;
|
||||
readonly label?: React.ReactNode;
|
||||
readonly help?: React.ReactNode;
|
||||
readonly required?: boolean;
|
||||
readonly rules?: Omit<
|
||||
RegisterOptions<MountedFormValues, string>,
|
||||
"valueAsNumber" | "valueAsDate" | "setValueAs" | "disabled"
|
||||
>;
|
||||
readonly defaultValue?: unknown;
|
||||
readonly bare?: boolean;
|
||||
readonly className?: string;
|
||||
readonly children: (control: MountedFieldControlProps) => React.ReactNode;
|
||||
}
|
||||
|
||||
export const MountedFormField: React.FC<MountedFormFieldProps> = ({
|
||||
name,
|
||||
label,
|
||||
help,
|
||||
required,
|
||||
rules,
|
||||
defaultValue,
|
||||
bare,
|
||||
className,
|
||||
children,
|
||||
}) => {
|
||||
const { control, registry } = React.useContext(MountedFormContext);
|
||||
React.useEffect(() => registry.register(name), [registry, name]);
|
||||
|
||||
const helpId = `${name}_help`;
|
||||
const hasHelp = help !== undefined && help !== null;
|
||||
|
||||
const renderField: ControllerProps<MountedFormValues, string>["render"] = ({ field, fieldState }) => {
|
||||
const invalid = fieldState.error !== undefined;
|
||||
const controlProps: MountedFieldControlProps = {
|
||||
id: name,
|
||||
name: field.name,
|
||||
value: field.value,
|
||||
onChange: field.onChange,
|
||||
onBlur: field.onBlur,
|
||||
"aria-required": required ? "true" : undefined,
|
||||
"aria-invalid": invalid ? "true" : undefined,
|
||||
"aria-describedby": hasHelp || invalid ? helpId : undefined,
|
||||
};
|
||||
|
||||
if (bare) {
|
||||
return <>{children(controlProps)}</>;
|
||||
}
|
||||
|
||||
return (
|
||||
<Field data-invalid={invalid || undefined} className={className}>
|
||||
{label !== undefined && <FieldLabel htmlFor={name}>{label}</FieldLabel>}
|
||||
{children(controlProps)}
|
||||
{hasHelp ? (
|
||||
<FieldDescription id={helpId}>{help}</FieldDescription>
|
||||
) : (
|
||||
<FieldError id={helpId} errors={[fieldState.error]} />
|
||||
)}
|
||||
</Field>
|
||||
);
|
||||
};
|
||||
|
||||
return <Controller control={control} name={name} rules={rules} defaultValue={defaultValue} render={renderField} />;
|
||||
};
|
||||
|
|
@ -1,10 +1,12 @@
|
|||
import React, { useState, useEffect } from "react";
|
||||
import { Form, Input as AntdInput, InputNumber, Select } from "antd";
|
||||
import { Input as AntdInput, InputNumber, Select } from "antd";
|
||||
import { Input } from "@/components/ui/input";
|
||||
import { InfoCircleOutlined } from "@ant-design/icons";
|
||||
import { Tooltip } from "antd";
|
||||
import type { UseFormSetValue } from "react-hook-form";
|
||||
import { getOpenAPISchema } from "../networking";
|
||||
import { formatLabel } from "@/utils/textUtils";
|
||||
import { MountedFormField, type MountedFormValues } from "./MountedFormField";
|
||||
|
||||
interface SchemaProperty {
|
||||
type?: string;
|
||||
|
|
@ -25,13 +27,13 @@ interface OpenAPISchema {
|
|||
interface SchemaFormFieldsProps {
|
||||
schemaComponent: string;
|
||||
excludedFields?: string[];
|
||||
form: any;
|
||||
setValue: UseFormSetValue<MountedFormValues>;
|
||||
overrideLabels?: { [key: string]: string };
|
||||
overrideTooltips?: { [key: string]: string };
|
||||
customValidation?: {
|
||||
[key: string]: (rule: any, value: any) => Promise<void>;
|
||||
[key: string]: (rule: unknown, value: unknown) => Promise<void>;
|
||||
};
|
||||
defaultValues?: { [key: string]: any };
|
||||
defaultValues?: { [key: string]: unknown };
|
||||
}
|
||||
|
||||
// Define which fields should be parsed as JSON
|
||||
|
|
@ -53,6 +55,10 @@ const validateJSON = (value: string): boolean => {
|
|||
}
|
||||
};
|
||||
|
||||
const isBlank = (value: unknown): boolean => value === undefined || value === null || value === "";
|
||||
|
||||
const messageOf = (error: unknown): string => (error instanceof Error ? error.message : String(error));
|
||||
|
||||
const getFieldHelp = (key: string, property: SchemaProperty, type: string): string => {
|
||||
// Default help text based on type
|
||||
const defaultHelp =
|
||||
|
|
@ -99,7 +105,7 @@ const getFieldHelp = (key: string, property: SchemaProperty, type: string): stri
|
|||
const SchemaFormFields: React.FC<SchemaFormFieldsProps> = ({
|
||||
schemaComponent,
|
||||
excludedFields = [],
|
||||
form,
|
||||
setValue,
|
||||
overrideLabels = {},
|
||||
overrideTooltips = {},
|
||||
customValidation = {},
|
||||
|
|
@ -120,14 +126,11 @@ const SchemaFormFields: React.FC<SchemaFormFieldsProps> = ({
|
|||
|
||||
setSchemaProperties(componentSchema);
|
||||
|
||||
const defaultFormValues: { [key: string]: any } = {};
|
||||
Object.keys(componentSchema.properties)
|
||||
.filter((key) => !excludedFields.includes(key) && defaultValues[key] !== undefined)
|
||||
.forEach((key) => {
|
||||
defaultFormValues[key] = defaultValues[key];
|
||||
setValue(key, defaultValues[key]);
|
||||
});
|
||||
|
||||
form.setFieldsValue(defaultFormValues);
|
||||
} catch (error) {
|
||||
console.error("Schema fetch error:", error);
|
||||
setError(error instanceof Error ? error.message : "Failed to fetch schema");
|
||||
|
|
@ -135,7 +138,7 @@ const SchemaFormFields: React.FC<SchemaFormFieldsProps> = ({
|
|||
};
|
||||
|
||||
fetchOpenAPISchema();
|
||||
}, [schemaComponent, form, excludedFields]);
|
||||
}, [schemaComponent, setValue, excludedFields]);
|
||||
|
||||
const getPropertyType = (property: SchemaProperty): string => {
|
||||
if (property.type) {
|
||||
|
|
@ -156,22 +159,25 @@ const SchemaFormFields: React.FC<SchemaFormFieldsProps> = ({
|
|||
const label = overrideLabels[key] || property.title || formatLabel(key);
|
||||
const tooltip = overrideTooltips[key] || property.description;
|
||||
|
||||
const rules = [];
|
||||
if (isRequired) {
|
||||
rules.push({ required: true, message: `${label} is required` });
|
||||
}
|
||||
if (customValidation[key]) {
|
||||
rules.push({ validator: customValidation[key] });
|
||||
}
|
||||
if (isJSONField(key, property)) {
|
||||
rules.push({
|
||||
validator: async (_: any, value: string) => {
|
||||
if (value && !validateJSON(value)) {
|
||||
throw new Error("Please enter valid JSON");
|
||||
const validate = {
|
||||
...(isRequired && {
|
||||
required: (value: unknown) => (isBlank(value) ? `${label} is required` : true),
|
||||
}),
|
||||
...(customValidation[key] && {
|
||||
custom: async (value: unknown) => {
|
||||
try {
|
||||
await customValidation[key](null, value);
|
||||
return true;
|
||||
} catch (thrown) {
|
||||
return messageOf(thrown);
|
||||
}
|
||||
},
|
||||
});
|
||||
}
|
||||
}),
|
||||
...(isJSONField(key, property) && {
|
||||
json: (value: unknown) =>
|
||||
value && !validateJSON(value as string) ? "Please enter valid JSON" : (true as const),
|
||||
}),
|
||||
};
|
||||
|
||||
const formLabel = tooltip ? (
|
||||
<span>
|
||||
|
|
@ -184,44 +190,63 @@ const SchemaFormFields: React.FC<SchemaFormFieldsProps> = ({
|
|||
label
|
||||
);
|
||||
|
||||
let inputComponent;
|
||||
if (isJSONField(key, property)) {
|
||||
inputComponent = <AntdInput.TextArea rows={4} placeholder="Enter as JSON" className="font-mono" />;
|
||||
} else if (property.enum) {
|
||||
inputComponent = (
|
||||
<Select>
|
||||
{property.enum.map((value) => (
|
||||
<Select.Option key={value} value={value}>
|
||||
{value}
|
||||
</Select.Option>
|
||||
))}
|
||||
</Select>
|
||||
);
|
||||
} else if (type === "number" || type === "integer") {
|
||||
inputComponent = <InputNumber style={{ width: "100%" }} precision={type === "integer" ? 0 : undefined} />;
|
||||
} else if (key === "duration") {
|
||||
inputComponent = <Input placeholder="eg: 30s, 30h, 30d" />;
|
||||
} else {
|
||||
inputComponent = <Input placeholder={tooltip || ""} />;
|
||||
}
|
||||
|
||||
return (
|
||||
<Form.Item
|
||||
<MountedFormField
|
||||
key={key}
|
||||
label={formLabel}
|
||||
name={key}
|
||||
className="mt-8"
|
||||
rules={rules}
|
||||
initialValue={defaultValues[key]}
|
||||
help={<div className="text-xs text-gray-500">{getFieldHelp(key, property, type)}</div>}
|
||||
required={isRequired}
|
||||
rules={Object.keys(validate).length > 0 ? { validate } : undefined}
|
||||
defaultValue={defaultValues[key]}
|
||||
help={<div className="text-xs text-muted-foreground">{getFieldHelp(key, property, type)}</div>}
|
||||
>
|
||||
{inputComponent}
|
||||
</Form.Item>
|
||||
{(control) => {
|
||||
if (isJSONField(key, property)) {
|
||||
return (
|
||||
<AntdInput.TextArea
|
||||
{...control}
|
||||
value={control.value as string | undefined}
|
||||
rows={4}
|
||||
placeholder="Enter as JSON"
|
||||
className="font-mono"
|
||||
/>
|
||||
);
|
||||
}
|
||||
if (property.enum) {
|
||||
return (
|
||||
<Select {...control} value={control.value as string | undefined}>
|
||||
{property.enum.map((value) => (
|
||||
<Select.Option key={value} value={value}>
|
||||
{value}
|
||||
</Select.Option>
|
||||
))}
|
||||
</Select>
|
||||
);
|
||||
}
|
||||
if (type === "number" || type === "integer") {
|
||||
return (
|
||||
<InputNumber
|
||||
{...control}
|
||||
value={control.value as number | undefined}
|
||||
style={{ width: "100%" }}
|
||||
precision={type === "integer" ? 0 : undefined}
|
||||
/>
|
||||
);
|
||||
}
|
||||
if (key === "duration") {
|
||||
return (
|
||||
<Input {...control} value={(control.value as string | undefined) ?? ""} placeholder="eg: 30s, 30h, 30d" />
|
||||
);
|
||||
}
|
||||
return <Input {...control} value={(control.value as string | undefined) ?? ""} placeholder={tooltip || ""} />;
|
||||
}}
|
||||
</MountedFormField>
|
||||
);
|
||||
};
|
||||
|
||||
if (error) {
|
||||
return <div className="text-red-500">Error: {error}</div>;
|
||||
return <div className="text-destructive">Error: {error}</div>;
|
||||
}
|
||||
|
||||
if (!schemaProperties?.properties) {
|
||||
|
|
|
|||
|
|
@ -875,4 +875,70 @@ describe("CreateKey", () => {
|
|||
expect(screen.queryByRole("button", { name: /Optional Settings/i })).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
describe("writers outside the submit path", () => {
|
||||
it("lets the selected user win over the search text typed into the same field", async () => {
|
||||
vi.mocked(userFilterUICall).mockResolvedValue([
|
||||
{ user_id: "u-77", user_email: "alice@example.com" },
|
||||
] as unknown as Awaited<ReturnType<typeof userFilterUICall>>);
|
||||
|
||||
await openModal();
|
||||
await userEvent.click(screen.getByRole("radio", { name: "Another User" }));
|
||||
await nameTheKey();
|
||||
|
||||
await userEvent.type(antdSearchInput(await screen.findByText("Type email to search for users")), "alice");
|
||||
await userEvent.click(await screen.findByText("alice@example.com (u-77)"));
|
||||
await submit();
|
||||
|
||||
expect((await createdPayload()).user_id).toBe("u-77");
|
||||
});
|
||||
|
||||
it("surfaces the required message on a field that carries no help text", async () => {
|
||||
await openModal();
|
||||
await userEvent.click(screen.getByRole("radio", { name: "Another User" }));
|
||||
await nameTheKey();
|
||||
await submit();
|
||||
|
||||
expect(
|
||||
await screen.findByText("Please input the user ID of the user you are assigning the key to"),
|
||||
).toBeInTheDocument();
|
||||
expect(vi.mocked(keyCreateCall)).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("validation follows the mounted set", () => {
|
||||
it("submits an over-ceiling budget typed into a section the user closed again, omitting the key", async () => {
|
||||
await openModal({ team: { team_id: "team-1", max_budget: 10 } as unknown as Team });
|
||||
await nameTheKey();
|
||||
await openSection(/Optional Settings/i);
|
||||
await userEvent.type(await screen.findByLabelText(/Max Budget \(USD\)/), "50");
|
||||
await openSection(/Optional Settings/i);
|
||||
await submit();
|
||||
|
||||
const payload = await createdPayload();
|
||||
expect(payload).not.toHaveProperty("max_budget");
|
||||
expect(payload.key_alias).toBe("contract-key");
|
||||
});
|
||||
});
|
||||
|
||||
describe("submit gestures", () => {
|
||||
it("creates the key when Enter is pressed inside a text field", async () => {
|
||||
await openModal();
|
||||
await userEvent.type(await screen.findByLabelText(/Key Name/), "enter-key{Enter}");
|
||||
|
||||
expect((await createdPayload()).key_alias).toBe("enter-key");
|
||||
});
|
||||
});
|
||||
|
||||
describe("switch coercion", () => {
|
||||
it("sends enable_prompt_caching as a boolean once the switch is on", async () => {
|
||||
await openModal();
|
||||
await nameTheKey();
|
||||
await openSection(/Optional Settings/i);
|
||||
await userEvent.click(await screen.findByLabelText("Enable Prompt Caching"));
|
||||
await submit();
|
||||
|
||||
expect((await createdPayload()).enable_prompt_caching).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load diff
Loading…
Add table
Reference in a new issue