Repository navigation
🐞 fix(useForm): reset options that are removed from the props on re-render - #13831
bluebill1049 merged 4 commits into
Conversation
bluebill1049
left a comment
bluebill1049
left a comment
There was a problem hiding this comment.
I think the stale option issue is valid, but can we avoid tracking/resetting every key from UseFormProps?
Props such as values, errors, defaultValues, formControl, etc. have their own lifecycle handling in useForm, so treating every removed prop as a control._options option feels a bit too broad.
Could we limit this to the actual persistent control options that need resetting, or centralize the option update logic instead?
653409e to
b8e7378
Compare
|
Good point, thanks - updated. The reset now skips the props that have their own lifecycle handling ( Both new tests still pass, and the full |
|
Thanks, this is safer than the previous version. I still think tracking all prop keys and excluding special cases is a bit broad though. It means future UseFormProps with their own lifecycle need to be manually added to this exclusion list. Could we make this opt-in instead and explicitly define the control options that should be synchronized/reset? That would also avoid needing the Record<string, unknown> lookup against defaultOptions. |
…ender Signed-off-by: Charan Rathore <[email protected]>
|
Done - it is opt-in now. There is an explicit SYNCED_OPTIONS list (mode, reValidateMode, shouldFocusError, resolver, criteriaMode, delayError, shouldUnregister, shouldUseNativeValidation, context), and only a listed option that was present in the previous options and is gone from the props gets reset: to its defaultOptions value when it has one, cleared otherwise. No prop-key tracking and no blanket Record lookup anymore; new UseFormProps stay out of this unless someone adds them to the list deliberately. useForm suite and tsc are green. |
b8e7378 to
54c495e
Compare
Replace SYNCED_OPTIONS with RESET_OPTIONS for default handling.
|
I did a bit of cleanup there. Could you help me re-run Prettier? Thanks! |
Signed-off-by: Charan Rathore <[email protected]>
|
Done, reran Prettier on the touched files and pushed the formatting fix. Thanks! |
Pull Request Template
Proposed Changes
When an option is left out of the props
useFormgets on a later render, the form keeps using the value from the earlier render.For example
useForm(useResolver ? { resolver } : {}): afteruseResolverturns false the resolver is still called on submit and its errors still block the form. The same happens withmode: going from{ mode: 'onChange' }to{}keeps validating on change instead of going back to the defaultonSubmit.The cause is that the options are merged into the previous ones on every render, so a key that is missing is never cleared.
validatealready had a special case for this.The fix remembers which option keys were passed last render. Keys that are now missing go back to their default (
mode,reValidateModeandshouldFocusErrorhave defaults, the rest become undefined). Options passed to a form made withcreateFormControlare untouched, because only keys that came fromuseFormprops are reset.I added tests for a removed resolver and a removed mode. Both fail without the fix.
Fixes # (no issue, found while reading how the options are updated)
Type of change
Checklist: