refactor: replace react-hook-form with TanStack Form - #756
Open
azizbecha wants to merge 2 commits into
Open
Conversation
Swap the form engine while keeping the component surface (Form,
FormField*, FormFieldController). Zod 4 validates natively via
Standard Schema, so @hookform/resolvers is dropped too.
- new useForm wrapper ({ schema, mode, defaultValues, onSubmit })
built on revalidateLogic for react-hook-form validation-timing
parity; onSubmit receives schema.parse(value) so zod transforms
still apply
- controller context exposes { field, fieldState, isInvalid } with
a useStore-subscribed snapshot to stay reactive under React
Compiler memoization
- setError -> setErrorMap onDynamic fields; isDirty -> !isDefaultValue;
watch/formState -> useStore selectors; reactive values: -> computed
defaultValues
- Form keeps focus-first-invalid-field behavior on failed submit
- disabled fields now submit their value (react-hook-form submitted
undefined); browser specs updated accordingly
Co-authored-by: Claude <noreply@anthropic.com>
|
@azizbecha is attempting to deploy a commit to the Team Bearstudio Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
WalkthroughThis PR migrates the entire form system from react-hook-form to ChangesTanStack React Form Migration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dedupe the setErrorMap incantation repeated across five pages; SonarCloud flagged the copies on the quality gate. Co-authored-by: Claude <noreply@anthropic.com>
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/features/user/manager/page-user-update.tsx (1)
62-93: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInitialize the form from resolved user data
src/features/user/manager/page-user-update.tsx:81-93
defaultValuesare mount-time only here, so the form can stay on the empty fallback values and submit blanks if the query resolves later. Gate the form onuserQuery.status === 'success'or callform.reset(userQuery.data)when the data arrives.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/user/manager/page-user-update.tsx` around lines 62 - 93, The user update form is using mount-time default values from userQuery.data in useForm, so it can stay on empty fallback values when the query resolves later. Update the form initialization in page-user-update.tsx by either rendering/gating the form until userQuery.status is success, or by resetting the existing form with the resolved userQuery.data once it arrives. Use the useForm setup, userQuery, and the form.reset path to ensure the fields are populated from the fetched user before submit.src/features/book/manager/page-book-update.tsx (1)
27-33: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate the update form until the book query resolves
useFormis seeded frombookQuery.data, but the form mounts immediately and never rehydrates when that query finishes. On a direct load, the edit screen can stay on the empty fallback values and submit blanks for an existing book.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/book/manager/page-book-update.tsx` around lines 27 - 33, The update screen in PageBookUpdate mounts the form before bookQuery finishes, so useForm is initialized with empty fallback values and never picks up the loaded book. Gate rendering of the form until bookQuery resolves successfully, and initialize the form only from the resolved book data in PageBookUpdate/useForm so direct loads cannot submit blank fields.src/components/form/field-radio-group/index.tsx (1)
45-59: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInconsistent
aria-invalidhandling inrenderOptionpath.Line 53 passes raw
isInvalidto the custom render path, while line 64 (defaultRadiopath) usesisInvalid ? true : undefined. This means custom-rendered radios always get an explicitaria-invalidattribute (even"false"), diverging from the pattern established elsewhere in this same file.🐛 Proposed fix
{renderOption({ label, - 'aria-invalid': isInvalid, + 'aria-invalid': isInvalid ? true : undefined, size: ctx.size, ...option, })}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/form/field-radio-group/index.tsx` around lines 45 - 59, The `renderOption` branch in `FieldRadioGroup` is passing `isInvalid` directly as `aria-invalid`, which can emit an explicit false value and differs from the default `Radio` path. Update the custom render payload in `field-radio-group/index.tsx` so `aria-invalid` follows the same pattern as the non-custom branch, using the `isInvalid ? true : undefined` behavior when calling `renderOption` from `FieldRadioGroup`.src/components/form/field-otp/index.tsx (1)
24-58: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
onCompletewrapper can be overridden by consumer-suppliedonComplete, silently disabling auto-submit.
onComplete(Line 36) is defined before{...rest}(Line 48), whileonChange/onBlur/name/valueare correctly placed after{...rest}to enforce the controlled-field bindings. SinceonCompleteisn't destructured out ofrest, any consumer passing their ownonCompleteprop toFieldOtpwill have it clobber this wrapper via the spread, silently skipping the auto-submit logic (Lines 39-46) even whenautoSubmitis set.🐛 Proposed fix: destructure `onComplete` and place it after the spread
- const { containerProps, autoSubmit, ...rest } = props; + const { containerProps, autoSubmit, onComplete, ...rest } = props; ... <InputOTP id={ctx.id} aria-invalid={isInvalid ? true : undefined} aria-describedby={ctx.describedBy(isInvalid)} - onComplete={(v) => { - rest.onComplete?.(v); - // Only auto submit on first try - if (!isSubmitted && autoSubmit) { - const button = document.createElement('button'); - button.type = 'submit'; - button.style.display = 'none'; - containerRef.current?.append(button); - button.click(); - button.remove(); - } - }} {...rest} name={field.name} value={fieldState.value ?? ''} + onComplete={(v) => { + onComplete?.(v); + // Only auto submit on first try + if (!isSubmitted && autoSubmit) { + const button = document.createElement('button'); + button.type = 'submit'; + button.style.display = 'none'; + containerRef.current?.append(button); + button.click(); + button.remove(); + } + }} onChange={(value) => { field.handleChange(value); rest.onChange?.(value); }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/form/field-otp/index.tsx` around lines 24 - 58, The `FieldOtp` `onComplete` handler is being overridden by the consumer-provided prop from `rest`, which disables the auto-submit wrapper. In `src/components/form/field-otp/index.tsx`, destructure `onComplete` from `props` alongside the other props, then pass the wrapper handler after spreading `rest` in the `InputOTP` props so it cannot be clobbered. Keep the existing auto-submit logic inside the `onComplete` wrapper and still call the consumer callback from that wrapper.
🧹 Nitpick comments (3)
src/components/form/use-form.ts (2)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant union type flagged by SonarCloud.
unknown | Promise<unknown>collapses tounknownsincePromise<unknown>is already assignable tounknown. No runtime impact, but considervoid | Promise<void>for clearer intent.♻️ Suggested fix
- onSubmit?: (values: z.output<TSchema>) => unknown | Promise<unknown>; + onSubmit?: (values: z.output<TSchema>) => void | Promise<void>;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/form/use-form.ts` at line 19, The onSubmit type in use-form is using a redundant union because Promise<unknown> already fits within unknown, so simplify the signature to make the intent clearer. Update the onSubmit property in the use-form type definition to use a cleaner return type such as void | Promise<void>, and keep the change localized to the onSubmit declaration so the rest of the form API remains unchanged.Source: Linters/SAST tools
48-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBlanket
as ExplicitAnycast hides type errors onsetErrorMappayload.The whole options object is cast away rather than just the ambiguous parts, so any future mismatch between this shape and FormApi's expected
errorMaptype (e.g. a library upgrade changing theonDynamicpayload shape) would silently typecheck. Confirmed via TanStack Form docs thatsetErrorMap({ cause: { fields, form } })is the correct shape, so a narrower assertion (or improvingFormInstance's generics to accept this) would be safer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/form/use-form.ts` around lines 48 - 59, The blanket as ExplicitAny cast in setFormFieldError is hiding the real type of the setErrorMap payload. Update setFormFieldError to use the FormApi/FormInstance error-map shape directly, or narrow the assertion to only the ambiguous nested part instead of casting the whole object. Check the setErrorMap call in use-form.ts and align the payload with the expected errorMap structure from TanStack Form so future shape changes still typecheck.src/components/form/form-field-controller/index.tsx (1)
57-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMark the internal component props as read-only.
This resolves the current SonarCloud warning without changing behavior.
Proposed fix
}: { +}: Readonly<{ field: AnyFieldApi; type: FieldType | 'custom'; displayError: boolean; fieldProps: Record<string, unknown>; customRender?: FormFieldControllerCustomRender; -}) { +}>) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/form/form-field-controller/index.tsx` around lines 57 - 69, Mark the props for FormFieldControllerRender as read-only by updating its parameter type so the field, type, displayError, fieldProps, and customRender values cannot be reassigned inside the component. Keep the behavior unchanged and apply the readonly typing directly on the FormFieldControllerRender props shape to satisfy the SonarCloud warning.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/form/field-upload-input/index.tsx`:
- Around line 30-40: The field upload input rebuilds UploadInput.defaultValue on
every render, which breaks the cleared-state reference check in UploadInput and
can make a cleared file reappear. In the field-upload-input component, memoize
the object passed as defaultValue so the same reference is reused when the
underlying URL value from getFieldUrl(fieldState.value) has not changed; keep
the logic localized around the UploadInput props and the value-to-defaultValue
mapping.
In `@src/components/form/form-field-controller/index.tsx`:
- Around line 13-16: The custom render API in FormFieldController is dropping
the disabled prop, so type="custom" callers never receive it in render. Update
FormFieldControllerCustomRender and the FormFieldController custom branch to
pass disabled through alongside field and fieldState, using the existing
FormFieldController / render / fieldProps flow so custom renderers can consume
the common prop directly.
In `@src/components/form/form-field-error.tsx`:
- Around line 26-34: The standalone props union in FormFieldErrorProps currently
allows partial combinations like form without name, which can slip into the
controller path and fail at runtime. Tighten the union by replacing the empty
object branch with an explicit “neither” shape using never fields so
FormFieldErrorDisplayProps is accepted only when both form and name are provided
together or both are omitted. Update the type alias in form-field-error.tsx
accordingly, keeping the FormFieldErrorProps and DeepKeys/FormInstance
references aligned.
In `@src/features/auth/page-login-verify.tsx`:
- Around line 62-64: The submit flow in the login verification handler is
resolving before the session refresh finishes, which can let users resubmit too
early. Update the `onSubmit` logic in `page-login-verify.tsx` so
`session.refetch()` is awaited before the handler completes, ensuring
`isSubmitting` stays true until the refresh and redirect guard work is done.
In `@src/features/auth/page-onboarding.tsx`:
- Around line 43-46: The onboarding form seeds `defaultValues` from
`session.data?.user.name`, but that only runs once, so late-arriving session
data can leave the name field empty. Update the `page-onboarding.tsx` form setup
to react to session changes by resetting/re-seeding the form when
`session.data?.user.name` updates, or defer rendering until the session is
ready; use the `zFormFieldsOnboarding` form initialization and the onboarding
form component as the places to adjust.
---
Outside diff comments:
In `@src/components/form/field-otp/index.tsx`:
- Around line 24-58: The `FieldOtp` `onComplete` handler is being overridden by
the consumer-provided prop from `rest`, which disables the auto-submit wrapper.
In `src/components/form/field-otp/index.tsx`, destructure `onComplete` from
`props` alongside the other props, then pass the wrapper handler after spreading
`rest` in the `InputOTP` props so it cannot be clobbered. Keep the existing
auto-submit logic inside the `onComplete` wrapper and still call the consumer
callback from that wrapper.
In `@src/components/form/field-radio-group/index.tsx`:
- Around line 45-59: The `renderOption` branch in `FieldRadioGroup` is passing
`isInvalid` directly as `aria-invalid`, which can emit an explicit false value
and differs from the default `Radio` path. Update the custom render payload in
`field-radio-group/index.tsx` so `aria-invalid` follows the same pattern as the
non-custom branch, using the `isInvalid ? true : undefined` behavior when
calling `renderOption` from `FieldRadioGroup`.
In `@src/features/book/manager/page-book-update.tsx`:
- Around line 27-33: The update screen in PageBookUpdate mounts the form before
bookQuery finishes, so useForm is initialized with empty fallback values and
never picks up the loaded book. Gate rendering of the form until bookQuery
resolves successfully, and initialize the form only from the resolved book data
in PageBookUpdate/useForm so direct loads cannot submit blank fields.
In `@src/features/user/manager/page-user-update.tsx`:
- Around line 62-93: The user update form is using mount-time default values
from userQuery.data in useForm, so it can stay on empty fallback values when the
query resolves later. Update the form initialization in page-user-update.tsx by
either rendering/gating the form until userQuery.status is success, or by
resetting the existing form with the resolved userQuery.data once it arrives.
Use the useForm setup, userQuery, and the form.reset path to ensure the fields
are populated from the fetched user before submit.
---
Nitpick comments:
In `@src/components/form/form-field-controller/index.tsx`:
- Around line 57-69: Mark the props for FormFieldControllerRender as read-only
by updating its parameter type so the field, type, displayError, fieldProps, and
customRender values cannot be reassigned inside the component. Keep the behavior
unchanged and apply the readonly typing directly on the
FormFieldControllerRender props shape to satisfy the SonarCloud warning.
In `@src/components/form/use-form.ts`:
- Line 19: The onSubmit type in use-form is using a redundant union because
Promise<unknown> already fits within unknown, so simplify the signature to make
the intent clearer. Update the onSubmit property in the use-form type definition
to use a cleaner return type such as void | Promise<void>, and keep the change
localized to the onSubmit declaration so the rest of the form API remains
unchanged.
- Around line 48-59: The blanket as ExplicitAny cast in setFormFieldError is
hiding the real type of the setErrorMap payload. Update setFormFieldError to use
the FormApi/FormInstance error-map shape directly, or narrow the assertion to
only the ambiguous nested part instead of casting the whole object. Check the
setErrorMap call in use-form.ts and align the payload with the expected errorMap
structure from TanStack Form so future shape changes still typecheck.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 05408bb6-dc35-4612-a65f-92920b25c7ba
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (59)
package.jsonsrc/components/form/docs.stories.tsxsrc/components/form/field-checkbox-group/docs.stories.tsxsrc/components/form/field-checkbox-group/field-checkbox-group.browser.spec.tsxsrc/components/form/field-checkbox-group/index.tsxsrc/components/form/field-checkbox/docs.stories.tsxsrc/components/form/field-checkbox/field-checkbox.browser.spec.tsxsrc/components/form/field-checkbox/index.tsxsrc/components/form/field-combobox-multiple/docs.stories.tsxsrc/components/form/field-combobox-multiple/field-combobox-multiple.browser.spec.tsxsrc/components/form/field-combobox-multiple/index.tsxsrc/components/form/field-combobox/docs.stories.tsxsrc/components/form/field-combobox/field-combobox.browser.spec.tsxsrc/components/form/field-combobox/index.tsxsrc/components/form/field-custom/docs.stories.tsxsrc/components/form/field-date/docs.stories.tsxsrc/components/form/field-date/index.tsxsrc/components/form/field-number/docs.stories.tsxsrc/components/form/field-number/index.tsxsrc/components/form/field-otp/docs.stories.tsxsrc/components/form/field-otp/field-otp.browser.spec.tsxsrc/components/form/field-otp/index.tsxsrc/components/form/field-radio-group/docs.stories.tsxsrc/components/form/field-radio-group/field-radio-group.browser.spec.tsxsrc/components/form/field-radio-group/index.tsxsrc/components/form/field-select/docs.stories.tsxsrc/components/form/field-select/field-select.browser.spec.tsxsrc/components/form/field-select/index.tsxsrc/components/form/field-text/docs.stories.tsxsrc/components/form/field-text/index.browser.spec.tsxsrc/components/form/field-text/index.tsxsrc/components/form/field-textarea/docs.stories.tsxsrc/components/form/field-textarea/field-textarea.browser.spec.tsxsrc/components/form/field-textarea/index.tsxsrc/components/form/field-upload-input/docs.stories.tsxsrc/components/form/field-upload-input/index.tsxsrc/components/form/form-field-controller/context.tsxsrc/components/form/form-field-controller/index.tsxsrc/components/form/form-field-error.stories.tsxsrc/components/form/form-field-error.tsxsrc/components/form/form-test-utils.tsxsrc/components/form/form.tsxsrc/components/form/index.tssrc/components/form/types.tssrc/components/form/use-form.tssrc/components/ui/input.tsxsrc/components/ui/popover.stories.tsxsrc/features/account/change-name-drawer.tsxsrc/features/auth/page-login-verify.tsxsrc/features/auth/page-login.tsxsrc/features/auth/page-onboarding.tsxsrc/features/book/manager/form-book-cover.tsxsrc/features/book/manager/form-book.tsxsrc/features/book/manager/page-book-new.tsxsrc/features/book/manager/page-book-update.tsxsrc/features/devtools/login-hint.tsxsrc/features/user/manager/form-user.tsxsrc/features/user/manager/page-user-new.tsxsrc/features/user/manager/page-user-update.tsx
Comment on lines
+30
to
40
| const value = getFieldUrl(fieldState.value); | ||
|
|
||
| return ( | ||
| <FormFieldContainer {...containerProps}> | ||
| <UploadInput | ||
| {...rest} | ||
| disabled={field.disabled ?? rest.disabled} | ||
| defaultValue={ | ||
| value | ||
| ? { name: value.split('/').at(-1) ?? value, url: value } | ||
| : undefined | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Memoize defaultValue to preserve clear behavior.
defaultValue is rebuilt as a new object literal on every render. UploadInput detects a "cleared" state via defaultValue !== clearedDefaultValue (strict reference equality), so a fresh object each render will make the cleared file/URL reappear on the next unrelated re-render.
🐛 Proposed fix: stabilize the object reference
+ const value = getFieldUrl(fieldState.value);
+ const defaultValue = useMemo(
+ () =>
+ value ? { name: value.split('/').at(-1) ?? value, url: value } : undefined,
+ [value]
+ );
-
- const value = getFieldUrl(fieldState.value);
return (
<FormFieldContainer {...containerProps}>
<UploadInput
{...rest}
- defaultValue={
- value
- ? { name: value.split('/').at(-1) ?? value, url: value }
- : undefined
- }
+ defaultValue={defaultValue}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const value = getFieldUrl(fieldState.value); | |
| return ( | |
| <FormFieldContainer {...containerProps}> | |
| <UploadInput | |
| {...rest} | |
| disabled={field.disabled ?? rest.disabled} | |
| defaultValue={ | |
| value | |
| ? { name: value.split('/').at(-1) ?? value, url: value } | |
| : undefined | |
| } | |
| const value = getFieldUrl(fieldState.value); | |
| const defaultValue = useMemo( | |
| () => | |
| value ? { name: value.split('/').at(-1) ?? value, url: value } : undefined, | |
| [value] | |
| ); | |
| return ( | |
| <FormFieldContainer {...containerProps}> | |
| <UploadInput | |
| {...rest} | |
| defaultValue={defaultValue} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/form/field-upload-input/index.tsx` around lines 30 - 40, The
field upload input rebuilds UploadInput.defaultValue on every render, which
breaks the cleared-state reference check in UploadInput and can make a cleared
file reappear. In the field-upload-input component, memoize the object passed as
defaultValue so the same reference is reused when the underlying URL value from
getFieldUrl(fieldState.value) has not changed; keep the logic localized around
the UploadInput props and the value-to-defaultValue mapping.
Comment on lines
+13
to
+16
| type FormFieldControllerCustomRender = (props: { | ||
| field: AnyFieldApi; | ||
| fieldState: AnyFieldApi['state']; | ||
| }) => ReactNode; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pass disabled through the custom render API.
disabled is accepted by FormFieldController, but for type="custom" it stays in fieldProps and is never exposed to render, making the common prop a no-op unless callers duplicate it manually.
Proposed fix
type FormFieldControllerCustomRender = (props: {
field: AnyFieldApi;
fieldState: AnyFieldApi['state'];
+ disabled?: boolean;
}) => ReactNode;
...
- if (type === 'custom') return customRender?.({ field, fieldState });
+ if (type === 'custom') {
+ return customRender?.({
+ field,
+ fieldState,
+ disabled:
+ typeof fieldProps.disabled === 'boolean'
+ ? fieldProps.disabled
+ : undefined,
+ });
+ }Also applies to: 84-85
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/form/form-field-controller/index.tsx` around lines 13 - 16,
The custom render API in FormFieldController is dropping the disabled prop, so
type="custom" callers never receive it in render. Update
FormFieldControllerCustomRender and the FormFieldController custom branch to
pass disabled through alongside field and fieldState, using the existing
FormFieldController / render / fieldProps flow so custom renderers can consume
the common prop directly.
Comment on lines
+26
to
34
| type FormFieldErrorProps<TFormData = ExplicitAny> = FormFieldErrorDisplayProps & | ||
| ( | ||
| | { | ||
| form: FormInstance<TFormData>; | ||
| name: DeepKeys<TFormData>; | ||
| } | ||
| // eslint-disable-next-line @typescript-eslint/no-empty-object-type | ||
| | {} | ||
| ); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Tighten the standalone props union.
The {} branch allows partial standalone usage such as form without name, which then falls through to the controller path and throws at runtime. Use never fields to enforce “both or neither”.
Proposed fix
type FormFieldErrorProps<TFormData = ExplicitAny> = FormFieldErrorDisplayProps &
(
| {
form: FormInstance<TFormData>;
name: DeepKeys<TFormData>;
}
- // eslint-disable-next-line `@typescript-eslint/no-empty-object-type`
- | {}
+ | {
+ form?: never;
+ name?: never;
+ }
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| type FormFieldErrorProps<TFormData = ExplicitAny> = FormFieldErrorDisplayProps & | |
| ( | |
| | { | |
| form: FormInstance<TFormData>; | |
| name: DeepKeys<TFormData>; | |
| } | |
| // eslint-disable-next-line @typescript-eslint/no-empty-object-type | |
| | {} | |
| ); | |
| type FormFieldErrorProps<TFormData = ExplicitAny> = FormFieldErrorDisplayProps & | |
| ( | |
| | { | |
| form: FormInstance<TFormData>; | |
| name: DeepKeys<TFormData>; | |
| } | |
| | { | |
| form?: never; | |
| name?: never; | |
| } | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/form/form-field-error.tsx` around lines 26 - 34, The
standalone props union in FormFieldErrorProps currently allows partial
combinations like form without name, which can slip into the controller path and
fail at runtime. Tighten the union by replacing the empty object branch with an
explicit “neither” shape using never fields so FormFieldErrorDisplayProps is
accepted only when both form and name are provided together or both are omitted.
Update the type alias in form-field-error.tsx accordingly, keeping the
FormFieldErrorProps and DeepKeys/FormInstance references aligned.
Comment on lines
+62
to
+64
| // Refetch session to update guards and redirect | ||
| session.refetch(); | ||
| }, |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Await the session refresh before completing submit.
isSubmitting clears as soon as onSubmit resolves; without await, users can resubmit while the session refresh/redirect guard is still pending.
Proposed fix
// Refetch session to update guards and redirect
- session.refetch();
+ await session.refetch();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Refetch session to update guards and redirect | |
| session.refetch(); | |
| }, | |
| // Refetch session to update guards and redirect | |
| await session.refetch(); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/auth/page-login-verify.tsx` around lines 62 - 64, The submit
flow in the login verification handler is resolving before the session refresh
finishes, which can let users resubmit too early. Update the `onSubmit` logic in
`page-login-verify.tsx` so `session.refetch()` is awaited before the handler
completes, ensuring `isSubmitting` stays true until the refresh and redirect
guard work is done.
Comment on lines
+43
to
46
| schema: zFormFieldsOnboarding(), | ||
| defaultValues: { | ||
| name: session.data?.user.name ?? '', | ||
| }, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve reactive hydration of the session name. defaultValues only applies on first render, so if session.data?.user.name arrives later the onboarding field can remain empty. Reset or re-seed the form when the session name changes, or delay rendering until the session is ready.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/auth/page-onboarding.tsx` around lines 43 - 46, The onboarding
form seeds `defaultValues` from `session.data?.user.name`, but that only runs
once, so late-arriving session data can leave the name field empty. Update the
`page-onboarding.tsx` form setup to react to session changes by
resetting/re-seeding the form when `session.data?.user.name` updates, or defer
rendering until the session is ready; use the `zFormFieldsOnboarding` form
initialization and the onboarding form component as the places to adjust.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces
react-hook-form(+@hookform/resolvers) with@tanstack/react-form@1.33.0while keeping the existing component surface (Form,FormField*,FormFieldController type="..."). Zod 4 is validated natively through Standard Schema — no adapter package.Core changes (
src/components/form/)useFormwrapper:useForm({ schema, mode?, defaultValues, onSubmit }). Built on TanStack'srevalidateLogicfor react-hook-form validation-timing parity (validate on submit/blur, revalidate on change after first submission).onSubmitreceivesschema.parse(value), so zod transforms (trim,'' → null) still apply exactly likezodResolverdid.FormFieldController:control={form.control}→form={form}. Submit handlers move intouseForm({ onSubmit });<Form form={form}>replaces<Form {...form} onSubmit>.{ field, fieldState, isInvalid }, wherefieldStateis auseStore-subscribed snapshot — keeps field components and custom renders reactive under React Compiler memoization.useFormContextprovided by the library (replaces the react-hook-form one).Formre-implements focus-first-invalid-field on failed submit.FormFieldErrorstandalone usage:control+name→form+name.Consumer mappings
form.setError(name, { message })form.setErrorMap({ onDynamic: { fields: { [name]: message } } })(clears on revalidation)form.formState.isDirty!useStore(form.store, (s) => s.isDefaultValue)(value-based parity)useWatch/form.formState.XuseStore(form.store, selector)values:optiondefaultValues(updates pristine fields on re-render)form.setValue(n, v, { shouldValidate })form.setFieldValue(n, v)Behavior change (deliberate)
Disabled fields now submit their current value — react-hook-form submitted
undefined. Browser specs updated accordingly.Also:
Inputgained a typednameprop (react-hook-form previously smuggled it through an untyped spread).Verification
tsc --noEmitcleanoxlintclean (0 warnings)vitest run: 16/16 files, 150/150 tests pass — including all 10 real-browser field-component specsGenerated with Claude
Summary by CodeRabbit
New Features
Bug Fixes