Refactor: Use Named Exports for Form Components and Import Types from Schema #28
Conversation
WalkthroughAdds exported SigninForm and SignupForm components using react-hook-form + Zod and action execution; page containers now delegate to these components. Changes remove default exports from FormInput/FormCheckbox and update signup schema's Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant SF as SigninForm
participant SA as signinAction
participant T as Toast
participant R as Router
U->>SF: Submit email/password
SF->>SF: Validate with Zod
alt Valid
SF->>SA: execute(credentials)
alt Success
SA-->>SF: OK
SF->>T: Show success toast
SF->>R: Navigate to "/"
else Error
SA-->>SF: serverError/fieldErrors
SF->>SF: Compose error message
SF->>T: Show error toast
end
else Invalid
SF->>T: Show validation errors
end
sequenceDiagram
autonumber
actor U as User
participant SUF as SignupForm
participant UA as signupAction
participant T as Toast
participant R as Router
U->>SUF: Submit signup fields
SUF->>SUF: Validate with Zod
alt Valid
SUF->>UA: execute(payload)
alt Success
UA-->>SUF: OK
SUF->>SUF: Reset form
SUF->>T: Success toast
SUF->>R: Navigate to "/"
else Error
UA-->>SUF: serverError/fieldErrors
SUF->>SUF: Compose error message
SUF->>T: Error toast
end
else Invalid
SUF->>T: Show validation errors
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (12)
src/components/models/auth/SigninForm.tsx (5)
35-38: Prefer replace() over push() after successful sign-in.Prevents the user from navigating back to the sign-in page post-auth.
- onSuccess: () => { + onSuccess: () => { toast.success("Signed in successfully!"); - router.push("/"); + router.replace("/"); },
65-65: Disable native browser validation to avoid double validation UX.Rely on Zod/RHF errors only.
- <form onSubmit={form.handleSubmit(execute)}> + <form noValidate onSubmit={form.handleSubmit(execute)}>Based on learnings
67-74: Add email autocomplete for better UX.Requires FormInput to forward autoComplete.
Proposed usage here (after updating FormInput to accept autoComplete):
<FormInput control={form.control} name="email" label="Email" placeholder="john@example.com" type="email" + autoComplete="email" required />Update FormInput component (outside this file) to support autoComplete:
export function FormInput<T extends FieldValues>({ placeholder, control, name, label, type = "text", required = false, helperText, endComponent, min, step, max, + autoComplete, }: { max?: number; step?: number; label: string; placeholder: string; control: Control<T>; name: Path<T>; type?: React.HTMLInputTypeAttribute; required?: boolean; helperText?: string; endComponent?: React.ReactNode; min?: number; + autoComplete?: string; }) { return ( <FormField control={control} name={name} render={({ field }) => ( <FormItem> <FormLabel> {label} {required && <span className="text-red-500">*</span>} </FormLabel> <div className="flex flex-row gap-2"> <FormControl> <Input placeholder={placeholder} {...field} min={min} max={max} step={step} + autoComplete={autoComplete} onChange={(e) => {
75-82: Add password autocomplete for better UX.After extending FormInput as above:
<FormInput control={form.control} name="password" label="Password" placeholder="Enter your password" type="password" + autoComplete="current-password" required />
40-49: Harden error aggregation to handle arrays safely.Prevents “[a,b]” stringification quirks and non-array cases.
- const fieldErrors = error.error.validationErrors?.fieldErrors; - const errorMessage = - error.error.serverError ?? - (fieldErrors - ? Object.entries(fieldErrors) - .map(([key, value]) => `${key}: ${value}`) - .join(", ") - : "An unknown error occurred"); + const fieldErrors = error.error.validationErrors?.fieldErrors; + const errorMessage = + error.error.serverError ?? + (fieldErrors + ? Object.entries(fieldErrors) + .map(([key, value]) => { + const messages = Array.isArray(value) ? value.join(", ") : String(value); + return `${key}: ${messages}`; + }) + .join(", ") + : "An unknown error occurred");src/components/models/auth/SignupForm.tsx (7)
39-42: Prefer replace() over push() after successful signup.Prevents navigating back to the signup page post-account creation.
- toast.success("Account created successfully!"); - form.reset(); - router.push("/"); + toast.success("Account created successfully!"); + form.reset(); + router.replace("/");
66-66: Disable native browser validation to avoid mixed HTML/RHF validation.Use Zod/RHF messages only.
- <form onSubmit={form.handleSubmit(execute)}> + <form noValidate onSubmit={form.handleSubmit(execute)}>Based on learnings
69-74: Add autocomplete for name.Requires FormInput to support autoComplete as suggested in SigninForm comment.
<FormInput control={form.control} name="name" label="Full Name" placeholder="John Doe" + autoComplete="name" required />
76-82: Add autocomplete for email.<FormInput control={form.control} name="email" label="Email" placeholder="john@example.com" type="email" + autoComplete="email" required />
84-90: Add autocomplete for password fields (new-password).Improves password manager behavior.
<FormInput control={form.control} name="password" label="Password" placeholder="Create a strong password" type="password" + autoComplete="new-password" required />
92-98: Add autocomplete for confirm password (new-password).<FormInput control={form.control} name="confirmPassword" label="Confirm Password" placeholder="Confirm your password" type="password" + autoComplete="new-password" required />
44-53: Harden error aggregation to join arrays safely.Same improvement as SigninForm.
- const fieldErrors = error.error.validationErrors?.fieldErrors; - const errorMessage = - error.error.serverError ?? - (fieldErrors - ? Object.entries(fieldErrors) - .map(([key, value]) => `${key}: ${value}`) - .join(", ") - : "An unknown error occurred"); + const fieldErrors = error.error.validationErrors?.fieldErrors; + const errorMessage = + error.error.serverError ?? + (fieldErrors + ? Object.entries(fieldErrors) + .map(([key, value]) => { + const messages = Array.isArray(value) ? value.join(", ") : String(value); + return `${key}: ${messages}`; + }) + .join(", ") + : "An unknown error occurred");
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/models/auth/SigninForm.tsx(1 hunks)src/components/models/auth/SignupForm.tsx(1 hunks)src/components/pages/signin/SigninPageContainer.tsx(1 hunks)src/components/pages/signup/SignupPageContainer.tsx(1 hunks)src/components/shared/Form/FormCheckbox.tsx(0 hunks)src/components/shared/Form/FormInput.tsx(0 hunks)
💤 Files with no reviewable changes (2)
- src/components/shared/Form/FormInput.tsx
- src/components/shared/Form/FormCheckbox.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
src/components/pages/signup/SignupPageContainer.tsx (1)
src/components/models/auth/SignupForm.tsx (1)
SignupForm(23-151)
src/components/models/auth/SigninForm.tsx (4)
src/actions/auth/signin/schema.ts (2)
SigninInput(8-8)signinSchema(3-6)src/actions/auth/signin/action.ts (1)
signinAction(7-32)src/components/shared/Form/FormInput.tsx (1)
FormInput(11-81)src/components/shared/Form/FormButton.tsx (1)
FormButton(11-27)
src/components/models/auth/SignupForm.tsx (5)
src/actions/auth/signup/schema.ts (2)
SignupInput(22-22)signupSchema(3-20)src/actions/auth/signup/action.ts (1)
signupAction(7-31)src/components/shared/Form/FormInput.tsx (1)
FormInput(11-81)src/components/shared/Form/FormCheckbox.tsx (1)
FormCheckbox(17-51)src/components/shared/Form/FormButton.tsx (1)
FormButton(11-27)
src/components/pages/signin/SigninPageContainer.tsx (1)
src/components/models/auth/SigninForm.tsx (1)
SigninForm(23-112)
🔇 Additional comments (5)
src/components/pages/signin/SigninPageContainer.tsx (1)
1-5: LGTM: Minimal wrapper with named import is correct.Container delegates cleanly to SigninForm and aligns with the named‑export refactor.
src/components/pages/signup/SignupPageContainer.tsx (1)
1-5: LGTM: Wrapper pattern and named import look good.Consistent with the refactor objectives.
src/components/models/auth/SigninForm.tsx (2)
17-19: Good: Uses named exports and imports types from schema.This aligns with the PR goals and improves type consistency.
1-22: No lingering default imports/exports or manual form-type definitions detectedsrc/components/models/auth/SignupForm.tsx (1)
16-19: Good: Named imports and schema-derived types.Matches the PR goals and reduces duplication.
| terms: true, | ||
| }, |
There was a problem hiding this comment.
Do not pre-check the Terms checkbox (compliance risk).
Pre-checked consent can violate GDPR/consent best practices; require explicit user action.
- terms: true,
+ terms: false,🤖 Prompt for AI Agents
In src/components/models/auth/SignupForm.tsx around lines 32-33 the terms
checkbox is pre-checked (terms: true), which violates consent best practices;
change the initial form value to false (terms: false), ensure the checkbox is
controlled to reflect that initial value, and add or ensure existing validation
blocks submission unless terms is explicitly checked by the user.
…message in signup schema
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/actions/auth/signup/schema.ts (1)
5-6: Normalize name/email inputs (trim and lowercase email).Improves validation UX and reduces backend normalization work.
Apply:
- name: z.string().min(1, 'Name is required'), - email: z.string().email('Invalid email format'), + name: z.string().trim().min(1, 'Name is required'), + email: z.string().trim().toLowerCase().email('Invalid email format'),src/components/models/auth/SignupForm.tsx (1)
44-51: Handle fieldErrors as arrays for clearer toasts.Zod/next-safe-action fieldErrors are typically string[]. Join them explicitly for readability.
- const errorMessage = - error.error.serverError ?? - (fieldErrors - ? Object.entries(fieldErrors) - .map(([key, value]) => `${key}: ${value}`) - .join(", ") - : "An unknown error occurred"); + const errorMessage = + error.error.serverError ?? + (fieldErrors + ? Object.entries(fieldErrors) + .map(([key, msgs]) => `${key}: ${(Array.isArray(msgs) ? msgs : [msgs]).join("; ")}`) + .join(" | ") + : "An unknown error occurred");
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/actions/auth/signup/schema.ts(1 hunks)src/components/models/auth/SignupForm.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/models/auth/SignupForm.tsx (5)
src/actions/auth/signup/schema.ts (2)
SignupInput(22-22)signupSchema(3-20)src/actions/auth/signup/action.ts (1)
signupAction(7-31)src/components/shared/Form/FormInput.tsx (1)
FormInput(11-81)src/components/shared/Form/FormCheckbox.tsx (1)
FormCheckbox(17-51)src/components/shared/Form/FormButton.tsx (1)
FormButton(11-27)
🔇 Additional comments (4)
src/actions/auth/signup/schema.ts (2)
13-15: LGTM: boolean + refine for terms is appropriate with RHF checkboxes.Using z.boolean().refine(...) keeps UX consistent and provides a clear message.
22-22: LGTM: exporting SignupInput from schema.Removes duplicated form value types and ensures type-source-of-truth.
src/components/models/auth/SignupForm.tsx (2)
27-33: Good: Terms default to false (addresses prior compliance concern).Prevents pre-checked consent; aligns with best practices.
16-18: No remaining default imports for FormInput/FormCheckbox Verified across all TS/TSX files; all usages migrated to named imports.
| <FormCheckbox | ||
| control={form.control} | ||
| name="terms" | ||
| label={ | ||
| <span> | ||
| I agree to the{" "} | ||
| <Link | ||
| href="/terms" | ||
| className="text-blue-600 hover:underline" | ||
| > | ||
| Terms of Service | ||
| </Link>{" "} | ||
| and{" "} | ||
| <Link | ||
| href="/privacy" | ||
| className="text-blue-600 hover:underline" | ||
| > | ||
| Privacy Policy | ||
| </Link> | ||
| </span> | ||
| } | ||
| required | ||
| /> |
There was a problem hiding this comment.
Checkbox onChange likely sets non-boolean values — fix in FormCheckbox.
Current FormCheckbox uses onChange={field.onChange} on a native checkbox. Controller expects you to pass checked boolean; otherwise RHF may store "on" or the event, breaking z.boolean() and preventing valid submission.
Update FormCheckbox:
- <input
+ <input
id={name}
type="checkbox"
className="h-4 w-4 text-blue-600 border-gray-300 rounded"
- checked={!!field.value}
- onChange={field.onChange}
+ checked={!!field.value}
+ onChange={(e) => field.onChange((e.target as HTMLInputElement).checked)}
ref={field.ref}
required={required}
/>File: src/components/shared/Form/FormCheckbox.tsx
📝 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.
| <FormCheckbox | |
| control={form.control} | |
| name="terms" | |
| label={ | |
| <span> | |
| I agree to the{" "} | |
| <Link | |
| href="/terms" | |
| className="text-blue-600 hover:underline" | |
| > | |
| Terms of Service | |
| </Link>{" "} | |
| and{" "} | |
| <Link | |
| href="/privacy" | |
| className="text-blue-600 hover:underline" | |
| > | |
| Privacy Policy | |
| </Link> | |
| </span> | |
| } | |
| required | |
| /> | |
| <input | |
| id={name} | |
| type="checkbox" | |
| className="h-4 w-4 text-blue-600 border-gray-300 rounded" | |
| checked={!!field.value} | |
| onChange={(e) => field.onChange((e.target as HTMLInputElement).checked)} | |
| ref={field.ref} | |
| required={required} | |
| /> |
🤖 Prompt for AI Agents
In src/components/shared/Form/FormCheckbox.tsx (update this file), the component
currently wires the native checkbox directly to field.onChange which passes the
event (or "on") into react-hook-form and causes non-boolean values to be stored;
change the input to call field.onChange(e.target.checked) and bind
checked={!!field.value} (defaulting to false) instead of spreading the whole
field onto the input for onChange/checked, and keep other props (onBlur, name,
ref) from field; this ensures RHF receives a boolean and zod/z.boolean()
validation works correctly.
Description:
This PR refactors the form components to improve code consistency and maintainability:
FormInputandFormCheckbox, switching to named exports only.SigninFormandSignupForm, importing types directly from their respective schema files.These changes help prevent import/export confusion, reduce code duplication, and align with modern ES6 best practices.
Summary by CodeRabbit
New Features
Refactor