Merge pull request #727 from code0-tech/feat/#724

useForm: getInputProps returns unstable references

Author: nicosammito

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<Record<string, Record<string, number>>>({
+        typed: {props: 0, setValue: 0, validation: 0, value: 0},
+        untouched: {props: 0, setValue: 0, validation: 0, value: 0}
+    })
+    const prevRef = React.useRef<Record<string, ValidationProps<any>>>({})
+
+    const propsByKey: Record<string, ValidationProps<any>> = {}
+    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 <Card color={"secondary"} w={"400px"}>
+        <TextInput
+            placeholder={"typed"}
+            title={"Typed input"}
+            onChange={() => validate("typed")}
+            {...propsByKey.typed}
+        />
+        <br/>
+        <TextInput
+            placeholder={"untouched"}
+            title={"Untouched input"}
+            {...propsByKey.untouched}
+        />
+        <br/>
+        {keys.map(key => (
+            <div key={key}>
+                <Text size={"sm"}>{key}: </Text>
+                <span data-testid={`stats-${key}`}>{JSON.stringify(statsRef.current[key])}</span>
+            </div>
+        ))}
+    </Card>
+}
+
+export const FormValidationPropStability = {
+    render: () => <PropStabilityStats/>,
+    play: async ({canvasElement}: { canvasElement: HTMLElement }) => {
+        const canvas = within(canvasElement)
+        const readStats = (key: string): Record<string, number> =>
+            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.

src/components/form/useForm.ts

@@ -37,26 +37,39 @@ export interface IValidation<Values> {
     isValid(): boolean
 }
 
+interface CachedInputProps<Value> {
+    value: Value | null
+    message: string | null
+    required: boolean
+    props: ValidationProps<Value>
+}
+
 class Validation<Values> implements IValidation<Values> {
 
     private readonly changeValue: (key: string, value: any) => void
     private readonly currentValues: Values
     private readonly currentValidations?: Validations<Values>
     private readonly shouldValidate: Map<keyof Values, boolean>
     private readonly cachedMessages: Map<keyof Values, string | null>
+    private readonly cachedSetters: Map<keyof Values, (value: any) => void>
+    private readonly cachedProps: Map<keyof Values, CachedInputProps<any>>
 
     constructor(
         changeValue: (key: string, value: any) => void,
         values: Values,
         validations: Validations<Values>,
         shouldValidate: Map<keyof Values, boolean> = new Map<keyof Values, boolean>(),
-        cachedMessages: Map<keyof Values, string | null> = new Map()
+        cachedMessages: Map<keyof Values, string | null> = new Map(),
+        cachedSetters: Map<keyof Values, (value: any) => void> = new Map(),
+        cachedProps: Map<keyof Values, CachedInputProps<any>> = 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<Values> implements IValidation<Values> {
             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<Values[Key]> = {
             // @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<Values>(initValues)
     const cachedMessagesRef = useRef<Map<keyof Values, string | null>>(new Map())
+    const cachedSettersRef = useRef<Map<keyof Values, (value: any) => void>>(new Map())
+    const cachedPropsRef = useRef<Map<keyof Values, CachedInputProps<any>>>(new Map())
     const valuesRef = useRef<Values>(values)
     valuesRef.current = values
 
@@ -142,18 +177,30 @@ export const useForm = <
         values,
         validate,
         useInitialValidation ? new Map<keyof Values, boolean>(Object.keys(initValues).map(k => [k as keyof Values, true])) : new Map<keyof Values, boolean>(),
-        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<Values>(
             changeValue,
             initValues,
             validate,
             useInitialValidation ? new Map<keyof Values, boolean>(Object.keys(initValues).map(k => [k as keyof Values, true])) : new Map<keyof Values, boolean>(),
-            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)

vite.config.js

@@ -57,6 +57,9 @@ export default defineConfig({
                 storybookTest({
                     configDir: path.join(dirname, '.storybook')
                 })],
+            optimizeDeps: {
+                include: ['storybook/test']
+            },
             test: {
                 name: 'storybook',
                 browser: {