Upgrade react-hook-form and improve subnet pool member and IP range validation (#3188)

Resolvers are really for transforming form input into another shape. We
were using it for form-level cross-field validation. The new form-level
`validate` API 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 think `resolver` only
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**
-
[v7.69.0](https://github.com/react-hook-form/react-hook-form/releases/tag/v7.69.0)
`reset({ keepIsValid: true })` fix — the existing `keepIsValid` option
was buggy before. We don't currently use it but `SideModalForm`'s "name
already exists" flow uses `setError` in a post-submit effect; switching
to `errors` prop with focus
([v7.57.0](https://github.com/react-hook-form/react-hook-form/releases/tag/v7.57.0)
`focus form field for errors supplied by errors prop`) could be cleaner.
-
[v7.65.0](https://github.com/react-hook-form/react-hook-form/releases/tag/v7.65.0)
`<Watch />` +
[v7.68.0](https://github.com/react-hook-form/react-hook-form/releases/tag/v7.68.0)
`<FormStateSubscribe />` — render-prop components that re-render only
when a named field (or formState slice) changes. `instance-create.tsx`
and `firewall-rules-common.tsx` have lots of `useWatch` at 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.
-
[v7.61.0](https://github.com/react-hook-form/react-hook-form/releases/tag/v7.61.0)
`useWatch({ compute })` — subscribe to the whole form but only surface a
derived value when a condition matches. Could collapse `useWatch +
useMemo` pairs. `instance-create.tsx:451-463` computes `bootDiskSource`
from four `useWatch` calls; a single `useWatch({ compute })` would work.
-
[v7.63.0](https://github.com/react-hook-form/react-hook-form/releases/tag/v7.63.0)
`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.
-
[v7.55.0](https://github.com/react-hook-form/react-hook-form/releases/tag/v7.55.0)
`createFormControl` / `subscribe` — subscribe to form state outside
React (e.g., from a store). Not obviously useful here.
-
[v7.56.0](https://github.com/react-hook-form/react-hook-form/releases/tag/v7.56.0)
reactive `mode` / `reValidateMode` — these become reactive to
re-renders. The firewall-rules `HACK` comment at
`firewall-rules-common.tsx:136-143` is specifically about the
validate/reValidate regime swap, but that HACK is about `isSubmitted`
state after reset, not about mode being non-reactive, so this doesn't
directly help.

**Bug that might have been biting us**

-
[v7.72.1](https://github.com/react-hook-form/react-hook-form/releases/tag/v7.72.1)
— `setValue` with `shouldDirty` no longer pollutes unrelated dirty
fields. We use `setValue` heavily (silo-create, idp-create,
instance-create); if any of those use `shouldDirty` we likely had
ghost-dirty fields.
  - This sounds really familiar...........
-
[v7.66.1](https://github.com/react-hook-form/react-hook-form/releases/tag/v7.66.1)
— `deepEqual` uses `Object.is` for NaN. `subnet-pool-member-add` stores
`NaN` as the unset state for optional NumberFields — pre-fix, `isDirty`
might have flipped on those.
-
[v7.69.0](https://github.com/react-hook-form/react-hook-form/releases/tag/v7.69.0)
— race between `setError` and `setFocus` resolved. Relevant to our
`SideModalForm` effect that calls both.

Author: david-crespo

app/components/form/fields/NumberField.tsx

@@ -65,6 +65,7 @@ export const NumberFieldInner = <
   name,
   label = capitalize(name),
   validate,
+  deps,
   control,
   required,
   id: idProp,
@@ -83,6 +84,7 @@ export const NumberFieldInner = <
     control,
     rules: {
       required,
+      deps,
       // it seems we need special logic to enforce required on NaN
       validate(value, values) {
         if (required && Number.isNaN(value)) return `${label} is required`

app/components/form/fields/TextField.tsx

@@ -12,6 +12,7 @@ import {
   type FieldPath,
   type FieldPathValue,
   type FieldValues,
+  type RegisterOptions,
   type Validate,
 } from 'react-hook-form'
 
@@ -45,6 +46,7 @@ export interface TextFieldProps<
   placeholder?: string
   units?: string
   validate?: Validate<FieldPathValue<TFieldValues, TName>, TFieldValues>
+  deps?: RegisterOptions<TFieldValues, TName>['deps']
   control: Control<TFieldValues>
   /** Alters the value of the input during the field's onChange event. */
   transform?: (value: string) => string
@@ -98,6 +100,7 @@ export const TextFieldInner = <
   type = 'text',
   label = capitalize(name),
   validate,
+  deps,
   control,
   required,
   id: idProp,
@@ -109,7 +112,7 @@ export const TextFieldInner = <
   const {
     field: { onChange, ...fieldRest },
     fieldState: { error },
-  } = useController({ name, control, rules: { required, validate } })
+  } = useController({ name, control, rules: { required, validate, deps } })
   return (
     <>
       <UITextField

app/forms/ip-pool-range-add.tsx

@@ -5,7 +5,7 @@
  *
  * Copyright Oxide Computer Company
  */
-import { useForm, type FieldErrors } from 'react-hook-form'
+import { useForm } from 'react-hook-form'
 import { useNavigate } from 'react-router'
 
 import {
@@ -34,57 +34,11 @@ const defaultValues: IpRange = {
   last: '',
 }
 
-// Using a resolver overrides all field-level validation (required, min, max,
-// etc.), so this function must cover everything. Field-level `required` props
-// still affect UI display (hiding the "optional" label) but are inert for
-// validation.
-
-/**
- * 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) {
-  return (values: IpRange) => {
-    const first = parseIp(values.first)
-    const last = parseIp(values.last)
-
-    const errors: FieldErrors<IpRange> = {}
-
-    // Validate first address matches pool version
-    if (first.type === 'error') {
-      errors.first = { type: 'pattern', message: first.message }
-    } else if (first.type === 'v4' && poolVersion === 'v6') {
-      errors.first = {
-        type: 'pattern',
-        message: 'IPv4 address not allowed in IPv6 pool',
-      }
-    } else if (first.type === 'v6' && poolVersion === 'v4') {
-      errors.first = {
-        type: 'pattern',
-        message: 'IPv6 address not allowed in IPv4 pool',
-      }
-    }
-
-    // Validate last address matches pool version
-    if (last.type === 'error') {
-      errors.last = { type: 'pattern', message: last.message }
-    } else if (last.type === 'v4' && poolVersion === 'v6') {
-      errors.last = {
-        type: 'pattern',
-        message: 'IPv4 address not allowed in IPv6 pool',
-      }
-    } else if (last.type === 'v6' && poolVersion === 'v4') {
-      errors.last = {
-        type: 'pattern',
-        message: 'IPv6 address not allowed in IPv4 pool',
-      }
-    }
-
-    // TODO: if we were really cool we could check first <= last but it would add
-    // 6k gzipped to the bundle with ip-num
-
-    // no errors
-    return Object.keys(errors).length > 0 ? { values: {}, errors } : { values, errors: {} }
+const validateAddress = (value: string, poolVersion: IpVersion) => {
+  const parsed = parseIp(value)
+  if (parsed.type === 'error') return parsed.message
+  if (parsed.type !== poolVersion) {
+    return `IP${parsed.type} address not allowed in IP${poolVersion} pool`
   }
 }
 
@@ -107,10 +61,7 @@ export default function IpPoolAddRange() {
     },
   })
 
-  const form = useForm({
-    defaultValues,
-    resolver: createResolver(poolData.ipVersion),
-  })
+  const form = useForm({ defaultValues })
 
   return (
     <SideModalForm
@@ -132,12 +83,14 @@ export default function IpPoolAddRange() {
         description="First address in the range"
         control={form.control}
         required
+        validate={(value) => validateAddress(value, poolData.ipVersion)}
       />
       <TextField
         name="last"
         description="Last address in the range"
         control={form.control}
         required
+        validate={(value) => validateAddress(value, poolData.ipVersion)}
       />
       <SideModalFormDocs docs={[docLinks.systemIpPools]} />
     </SideModalForm>

app/forms/subnet-pool-member-add.spec.ts

@@ -7,91 +7,93 @@
  */
 import { describe, expect, it } from 'vitest'
 
-import { createResolver } from './subnet-pool-member-add'
+import { validateMember } from './subnet-pool-member-add'
 
-const resolve = createResolver('v4')
-const resolve6 = createResolver('v6')
+const validate = (values: Parameters<typeof validateMember>[1]) =>
+  validateMember('v4', values)
+const validate6 = (values: Parameters<typeof validateMember>[1]) =>
+  validateMember('v6', values)
 
 const valid = { subnet: '10.0.0.0/16', minPrefixLength: 20, maxPrefixLength: 28 }
 
-type Result = ReturnType<typeof resolve>
+type Field = 'subnet' | 'minPrefixLength' | 'maxPrefixLength'
 
-function errMsg(result: Result, field: keyof Result['errors']) {
-  return result.errors[field]?.message
+function errMsg(result: ReturnType<typeof validate>, field: Field) {
+  return result[field]
 }
 
-describe('createResolver', () => {
+describe('validateMember', () => {
   it('accepts valid v4 input', () => {
-    expect(Object.keys(resolve(valid).errors)).toEqual([])
+    expect(validate(valid)).toEqual({})
   })
 
   it('accepts valid v6 input', () => {
-    const result = resolve6({
+    const result = validate6({
       subnet: 'fd00:1000::/32',
       minPrefixLength: 48,
       maxPrefixLength: 64,
     })
-    expect(Object.keys(result.errors)).toEqual([])
+    expect(result).toEqual({})
   })
 
   it('accepts omitted prefix lengths', () => {
-    const result = resolve({
+    const result = validate({
       subnet: '10.0.0.0/16',
       minPrefixLength: NaN,
       maxPrefixLength: NaN,
     })
-    expect(Object.keys(result.errors)).toEqual([])
+    expect(result).toEqual({})
   })
 
   it('rejects invalid CIDR', () => {
-    const result = resolve({ ...valid, subnet: 'not-a-cidr' })
+    const result = validate({ ...valid, subnet: 'not-a-cidr' })
     expect(errMsg(result, 'subnet')).toMatch(/IP address/)
   })
 
   it('rejects v6 subnet in v4 pool', () => {
-    const result = resolve({ ...valid, subnet: 'fd00::/32' })
+    const result = validate({ ...valid, subnet: 'fd00::/32' })
     expect(errMsg(result, 'subnet')).toBe('IPv6 subnet not allowed in IPv4 pool')
   })
 
   it('rejects v4 subnet in v6 pool', () => {
-    const result = resolve6({ ...valid, subnet: '10.0.0.0/16' })
+    const result = validate6({ ...valid, subnet: '10.0.0.0/16' })
     expect(errMsg(result, 'subnet')).toBe('IPv4 subnet not allowed in IPv6 pool')
   })
 
   it('rejects min > max prefix length', () => {
-    const result = resolve({ ...valid, minPrefixLength: 28, maxPrefixLength: 20 })
+    const result = validate({ ...valid, minPrefixLength: 28, maxPrefixLength: 20 })
     expect(errMsg(result, 'minPrefixLength')).toMatch(/≤/)
   })
 
   it('rejects min prefix length < subnet width', () => {
-    const result = resolve({ ...valid, minPrefixLength: 8 })
+    const result = validate({ ...valid, minPrefixLength: 8 })
     expect(errMsg(result, 'minPrefixLength')).toMatch(/≥ subnet prefix length \(16\)/)
   })
 
   it('rejects max prefix length < subnet width', () => {
-    const result = resolve({ ...valid, maxPrefixLength: 8 })
+    const result = validate({ ...valid, maxPrefixLength: 8 })
     expect(errMsg(result, 'maxPrefixLength')).toMatch(/≥ subnet prefix length \(16\)/)
   })
 
   it('rejects prefix length above max bound (v4: 32)', () => {
-    const result = resolve({ ...valid, minPrefixLength: 33 })
+    const result = validate({ ...valid, minPrefixLength: 33 })
     expect(errMsg(result, 'minPrefixLength')).toBe('Must be between 0 and 32')
   })
 
   it('rejects prefix length below 0', () => {
-    const result = resolve({ ...valid, maxPrefixLength: -1 })
+    const result = validate({ ...valid, maxPrefixLength: -1 })
     expect(errMsg(result, 'maxPrefixLength')).toBe('Must be between 0 and 32')
   })
 
   it('shows min-≤-max error even when min is also below subnet width', () => {
     // min(12) > max(10) AND min(12) < subnetWidth(16): the min-≤-max error
     // should take priority over the subnet-width error
-    const result = resolve({ ...valid, minPrefixLength: 12, maxPrefixLength: 10 })
+    const result = validate({ ...valid, minPrefixLength: 12, maxPrefixLength: 10 })
     expect(errMsg(result, 'minPrefixLength')).toMatch(/≤/)
   })
 
   it('rejects prefix length above max bound (v6: 128)', () => {
-    const result = resolve6({
+    const result = validate6({
       subnet: 'fd00::/32',
       minPrefixLength: 48,
       maxPrefixLength: 200,