Upgrade react-hook-form and use new form-level validate#3188
Upgrade react-hook-form and use new form-level validate#3188david-crespo wants to merge 1 commit intomainfrom
validate#3188Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| * Validates IP range addresses against the pool's IP version. | ||
| * Ensures both addresses are valid IPs and match the pool's version. | ||
| */ | ||
| function createResolver(poolVersion: IpVersion) { |
There was a problem hiding this comment.
I believe this was originally a resolver because pools were not locked to a single IP version, so we had to make sure the start and end were of the same version. Now we can just compare each one individually to the version of the pool. So this can all just be a field-level validation. This change does not rely on form-level validation, this was just discovered while looking for candidates for form-level validation.
| // Uses form-level validate (RHF ≥7.72.0) so we can look at all three fields | ||
| // together. Unlike `resolver`, this runs alongside field-level validation, so | ||
| // `required` / `min` / `max` on the fields still apply. | ||
| export function validateForm(poolVersion: IpVersion, values: MemberAddForm) { |
There was a problem hiding this comment.
Look at this with whitespace changes hidden to see what a small change it is. Basically it's just the return type changing.
https://github.com/oxidecomputer/console/pull/3188/changes?w=1
| const subnetWidth = parsed.type !== 'error' ? parsed.width : undefined | ||
| const inRange = (v: number) => !Number.isNaN(v) && v >= 0 && v <= maxBound | ||
|
|
||
| const errors: Partial<Record<keyof MemberAddForm, { type: string; message: string }>> = {} |
There was a problem hiding this comment.
Don't love this, will revisit.
Resolvers are really for transforming form input into another shape. We were using it for form-level cross-field validation. The new form-level
validateAPI added in react-hook-form/react-hook-form#13195 is perfect for what we were doing. It does not override existing validations and it runs whenever validations are supposed to run (I thinkresolveronly runs on submit.Interesting changes in RHF
I had Claude go through the releases since the old version and summarize the interesting stuff.
Features worth a look
reset({ keepIsValid: true })fix — the existingkeepIsValidoption was buggy before. We don't currently use it butSideModalForm's "name already exists" flow usessetErrorin a post-submit effect; switching toerrorsprop with focus (v7.57.0focus form field for errors supplied by errors prop) could be cleaner.<Watch />+ v7.68.0<FormStateSubscribe />— render-prop components that re-render only when a named field (or formState slice) changes.instance-create.tsxandfirewall-rules-common.tsxhave lots ofuseWatchat the top level; some of that churn could be isolated inside a<Watch>render-prop. Worth trying only if profiling shows the re-renders hurt.useWatch({ compute })— subscribe to the whole form but only surface a derived value when a condition matches. Could collapseuseWatch + useMemopairs.instance-create.tsx:451-463computesbootDiskSourcefrom fouruseWatchcalls; a singleuseWatch({ compute })would work.getValues(undefined, { dirtyFields: true })— extract only dirty fields. Useful for PATCH-style edits; our edit forms mostly send the full object so the win is small.createFormControl/subscribe— subscribe to form state outside React (e.g., from a store). Not obviously useful here.mode/reValidateMode— these become reactive to re-renders. The firewall-rulesHACKcomment atfirewall-rules-common.tsx:136-143is specifically about the validate/reValidate regime swap, but that HACK is aboutisSubmittedstate after reset, not about mode being non-reactive, so this doesn't directly help.Bug that might have been biting us
setValuewithshouldDirtyno longer pollutes unrelated dirty fields. We usesetValueheavily (silo-create, idp-create, instance-create); if any of those useshouldDirtywe likely had ghost-dirty fields.deepEqualusesObject.isfor NaN.subnet-pool-member-addstoresNaNas the unset state for optional NumberFields — pre-fix,isDirtymight have flipped on those.setErrorandsetFocusresolved. Relevant to ourSideModalFormeffect that calls both.