feat: Add Sign In and Sign Up pages with form handling using React Hook Form #27
Conversation
WalkthroughIntroduces new auth pages and containers for sign-in and sign-up under (auth), removes legacy auth routes, adds form UI primitives and shared form components integrated with react-hook-form and Zod, updates signup/signin schemas and logic (type/name changes, password omission), and adds related dependencies in package.json. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as SignUpPageContainer
participant RHF as React Hook Form
participant Action as signupAction(useAction)
participant Server as Server Action
participant Toast as Toast (sonner)
participant Router as Router
User->>Page: Load / (auth)/signup
Page->>RHF: init form (zodResolver(signupSchema))
User->>RHF: Fill fields (name, email, password, confirm, terms)
User->>RHF: Submit
RHF->>Action: onSubmit(validated data)
Action->>Server: signup(input)
Server-->>Action: Result<Omit<User,'password'>> | Error
alt success
Action-->>Page: onSuccess(user)
Page->>Toast: success("Signed up")
Page->>RHF: reset()
Page->>Router: navigate("/")
else error
Action-->>Page: onError(message/field errors)
Page->>Toast: error(derived message)
end
sequenceDiagram
autonumber
actor User
participant Page as SigninPageContainer
participant RHF as React Hook Form
participant Action as signinAction(useAction)
participant Server as Server Action
participant Toast as Toast (sonner)
participant Router as Router
User->>Page: Load / (auth)/signin
Page->>RHF: init form (zodResolver(signinSchema))
User->>RHF: Enter email + password
User->>RHF: Submit
RHF->>Action: onSubmit(validated data)
Action->>Server: signin(input)
Server-->>Action: Result<Omit<User,'password'>> | Error
alt success
Action-->>Page: onSuccess(user)
Page->>Toast: success("Signed in")
Page->>Router: navigate("/")
else error
Action-->>Page: onError(message/field errors)
Page->>Toast: error(derived message)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 3
🧹 Nitpick comments (5)
src/app/(auth)/signin/page.tsx (2)
37-37: Don’t send unused fields; decide behavior for “Remember me”.
rememberisn’t part of the server schema. Either wire it to a “persistent session” behavior server-side or omit it from the action call.- await signinAction(data); + const { email, password } = data; + await signinAction({ email, password });Also applies to: 100-111
72-76: Small UX/accessibility polish.Add proper autocomplete and disable controls while submitting for better UX.
- <Input + <Input id="email" type="email" placeholder="john@example.com" + autoComplete="email" {...register("email", { required: "Email is required" })} /> ... - <Input + <Input id="password" type="password" placeholder="Enter your password" + autoComplete="current-password" {...register("password", { required: "Password is required" })} /> ... - <Button type="submit" className="w-full" disabled={isSubmitting}> + <Button type="submit" className="w-full" disabled={isSubmitting}> {isSubmitting ? "Signing in..." : "Sign in"} </Button>Optionally wrap form fields in
<fieldset disabled={isSubmitting}>…</fieldset>to block all inputs during submit.Also applies to: 88-93, 126-128
src/app/(auth)/signup/page.tsx (3)
41-41: Send only whitelisted fields to the action.Avoid sending
confirmPasswordandterms; pass{ name, email, password }only.- await signupAction(data); + const { name, email, password } = data; + await signupAction({ name, email, password });
146-149: Server-side enforcement of Terms (if required).Client-only required checkbox isn’t validated server-side. If ToS acceptance is a requirement, add it to the Zod schema and persist acceptance (timestamp, version) on the server.
88-94: Client-side validation enhancements (optional).Mirror server rules for better immediacy: add email pattern and password hints (min length/strength) via a resolver.
You can use
zodResolver(signupSchema)(add@hookform/resolvers), then drop ad-hoc rules:const form = useForm<SignUpFormValues>({ resolver: zodResolver(signupSchema) });Also applies to: 106-110, 124-132
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.json(1 hunks)src/actions/auth/signup/action.ts(1 hunks)src/app/(auth)/signin/page.tsx(1 hunks)src/app/(auth)/signup/page.tsx(1 hunks)src/app/auth/signin/page.tsx(0 hunks)src/app/auth/signup/page.tsx(0 hunks)
💤 Files with no reviewable changes (2)
- src/app/auth/signin/page.tsx
- src/app/auth/signup/page.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/actions/auth/signup/action.ts (1)
src/actions/auth/signup/schema.ts (1)
signupSchema(3-12)
src/app/(auth)/signin/page.tsx (3)
src/actions/auth/signin/action.ts (1)
signinAction(7-32)src/app/auth/signin/page.tsx (1)
SignInPage(3-5)src/actions/auth/signin/logic.ts (1)
signin(10-44)
src/app/(auth)/signup/page.tsx (3)
src/app/auth/signup/page.tsx (1)
SignUpPage(3-5)src/components/RegisterForm.tsx (2)
RegisterForm(17-187)e(25-74)src/actions/auth/signup/logic.ts (1)
Signup(10-38)
🔇 Additional comments (3)
src/actions/auth/signup/action.ts (1)
5-9: Schema import/usage change looks good.Import rename aligns with
signupSchemaand the action now binds the correct schema.src/app/(auth)/signin/page.tsx (1)
112-117: Broken route — confirm/update forgot-password linkNo page found at /auth/forgot-password; only occurrence is src/app/(auth)/signin/page.tsx:112-117 (href="/auth/forgot-password"). Update the href to the correct route or add the missing page and verify routing.
package.json (1)
32-32: Add @hookform/resolvers (if using Zod) and verify react-hook-form@^7.63.0 compatibility with React 19 / Next.js 15package.json: react-hook-form@^7.63.0
- If mirroring server-side Zod, add @hookform/resolvers and wire zodResolver.
- react-hook-form v7.63 is generally usable with React 19/Next 15 but may emit peer-dependency warnings and has watch/useWatch caveats — test in your app-router; if warnings persist use package-manager overrides or upgrade to a v8+ release that lists React 19 in peerDeps.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
src/app/(auth)/signin/page.tsx (8)
112-117: Verify forgot‑password route (likely wrong path).With route groups, URL shouldn’t include the folder name. If the page lives under app/(auth)/forgot-password, the path should be “/forgot-password”, not “/auth/forgot-password”.
- <Link - href="/auth/forgot-password" + <Link + href="/forgot-password" className="text-sm text-blue-600 hover:underline" > Forgot password? </Link>
62-62: Disable native HTML validation to avoid double messages.Let RHF own validation.
- <form onSubmit={handleSubmit(onSubmit)}> + <form noValidate onSubmit={handleSubmit(onSubmit)}>
71-76: Add accessibility and autofill hints to email field.Improves SR feedback and browser autofill.
<Input id="email" type="email" placeholder="john@example.com" - {...register("email", { required: "Email is required" })} + autoComplete="email" + aria-invalid={!!errors.email} + {...register("email", { required: "Email is required" })} />
88-93: Add accessibility and autofill hints to password field.<Input id="password" type="password" placeholder="Enter your password" - {...register("password", { required: "Password is required" })} + autoComplete="current-password" + aria-invalid={!!errors.password} + {...register("password", { required: "Password is required" })} />
119-123: Mark error container for assistive tech.Adds live region so errors are announced.
- {errors.root && ( - <div className="text-red-600 text-sm text-center"> + {errors.root && ( + <div className="text-red-600 text-sm text-center" role="alert" aria-live="polite"> {errors.root.message} </div> )}
34-39: Don’t send unused fields to the server action (schema compatibility).If signinSchema is strict, extra keys (remember) can cause validation errors. Send only what the action expects.
- await signinAction(data); + const { email, password } = data; + await signinAction({ email, password });
126-128: Minor a11y: expose busy state on submit.Optional: helps SR users.
- <Button type="submit" className="w-full" disabled={isSubmitting}> + <Button type="submit" className="w-full" disabled={isSubmitting} aria-busy={isSubmitting}> {isSubmitting ? "Signing in..." : "Sign in"} </Button>
15-16: Optional: add client‑side schema resolver for parity with server.Use your shared signinSchema with zodResolver so client and server validations match. Adjust import path to where the schema lives.
// imports import { zodResolver } from "@hookform/resolvers/zod"; import { signinSchema } from "@/schemas/auth/signin"; // adjust path // useForm const { register, handleSubmit, formState: { errors, isSubmitting }, setError, clearErrors } = useForm<SignInFormValues>({ resolver: zodResolver(signinSchema) });Also applies to: 24-33
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/(auth)/signin/page.tsx(1 hunks)src/app/(auth)/signup/page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/(auth)/signup/page.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/(auth)/signin/page.tsx (1)
src/actions/auth/signin/action.ts (1)
signinAction(7-32)
🔇 Additional comments (1)
src/app/(auth)/signin/page.tsx (1)
130-136: Sign up link path fix confirmed.Link correctly points to “/signup”. Thanks for addressing the earlier comment.
…ement reusable form components
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
src/components/shared/Form/FormInput/FormInput.tsx (1)
5-12: Optional: Support autocomplete passthroughAdding an optional
autoCompleteprop improves UX for email/password fields.type FormInputProps<T extends FieldValues> = { control: Control<T>; name: Path<T>; label: string; placeholder?: string; type?: string; required?: boolean; + autoComplete?: string; }; export function FormInput<T extends FieldValues>({ control, name, label, placeholder, type = "text", required = false, + autoComplete, }: FormInputProps<T>) { ... <Input {...field} placeholder={placeholder} type={type} required={required} + autoComplete={autoComplete} />Also applies to: 14-21, 32-37
src/components/pages/signup/SignupPageContainer.tsx (1)
40-47: Safer formatting of field errors (array handling)
fieldErrors[key]is typically a string array. Interpolating${value}relies on Array.toString. Join explicitly.- (fieldErrors - ? Object.entries(fieldErrors) - .map(([key, value]) => `${key}: ${value}`) - .join(", ") - : "An unknown error occurred"); + (fieldErrors + ? Object.entries(fieldErrors) + .map(([key, value]) => + `${key}: ${Array.isArray(value) ? value.join(", ") : String(value)}` + ) + .join(", ") + : "An unknown error occurred");Optionally, also set inline field errors:
// inside onError: if (fieldErrors) { for (const [key, value] of Object.entries(fieldErrors)) { form.setError(key as keyof SignUpFormValues, { type: "server", message: Array.isArray(value) ? value.join(", ") : String(value), }); } }src/components/pages/signin/SigninPageContainer.tsx (1)
42-46: Safer formatting of field errors (array handling)Explicitly join arrays for clearer messaging.
- (fieldErrors - ? Object.entries(fieldErrors) - .map(([key, value]) => `${key}: ${value}`) - .join(", ") - : "An unknown error occurred"); + (fieldErrors + ? Object.entries(fieldErrors) + .map(([key, value]) => + `${key}: ${Array.isArray(value) ? value.join(", ") : String(value)}` + ) + .join(", ") + : "An unknown error occurred");src/components/shared/Form/FormButton/FormButton.tsx (1)
19-25: Optional: Add aria-busy for a11yExpose loading state to AT.
<Button type={type} disabled={disabled || loading} className={className} + aria-busy={loading || undefined} >src/actions/auth/signup/schema.ts (1)
5-7: Trim and normalize inputs at the schema layerTrimming avoids UX friction; lowercasing email aligns with server normalization.
- 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/ui/form.tsx (2)
110-121: Expose aria-errormessage for better error associationAdd aria-errormessage to explicitly associate the control with the error element when invalid.
<Slot data-slot="form-control" id={formItemId} aria-describedby={ !error ? `${formDescriptionId}` : `${formDescriptionId} ${formMessageId}` } aria-invalid={!!error} + aria-errormessage={error ? formMessageId : undefined} {...props} />
146-155: Announce validation errors to screen readersMake error messages live regions so SR users are notified when errors appear.
return ( <p data-slot="form-message" id={formMessageId} className={cn("text-destructive text-sm", className)} + role="alert" + aria-live="polite" {...props} > {body} </p> )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
package.json(2 hunks)src/actions/auth/signup/schema.ts(1 hunks)src/app/(auth)/signin/page.tsx(1 hunks)src/app/(auth)/signup/page.tsx(1 hunks)src/components/pages/signin/SigninPageContainer.tsx(1 hunks)src/components/pages/signup/SignupPageContainer.tsx(1 hunks)src/components/shared/Form/FormButton/FormButton.tsx(1 hunks)src/components/shared/Form/FormCheckbox/FormCheckbox.tsx(1 hunks)src/components/shared/Form/FormInput/FormInput.tsx(1 hunks)src/components/ui/form.tsx(1 hunks)src/components/ui/label.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/(auth)/signin/page.tsx
- src/app/(auth)/signup/page.tsx
🧰 Additional context used
🧬 Code graph analysis (8)
src/components/shared/Form/FormCheckbox/FormCheckbox.tsx (1)
src/components/ui/form.tsx (5)
FormField(166-166)FormItem(161-161)FormControl(163-163)FormLabel(162-162)FormMessage(165-165)
src/components/shared/Form/FormButton/FormButton.tsx (1)
src/components/ui/button.tsx (3)
Button(50-50)props(16-46)ButtonProps(4-13)
src/actions/auth/signup/schema.ts (3)
src/actions/auth/signup/logic.ts (1)
Signup(10-38)src/components/RegisterForm.tsx (4)
RegisterForm(17-187)e(157-157)e(25-74)e(142-142)src/actions/auth/signup/action.ts (1)
result(10-31)
src/components/pages/signin/SigninPageContainer.tsx (4)
src/actions/auth/signin/schema.ts (1)
signinSchema(3-6)src/actions/auth/signin/action.ts (1)
signinAction(7-32)src/components/shared/Form/FormInput/FormInput.tsx (1)
FormInput(14-44)src/components/shared/Form/FormButton/FormButton.tsx (1)
FormButton(11-27)
src/components/pages/signup/SignupPageContainer.tsx (5)
src/actions/auth/signup/schema.ts (1)
signupSchema(3-20)src/actions/auth/signup/action.ts (1)
signupAction(7-31)src/components/shared/Form/FormInput/FormInput.tsx (1)
FormInput(14-44)src/components/shared/Form/FormCheckbox/FormCheckbox.tsx (1)
FormCheckbox(17-51)src/components/shared/Form/FormButton/FormButton.tsx (1)
FormButton(11-27)
src/components/shared/Form/FormInput/FormInput.tsx (2)
src/components/ui/form.tsx (5)
FormField(166-166)FormItem(161-161)FormLabel(162-162)FormControl(163-163)FormMessage(165-165)src/components/ui/input.tsx (2)
Input(21-21)Input(5-19)
src/components/ui/label.tsx (1)
src/lib/utils.ts (1)
cn(4-6)
src/components/ui/form.tsx (2)
src/lib/utils.ts (1)
cn(4-6)src/components/ui/label.tsx (1)
Label(24-24)
🔇 Additional comments (3)
package.json (1)
18-22: Deps additions look correct for RHF + Radix + toastsVersions and additions align with the new form components/pages.
Also applies to: 35-37
src/components/ui/label.tsx (1)
8-22: LGTMRadix Label wrapper is concise and consistent with your
cnpattern.src/components/ui/form.tsx (1)
19-44: Solid form composition and RHF integrationGood abstraction around RHF with FormProvider alias, Controller-wrapped FormField, and well-wired IDs/ARIA via contexts.
| <input | ||
| id={name} | ||
| type="checkbox" | ||
| className="h-4 w-4 text-blue-600 border-gray-300 rounded" | ||
| checked={!!field.value} | ||
| onChange={field.onChange} | ||
| ref={field.ref} | ||
| required={required} | ||
| /> |
There was a problem hiding this comment.
Fix RHF checkbox wiring and label association (blocks form submission)
onChange={field.onChange} forwards the event (value = "on"), not the boolean checked state. This will make terms be "on" instead of true, causing the zod z.literal(true) check to fail and preventing signup. Also, setting id/htmlFor to name can desync with FormItem’s generated id, breaking label association.
Apply this diff:
- <input
- id={name}
- type="checkbox"
- className="h-4 w-4 text-blue-600 border-gray-300 rounded"
- checked={!!field.value}
- onChange={field.onChange}
- ref={field.ref}
- required={required}
- />
+ <input
+ type="checkbox"
+ className="h-4 w-4 text-blue-600 border-gray-300 rounded"
+ name={field.name}
+ checked={!!field.value}
+ onChange={(e) => field.onChange(e.target.checked)}
+ onBlur={field.onBlur}
+ ref={field.ref}
+ required={required}
+ />- <FormLabel htmlFor={name} className="text-sm text-gray-600">
+ <FormLabel className="text-sm text-gray-600">
{label}
{required && <span className="text-destructive ml-1">*</span>}
</FormLabel>Also applies to: 41-44
🤖 Prompt for AI Agents
In src/components/shared/Form/FormCheckbox/FormCheckbox.tsx around lines 31-39
(and similarly lines 41-44), the checkbox is wired incorrectly:
onChange={field.onChange} forwards the whole event (yielding "on") and the id is
set to the plain name which can desync with FormItem's generated id. Fix by
calling onChange with the checked boolean: onChange={e =>
field.onChange(e.target.checked)} (retain checked={!!field.value} and
ref={field.ref}), and set the input's id to the FormItem-generated id prop (or
the component's id prop) instead of name so label htmlFor and input id stay in
sync.
| const FormFieldContext = React.createContext<FormFieldContextValue>( | ||
| {} as FormFieldContextValue | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix context defaults and guards to prevent silent mis-wiring
Both contexts default to an object cast, so the runtime guard in useFormField never triggers, leading to undefined names/ids (e.g., "undefined-form-item") when components are mis-nested. Initialize contexts with undefined and add a guard for FormItemContext. Also avoid reading fieldContext.name before verifying by passing an optional name to useFormState.
Apply this diff:
-const FormFieldContext = React.createContext<FormFieldContextValue>(
- {} as FormFieldContextValue
-)
+const FormFieldContext = React.createContext<
+ FormFieldContextValue | undefined
+>(undefined)
@@
-const useFormField = () => {
- const fieldContext = React.useContext(FormFieldContext)
- const itemContext = React.useContext(FormItemContext)
- const { getFieldState } = useFormContext()
- const formState = useFormState({ name: fieldContext.name })
- const fieldState = getFieldState(fieldContext.name, formState)
-
- if (!fieldContext) {
- throw new Error("useFormField should be used within <FormField>")
- }
-
- const { id } = itemContext
-
- return {
- id,
- name: fieldContext.name,
- formItemId: `${id}-form-item`,
- formDescriptionId: `${id}-form-item-description`,
- formMessageId: `${id}-form-item-message`,
- ...fieldState,
- }
-}
+const useFormField = () => {
+ const fieldContext = React.useContext(FormFieldContext)
+ const itemContext = React.useContext(FormItemContext)
+ const { getFieldState } = useFormContext()
+ // name is optional here to keep hooks unconditionally called
+ const formState = useFormState({ name: fieldContext?.name as any })
+
+ if (!fieldContext) {
+ throw new Error("useFormField must be used within <FormField>")
+ }
+ if (!itemContext) {
+ throw new Error("useFormField must be used within <FormItem>")
+ }
+
+ const fieldState = getFieldState(fieldContext.name, formState)
+ const { id } = itemContext
+
+ return {
+ id,
+ name: fieldContext.name,
+ formItemId: `${id}-form-item`,
+ formDescriptionId: `${id}-form-item-description`,
+ formMessageId: `${id}-form-item-message`,
+ ...fieldState,
+ }
+}
@@
-const FormItemContext = React.createContext<FormItemContextValue>(
- {} as FormItemContextValue
-)
+const FormItemContext = React.createContext<
+ FormItemContextValue | undefined
+>(undefined)Also applies to: 45-66, 72-75
🤖 Prompt for AI Agents
In src/components/ui/form.tsx around lines 28-31 (also applies to lines 45-66
and 72-75), the FormFieldContext and FormItemContext are initialized with a
forced object cast which prevents runtime guards from detecting mis-nesting;
change both contexts to be created with undefined as the default (i.e.,
React.createContext<FormFieldContextValue | undefined>(undefined)) and update
their types accordingly, add a runtime guard in useFormField to throw or warn
when FormItemContext is undefined before accessing its properties, and modify
useFormState calls to accept an optional name so you don't read
fieldContext.name until after the context presence is verified. Ensure any
downstream code checks for undefined context values or uses non-null assertions
only after the guard passes.
…e form components with default values and improved validation
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/auth/signup/logic.ts (1)
25-33: Handle unique constraint races on email (P2002).Between check and insert, concurrent signups can throw a unique constraint error. Catch and map to a safe error.
- const user = await prisma.user.create({ - data: { - name, - email: normalisedEmail, - password: hashedPassword - } - }); + let user: User; + try { + user = await prisma.user.create({ + data: { + name, + email: normalisedEmail, + password: hashedPassword + } + }); + } catch (e: any) { + if (e?.code === 'P2002' && e?.meta?.target?.includes('email')) { + console.error('Signup error: Email already exists (race)'); + return error('Something went wrong'); + } + console.error('Signup error:', e); + return error('Something went wrong'); + }
🧹 Nitpick comments (6)
src/actions/auth/signup/logic.ts (1)
15-23: Fix typos in variable and log message.Minor polish.
- const exisitingUser = await prisma.user.findUnique({ + const existingUser = await prisma.user.findUnique({ @@ - if (exisitingUser) { - console.error('Singup error: User with this email already exists') + if (existingUser) { + console.error('Signup error: User with this email already exists') return error('Something went wrong') };src/components/shared/Form/FormInput/FormInput.tsx (1)
53-67: Avoid setting null into RHF value; guard NaN and keep input controlled.Using
nullcan produce React controlled/uncontrolled warnings; also guardNaN.- onChange={(e) => { - if (type === "number") { - const value = e.target.value; - if (value === "" || value === null || value === undefined) { - field.onChange(null); - } else { - field.onChange(Number(value)); - } - } else { - field.onChange(e.target.value); - } - }} + onChange={(e) => { + if (type === "number") { + const raw = e.currentTarget.value; + if (raw === "") { + field.onChange(undefined); + } else { + const next = Number(raw); + field.onChange(Number.isNaN(next) ? undefined : next); + } + } else { + field.onChange(e.currentTarget.value); + } + }} type={type} required={required} + value={field.value ?? ""}src/actions/auth/signup/schema.ts (1)
3-16: Trim and normalize inputs at the schema level.Prevents whitespace-only names and enforces lowercase emails consistently.
- 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/actions/auth/signin/logic.ts (1)
26-31: Avoid non-null assertion on password.Handle missing password defensively to prevent runtime errors.
- const isValidPassword = await bcrypt.compare(password, user.password!); + if (!user.password) { + console.error('signin error: Missing password hash'); + return error('Invalid credentials'); + } + const isValidPassword = await bcrypt.compare(password, user.password);src/components/pages/signup/SignupPageContainer.tsx (2)
46-56: Flatten fieldErrors arrays for clearer toasts.Current join prints arrays verbatim; flatten messages by field.
- onError: (error) => { - 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"); - toast.error(errorMessage); - }, + onError: (error) => { + const fieldErrors = error.error.validationErrors?.fieldErrors as Record<string, string[] | undefined> | undefined; + const messages = + fieldErrors + ? Object.entries(fieldErrors).flatMap(([key, vals]) => + (vals ?? []).map((v) => `${key}: ${v}`) + ) + : []; + const errorMessage = error.error.serverError ?? (messages.length ? messages.join(", ") : "An unknown error occurred"); + toast.error(errorMessage); + },
68-101: Optional: surface server field errors inline via RHF.Also call
form.setErrorper field soFormMessageshows details.
- In onError, when
fieldErrorsexists:
- Iterate keys and
form.setError(key as any, { type: "server", message }).- Keep the toast for a general notice.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/actions/auth/signin/logic.ts(1 hunks)src/actions/auth/signin/schema.ts(1 hunks)src/actions/auth/signup/logic.ts(1 hunks)src/actions/auth/signup/schema.ts(1 hunks)src/app/(auth)/signup/page.tsx(1 hunks)src/components/pages/signin/SigninPageContainer.tsx(1 hunks)src/components/pages/signup/SignupPageContainer.tsx(1 hunks)src/components/shared/Form/FormButton/FormButton.tsx(1 hunks)src/components/shared/Form/FormInput/FormInput.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/pages/signin/SigninPageContainer.tsx
- src/components/shared/Form/FormButton/FormButton.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
src/actions/auth/signin/logic.ts (2)
src/actions/auth/signin/schema.ts (1)
SigninInput(8-8)src/lib/result.ts (1)
Result(5-5)
src/app/(auth)/signup/page.tsx (1)
src/components/pages/signup/SignupPageContainer.tsx (1)
SignupPageContainer(26-154)
src/actions/auth/signup/logic.ts (2)
src/actions/auth/signup/schema.ts (1)
SignupInput(22-22)src/lib/result.ts (1)
Result(5-5)
src/components/pages/signup/SignupPageContainer.tsx (5)
src/actions/auth/signup/schema.ts (1)
signupSchema(3-20)src/actions/auth/signup/action.ts (1)
signupAction(7-31)src/components/shared/Form/FormInput/FormInput.tsx (1)
FormInput(11-81)src/components/shared/Form/FormCheckbox/FormCheckbox.tsx (1)
FormCheckbox(17-51)src/components/shared/Form/FormButton/FormButton.tsx (1)
FormButton(11-27)
🔇 Additional comments (2)
src/app/(auth)/signup/page.tsx (1)
3-7: Thin wrapper looks good.src/actions/auth/signin/schema.ts (1)
8-8: Type alias rename is consistent.
src/actions/auth/signup/logic.ts
Outdated
| import { SignupInput } from './schema'; | ||
|
|
||
| export async function Signup(input: signupInput): Promise<Result<User>> { | ||
| export async function Signup(input: SignupInput): Promise<Result<User>> { |
There was a problem hiding this comment.
Do not return the password hash; omit it from the result.
Returning User leaks the hashed password to clients via the server action.
-export async function Signup(input: SignupInput): Promise<Result<User>> {
+export async function Signup(input: SignupInput): Promise<Result<Omit<User, 'password'>>> {
@@
- return success(user);
+ // Omit password before returning
+ // eslint-disable-next-line @typescript-eslint/no-unused-vars
+ const { password: _password, ...userWithoutPassword } = user;
+ return success(userWithoutPassword);Also applies to: 37-38
🤖 Prompt for AI Agents
In src/actions/auth/signup/logic.ts around lines 10 and 37-38, the Signup
function currently returns a User which includes the password hash; remove that
sensitive field from the action result by returning a sanitized user shape
(e.g., change the returned type to Result<PublicUser> or Result<Omit<User,
'passwordHash'>>) and ensure you copy/return only safe properties (id, email,
name, createdAt, etc.). Update the function to strip or exclude passwordHash
before resolving, and update any related type annotations/usages at lines 37-38
to accept the sanitized user type.
…ports in related files; add FormInput and FormCheckbox components
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/actions/auth/signup/action.ts(1 hunks)src/actions/auth/signup/logic.ts(2 hunks)src/app/(auth)/signin/page.tsx(1 hunks)src/app/(auth)/signup/page.tsx(1 hunks)src/components/pages/signin/SigninPageContainer.tsx(1 hunks)src/components/pages/signup/SignupPageContainer.tsx(1 hunks)src/components/shared/Form/FormButton.tsx(1 hunks)src/components/shared/Form/FormCheckbox.tsx(1 hunks)src/components/shared/Form/FormInput.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/app/(auth)/signup/page.tsx
- src/app/(auth)/signin/page.tsx
- src/actions/auth/signup/action.ts
- src/components/pages/signin/SigninPageContainer.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
src/components/shared/Form/FormButton.tsx (3)
src/components/ui/button.tsx (3)
Button(50-50)props(16-46)ButtonProps(4-13)src/components/RegisterForm.tsx (1)
RegisterForm(17-187)src/components/LoginForm.tsx (1)
LoginForm(18-111)
src/components/shared/Form/FormInput.tsx (2)
src/components/ui/form.tsx (5)
FormField(166-166)FormItem(161-161)FormLabel(162-162)FormControl(163-163)FormMessage(165-165)src/components/ui/input.tsx (2)
Input(21-21)Input(5-19)
src/components/shared/Form/FormCheckbox.tsx (1)
src/components/ui/form.tsx (5)
FormField(166-166)FormItem(161-161)FormControl(163-163)FormLabel(162-162)FormMessage(165-165)
src/actions/auth/signup/logic.ts (3)
src/actions/auth/signup/schema.ts (1)
SignupInput(22-22)src/lib/result.ts (2)
Result(5-5)success(10-15)src/actions/auth/signin/logic.ts (1)
signin(10-44)
src/components/pages/signup/SignupPageContainer.tsx (5)
src/actions/auth/signup/schema.ts (1)
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 (2)
src/actions/auth/signup/logic.ts (1)
37-40: Thanks for sanitizing the returned user object.Dropping the
passwordfield before returning the user keeps the server action from leaking sensitive hashes while preserving the rest of the payload—looks solid.src/components/shared/Form/FormCheckbox.tsx (1)
31-44: Fix checkbox wiring so RHF receives a boolean and the label stays associated
Forwarding the raw change event (onChange={field.onChange}) makes the controller store the string"on"instead of a boolean. Users can’t uncheck the box, the zodz.literal(true)check fails, and signup is effectively blocked. Also, forcingid/htmlFortonamecan desync with theFormItem-generated id, breaking accessibility. Convert the handler to passevent.target.checked, keeponBlur, and letFormControlinject the id so the label stays linked.<FormControl> <input - id={name} type="checkbox" className="h-4 w-4 text-blue-600 border-gray-300 rounded" - checked={!!field.value} - onChange={field.onChange} - ref={field.ref} + name={field.name} + checked={!!field.value} + onChange={(event) => field.onChange(event.target.checked)} + onBlur={field.onBlur} + ref={field.ref} required={required} /> </FormControl> - <FormLabel htmlFor={name} className="text-sm text-gray-600"> + <FormLabel className="text-sm text-gray-600">
| name: "", | ||
| email: "", | ||
| password: "", | ||
| confirmPassword: "", | ||
| terms: true, | ||
| }, |
There was a problem hiding this comment.
Don’t pre-check the terms checkbox
defaultValues.terms being true means the consent checkbox is pre-selected. That’s a compliance risk (opt-in must be explicit) and, combined with the checkbox wiring bug, the user can’t actually toggle it off. Default this to false so the user actively confirms.
password: "",
confirmPassword: "",
- terms: true,
+ terms: false,🤖 Prompt for AI Agents
In src/components/pages/signup/SignupPageContainer.tsx around lines 31 to 36,
defaultValues.terms is set to true which pre-checks the consent checkbox; change
it to false so the checkbox is not pre-selected and the user must actively opt
in, then verify the checkbox control is correctly wired to the form state
(value/checked and onChange/Controller) so users can toggle it.
| import { Button } from "@/components/ui/button"; | ||
|
|
||
| type FormButtonProps = { | ||
| type?: "button" | "submit" | "reset"; | ||
| loading?: boolean; | ||
| disabled?: boolean; | ||
| className?: string; | ||
| children: React.ReactNode; | ||
| }; |
There was a problem hiding this comment.
Add the missing React type import to fix compilation
FormButtonProps references React.ReactNode, but React isn’t imported anywhere in this module. TypeScript will raise “Cannot find namespace 'React'” and block the build. Pull in the type explicitly (or swap to a direct ReactNode import) before merging.
+import type { ReactNode } from "react";
import { Button } from "@/components/ui/button";
type FormButtonProps = {
type?: "button" | "submit" | "reset";
loading?: boolean;
disabled?: boolean;
className?: string;
- children: React.ReactNode;
+ children: ReactNode;
};📝 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.
| import { Button } from "@/components/ui/button"; | |
| type FormButtonProps = { | |
| type?: "button" | "submit" | "reset"; | |
| loading?: boolean; | |
| disabled?: boolean; | |
| className?: string; | |
| children: React.ReactNode; | |
| }; | |
| import type { ReactNode } from "react"; | |
| import { Button } from "@/components/ui/button"; | |
| type FormButtonProps = { | |
| type?: "button" | "submit" | "reset"; | |
| loading?: boolean; | |
| disabled?: boolean; | |
| className?: string; | |
| children: ReactNode; | |
| }; |
🤖 Prompt for AI Agents
In src/components/shared/Form/FormButton.tsx lines 1-9, the props use
React.ReactNode but the module doesn't import React, causing a TypeScript
"Cannot find namespace 'React'". Fix by importing the React types: either add an
import { ReactNode } from "react" and change the prop to ReactNode, or add
import * as React from "react" (or import React from "react") so React.ReactNode
is defined; update the type usage accordingly and save.
| onChange={(e) => { | ||
| if (type === "number") { | ||
| const value = e.target.value; | ||
| if (value === "" || value === null || value === undefined) { | ||
| field.onChange(null); | ||
| } else { | ||
| field.onChange(Number(value)); | ||
| } | ||
| } else { | ||
| field.onChange(e.target.value); | ||
| } | ||
| }} |
There was a problem hiding this comment.
Remove number coercion in onChange.
Casting every numeric keystroke to Number (and pushing null) makes inputs unusable: entering -, 3., or other intermediate states immediately becomes NaN/truncated and collapses the user’s value. React also warns when you write null back to a controlled input. Let the field keep the raw string and let the schema (e.g., Zod coercion) handle conversion instead.
- onChange={(e) => {
- if (type === "number") {
- const value = e.target.value;
- if (value === "" || value === null || value === undefined) {
- field.onChange(null);
- } else {
- field.onChange(Number(value));
- }
- } else {
- field.onChange(e.target.value);
- }
- }}
+ onChange={(e) => field.onChange(e.target.value)}🤖 Prompt for AI Agents
In src/components/shared/Form/FormInput.tsx around lines 53 to 64, remove the
number coercion and null assignment in the onChange handler: do not call
Number(...) or field.onChange(null) on every keystroke; instead always pass the
raw string value (e.target.value) to field.onChange so the input can represent
intermediate states like "-" or "3." and avoid React controlled-input null
warnings, leaving numeric conversion/validation to the schema/coercion layer.
Description:
This PR refactors the sign-in and sign-up pages to fully utilize React Hook Form for form state management, validation, and error handling. It removes unnecessary useState hooks and leverages setError for server-side errors. Both forms now submit data to server actions, improving type safety and user experience. Additionally, client-side redirects are handled after successful authentication. This update results in cleaner, more maintainable, and more robust authentication flows.
Summary by CodeRabbit
New Features
Refactor
Chores