diff --git a/src/components/form/Input.stories.tsx b/src/components/form/Input.stories.tsx index eb985f1a..12d60f04 100644 --- a/src/components/form/Input.stories.tsx +++ b/src/components/form/Input.stories.tsx @@ -53,6 +53,8 @@ import { FileInputTrigger } from "./FileInput"; import {FileUploadFileChangeDetails} from "@ark-ui/react"; +import {expect, userEvent, within} from "storybook/test"; +import {ValidationProps} from "./useForm"; export default { title: "Form" @@ -739,6 +741,99 @@ export const File = () => { } +// ---- formValidation prop identity stability ---- +// Measures whether getInputProps(key) keeps referential identity for keys whose value +// did NOT change. Typing happens only in the first input; the counters of the second +// (untouched) input show how often its props were rebuilt anyway. +// Hoisted: with "use no memo" the React Compiler no longer memoizes inline literals, +// and useForm keys its reset effect off the identity of `initialValues`. +const propStabilityInitialValues = { + typed: "", + untouched: "" +} +const propStabilityValidate = { + typed: (value: string) => value ? null : "Required", + untouched: (value: string) => value !== undefined ? null : "Required" +} + +const PropStabilityStats = () => { + "use no memo" // opt out of the React Compiler so the raw getInputProps behavior is measured + + const [inputs, validate] = useForm({ + useInitialValidation: false, + initialValues: propStabilityInitialValues, + validate: propStabilityValidate + }) + + const keys = ["typed", "untouched"] as const + const statsRef = React.useRef>>({ + typed: {props: 0, setValue: 0, validation: 0, value: 0}, + untouched: {props: 0, setValue: 0, validation: 0, value: 0} + }) + const prevRef = React.useRef>>({}) + + const propsByKey: Record> = {} + for (const key of keys) { + const props = inputs.getInputProps(key) + const prev = prevRef.current[key] + if (prev) { + const stats = statsRef.current[key] + if (props !== prev) stats.props++ + if (props.formValidation?.setValue !== prev.formValidation?.setValue) stats.setValue++ + if (props.formValidation?.valid !== prev.formValidation?.valid + || props.formValidation?.notValidMessage !== prev.formValidation?.notValidMessage) stats.validation++ + if (!Object.is(props.initialValue, prev.initialValue)) stats.value++ + } + prevRef.current[key] = props + propsByKey[key] = props + } + + return + validate("typed")} + {...propsByKey.typed} + /> +
+ +
+ {keys.map(key => ( +
+ {key}: + {JSON.stringify(statsRef.current[key])} +
+ ))} +
+} + +export const FormValidationPropStability = { + render: () => , + play: async ({canvasElement}: { canvasElement: HTMLElement }) => { + const canvas = within(canvasElement) + const readStats = (key: string): Record => + JSON.parse(canvas.getByTestId(`stats-${key}`).textContent ?? "{}") + + await userEvent.type(canvas.getByPlaceholderText("typed"), "hello") + + console.log("prop identity changes after typing 5 chars into 'typed':", + JSON.stringify({typed: readStats("typed"), untouched: readStats("untouched")})) + + const untouched = readStats("untouched") + // Sanity check: the untouched input's value really never changed. + expect(untouched.value).toBe(0) + // Desired behavior: props of a key whose value and validation result did not + // change keep their identity. Fails while getInputProps rebuilds them per render. + expect(untouched.validation).toBe(0) + expect(untouched.setValue).toBe(0) + expect(untouched.props).toBe(0) + } +} + // ---- Performance: many EditorInputs in a single form ---- // Stress-tests the Slate-based EditorInput by rendering up to 50 instances at once, // each with token highlighting + suggestions, so we can spot rendering/typing lag. diff --git a/src/components/form/useForm.ts b/src/components/form/useForm.ts index 02e91ef1..88228c07 100644 --- a/src/components/form/useForm.ts +++ b/src/components/form/useForm.ts @@ -37,6 +37,13 @@ export interface IValidation { isValid(): boolean } +interface CachedInputProps { + value: Value | null + message: string | null + required: boolean + props: ValidationProps +} + class Validation implements IValidation { private readonly changeValue: (key: string, value: any) => void @@ -44,19 +51,25 @@ class Validation implements IValidation { private readonly currentValidations?: Validations private readonly shouldValidate: Map private readonly cachedMessages: Map + private readonly cachedSetters: Map void> + private readonly cachedProps: Map> constructor( changeValue: (key: string, value: any) => void, values: Values, validations: Validations, shouldValidate: Map = new Map(), - cachedMessages: Map = new Map() + cachedMessages: Map = new Map(), + cachedSetters: Map void> = new Map(), + cachedProps: Map> = new Map() ) { this.changeValue = changeValue this.currentValues = values this.currentValidations = validations this.shouldValidate = shouldValidate this.cachedMessages = cachedMessages + this.cachedSetters = cachedSetters + this.cachedProps = cachedProps } isValid(): boolean { @@ -94,23 +107,43 @@ class Validation implements IValidation { message = this.cachedMessages.get(key) ?? null } - return { + const required = Boolean(this.currentValidations && this.currentValidations[key]) + + // Reuse the cached props object as long as nothing for this key changed, so + // consumers can memoize directly on the getInputProps output. + const cached = this.cachedProps.get(key) + if (cached && Object.is(cached.value, currentValue) && cached.message === message && cached.required === required) { + return cached.props + } + + // One setValue per key for the lifetime of the form: changeValue reads from + // valuesRef and never goes stale, so the wrapper never has to be rebuilt. + let setValue = this.cachedSetters.get(key) + if (!setValue) { + const changeValue = this.changeValue + setValue = (value: any) => { + changeValue(currentName, value) + } + this.cachedSetters.set(key, setValue) + } + + const props: ValidationProps = { // @ts-ignore – z.B. wenn dein Input `defaultValue` kennt defaultValue: currentValue ?? undefined, initialValue: currentValue ?? undefined, formValidation: { - setValue: (value: any) => { - this.changeValue(currentName, value) - }, + setValue, ...({ notValidMessage: message, valid: message === null, }) }, - ...(this.currentValidations && this.currentValidations[key] + ...(required ? {required: true} : {}) } + this.cachedProps.set(key, {value: currentValue, message, required, props}) + return props } } @@ -129,6 +162,8 @@ export const useForm = < const initValues = React.useMemo(() => initialValues as Values, [initialValues]) const [values, setValues] = useState(initValues) const cachedMessagesRef = useRef>(new Map()) + const cachedSettersRef = useRef void>>(new Map()) + const cachedPropsRef = useRef>>(new Map()) const valuesRef = useRef(values) valuesRef.current = values @@ -142,18 +177,30 @@ export const useForm = < values, validate, useInitialValidation ? new Map(Object.keys(initValues).map(k => [k as keyof Values, true])) : new Map(), - cachedMessagesRef.current + cachedMessagesRef.current, + cachedSettersRef.current, + cachedPropsRef.current )) + const didInitRef = useRef(false) useEffect(() => { valuesRef.current = initValues setValues(initValues) + // Form reset: props must be rebuilt from the new initial values. The setter + // cache survives on purpose — changeValue is stable, so the setters stay valid. + // On mount the cache only holds props built from these initValues, so keep it. + if (didInitRef.current) { + cachedPropsRef.current.clear() + } + didInitRef.current = true setValidation(new Validation( changeValue, initValues, validate, useInitialValidation ? new Map(Object.keys(initValues).map(k => [k as keyof Values, true])) : new Map(), - cachedMessagesRef.current + cachedMessagesRef.current, + cachedSettersRef.current, + cachedPropsRef.current )) }, [initValues]) @@ -170,7 +217,9 @@ export const useForm = < currentValues, validate, shouldValidateMap, - cachedMessagesRef.current + cachedMessagesRef.current, + cachedSettersRef.current, + cachedPropsRef.current ) setValidation(currentValidation) diff --git a/vite.config.js b/vite.config.js index f4b11e53..7afe1e97 100644 --- a/vite.config.js +++ b/vite.config.js @@ -57,6 +57,9 @@ export default defineConfig({ storybookTest({ configDir: path.join(dirname, '.storybook') })], + optimizeDeps: { + include: ['storybook/test'] + }, test: { name: 'storybook', browser: {