mantine
mantine copied to clipboard
[@mantine/form] use-form: Make functions referentially stable
Fixes #1383
The issue before was when getInputProps
was called, valuesRef
hadn't been updated by useEffect
yet (guessing React was deferring this effect). This caused the value and onChange to have an alternating state where typing "abcd" would result in "ac" and "bd" shown.
Now the ref should always be up to date when useForm
is called so any calls to getInputProps
shouldn't be able to occur with stale values.
The changes from the previous PR are in the 2 new hooks
Tested using all the Storybook examples
@mattrunyon did you have a chance to test it with React 18?
Tested on React 18 (but not with any explicit concurrent features as I haven't used them before). With useLayoutEffect
the same bug appears (in 17 and 18). I'll just make getInputProps
and getListInputProps
depend on values
and errors
so they're still optimized some, but not completely stable functions.
useLayoutEffect
happens after DOM mutation and before paint. useEffect
happens after mutation and paint. So both have the issue because getInputProps
is called before/during DOM mutation on render.
I'm not entirely sure if there would be issues not using useEffect
to update the ref in React 18 (again, I haven't used concurrent features), but my guess is in a concurrent render React might use the same ref object which could create issues if the concurrent render is discarded. Not entirely sure though since the values for the refs should only change on user inputs, so any renders which change the ref should be actual renders and not concurrent.
I also tested by adding this to Form.demo.password
since it was the original reason I wanted the functions to be stable. The use case was update form validation on user input
useEffect(() => {
form.validate();
}, [form.values]);
Well, seems very complicated, maybe there is a way to remove refs and implement it with state only
Implementing with state you will have to recreate most functions each render because any time the values change, any function which uses the values would change. Nearly every function uses the state at some point, so useCallback
would be useless then since they'll change on almost every call.
Or you have to have all the functions take in values, like form.setFieldValue(values, fieldName, value)
which is annoying
@mattrunyon I've updated v5 branch to react 18, it turns out if you just update react in package.json, storybook will not use it and turn off new async features by default. Can you check if your changes work with the updated v5 branch? Also, can you please resolve conflicts?