[Bug]: Setting value on a captured reference to the set function on `useControlField` doesn't update the value
Closed this issue · 3 comments
Which packages are impacted?
-
remix-validated-form -
@remix-validated-form/with-zod -
@remix-validated-form/with-yup -
zod-form-data
What version of these packages are you using?
latest
Please provide a link to a minimal reproduction of the issue.
https://codesandbox.io/p/sandbox/ecstatic-haze-y4wtdf?file=%2Fapp%2Froutes%2Findex.tsx%3A26%2C21
Steps to Reproduce the Bug or Issue
In the forked starter template, the first field uses useControlField and works as expected. The second field captures the setValue function with a useCallback and entering text into the field no longer works. The same problem happens when we use useMemo or any other function which captures a reference to the setter function. The same issue also seems to happen with useUpdateControlField as well.
This can cause really difficult to debug issues, since it is very easy with useEffect, useCallback and so on to accidentally use an "outdated" version of the setter function. It'd be really great if using an old version of it worked just fine.
In the starter template we also illustrated an ugly workaround using useRef. By using a ref and always setting it to the latest version of the setter function everything works as expected.
Expected behavior
I would want the setter function to work regardless of whether it has been captured in a previous render pass.
Screenshots or Videos
No response
Platform
- OS: [e.g. macOS, Windows, Linux]
- Browser: [e.g. Chrome, Safari, Firefox]
- Version: [e.g. 91.1]
Additional context
No response
I'm also seeing the same issue when using clearError() in a useCallback
The fact that this hook mirrors useState's api probably contributes to the confusion here. With useState it's safe to leave the setter of of the deps array, but here it isn't. Currently, you need to do this:
const [value, setValue] = useControlField("myFieldName");
const callback = useCallback((value) => setValue(value), [setValue]);It's definitely possible (and desirable) to change this. But it would require a bit of rearchitecting, so I can't promise a timeline.
A brief explanation of what's going on:
The form's zustand store is using a "slices"-esque pattern. The state for each form and the callbacks for updating the form's state get created for each form individually after the form is initially mounted. So on the first render we don't actually have a reference to the setValue function to provide via the hook. Therefore we use a no-op function on the first render and swap in the real one after.
This definitely has some downsides we can improve upon:
- There's a minimum of two render passes when mounting a form
- Each render pass has a different function reference for some callbacks (like you've brought up here).
To improve on this we have a couple options:
- Move the state update callbacks up to the top-most level of the form store instead of creating them for each "slice". Then update all usages of these in the project.
- Pull the callbacks out of the store using a ref to keep the function identity stable.