[FEAT] 회원가입 입력 폼 - #9
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough클라이언트 측 회원가입 흐름이 추가되었습니다. 폼 스키마·검증, 상태 훅, React Query 뮤테이션, 가입 API 호출 및 응답 Zod 검증, UI(다이얼로그) 연동, 이메일 검증·OAuth 흐름, 그리고 ApiError에 응답 데이터 추가가 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as "UI (로그인/회원가입 다이얼로그)"
participant Hook as "useSignupForm"
participant Mutation as "useSignupMutation / signup"
participant BaseApi as "baseApi / ApiError"
participant Server as "Backend"
User->>UI: 입력 및 버튼 클릭
UI->>Hook: handleChange / handleBlur
Hook->>Hook: 필드 검증 (validateSignupField)
User->>UI: 이메일 검증 클릭
UI->>Hook: handleVerifyEmail()
Hook->>Server: 이메일 검증 요청 (또는 시뮬레이션)
Server-->>Hook: 검증 응답
Hook->>UI: isEmailVerified 업데이트
User->>UI: 폼 제출
UI->>Hook: handleSubmit()
Hook->>Hook: 전체 검증 (validateSignupForm, isEmailVerified 확인)
Hook->>Mutation: mutateAsync(signupRequest)
Mutation->>BaseApi: POST /api/auth/signup
BaseApi->>Server: HTTP 요청 전송
alt 성공
Server-->>BaseApi: 200 응답 (payload)
BaseApi-->>Mutation: 파싱된 응답 (zod)
Mutation-->>Hook: 성공 결과 반환
Hook->>UI: 상태 초기화, onSuccess 호출
else 실패
Server-->>BaseApi: 오류 응답 (code/message/data)
BaseApi-->>Mutation: ApiError(..., data)
Mutation-->>Hook: 에러 전달
Hook->>UI: fieldErrors 및 top-level error 업데이트
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
kkhhmm3103
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
src/features/auth-dialog/model/signup.schema.ts (2)
52-55: 비밀번호 검증이 길이만 확인합니다.현재 8자 이상만 요구합니다. 보안 요구사항에 따라 대/소문자, 숫자, 특수문자 조합을 요구하는 것을 고려해 보세요.
♻️ 강화된 비밀번호 검증 예시
case 'password': if (!value) return '비밀번호를 입력해주세요.' if (value.length < 8) return '비밀번호는 8자 이상 입력해주세요.' + if (!/[A-Z]/.test(value)) return '비밀번호에 대문자를 포함해주세요.' + if (!/[a-z]/.test(value)) return '비밀번호에 소문자를 포함해주세요.' + if (!/[0-9]/.test(value)) return '비밀번호에 숫자를 포함해주세요.' return ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth-dialog/model/signup.schema.ts` around lines 52 - 55, The password validation in the 'password' case of signup.schema.ts only checks length; update the validator (the switch branch handling 'password') to also require at least one uppercase, one lowercase, one digit, and one special character (use regex tests), return clear localized error messages for each failing rule (or a combined message), and ensure the final success path still returns an empty string; reference the 'password' case in the signup validator function to locate and modify the logic.
35-68: Zod가 프로젝트 의존성에 있으므로 활용을 권장합니다.라이브러리 컨텍스트에 따르면 Zod 4.3.6이 설치되어 있습니다. 수동 검증 로직 대신 Zod 스키마를 사용하면 타입 추론과 검증을 통합하고 코드 중복을 줄일 수 있습니다.
♻️ Zod 스키마 예시
import { z } from 'zod' export const signupSchema = z.object({ nickname: z.string() .trim() .min(1, '닉네임을 입력해주세요.') .min(2, '닉네임은 2자 이상 입력해주세요.') .max(20, '닉네임은 20자 이하로 입력해주세요.'), email: z.string() .min(1, '이메일을 입력해주세요.') .email('올바른 이메일 형식을 입력해주세요.'), password: z.string() .min(1, '비밀번호를 입력해주세요.') .min(8, '비밀번호는 8자 이상 입력해주세요.'), confirmPassword: z.string() .min(1, '비밀번호 확인을 입력해주세요.'), inviteCode: z.string().optional() }).refine(data => data.password === data.confirmPassword, { message: '비밀번호가 일치하지 않습니다.', path: ['confirmPassword'] }) export type SignupFormData = z.infer<typeof signupSchema>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth-dialog/model/signup.schema.ts` around lines 35 - 68, The current manual validator validateSignupField should be replaced with a Zod schema to consolidate validation and types: create and export signupSchema (import { z } from 'zod') that defines nickname, email, password, confirmPassword, and optional inviteCode with the exact messages and constraints from the existing logic, add a .refine to ensure password === confirmPassword with the '비밀번호가 일치하지 않습니다.' message, and export SignupFormData as z.infer<typeof signupSchema>; then remove or deprecate validateSignupField and ensure any callers use the Zod parse/safeParse results (or map Zod errors to the previous UI messages) instead of relying on emailRegex or per-field switch logic.src/features/auth-dialog/api/signup.ts (1)
18-18:response.json()이 실패할 수 있습니다.서버가 500 에러나 HTML 페이지를 반환하는 경우
response.json()이 예외를 발생시킬 수 있습니다. 호출부(use-signup-form.ts)에서try-catch로 감싸고 있지만, 여기서도 방어적으로 처리하는 것이 좋습니다.♻️ 제안된 수정
- const result: ApiResponse<SignupResponseData | Record<string, string>> = await response.json() + let result: ApiResponse<SignupResponseData | Record<string, string>> + try { + result = await response.json() + } catch { + throw new Error('Invalid JSON response from server') + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth-dialog/api/signup.ts` at line 18, response.json()가 파싱 실패(예: 500 페이지, HTML 응답) 시 예외를 던질 수 있으니 응답 처리 블록에서 방어적으로 처리하세요: signup API 코드의 response.json() 호출을 try-catch로 감싸고, JSON 파싱에 실패하면 response.text()로 원본문을 읽어 ApiResponse<SignupResponseData | Record<string,string>> 형태의 result에 에러 메시지(예: { error: text })나 적절한 폼으로 할당해 반환하도록 수정하세요; 관련 식별자: response, result, ApiResponse, SignupResponseData, use-signup-form.ts.src/features/auth-dialog/model/use-signup-form.ts (2)
104-118: 이메일 인증 로직이 데모/플레이스홀더입니다.현재 구현은
setTimeout으로 지연만 시뮬레이션하고window.alert를 사용합니다. 프로덕션 배포 전에 실제 이메일 인증 API와 연동이 필요합니다.실제 이메일 인증 API 연동 구현을 도와드릴까요? 또는 이 작업을 추적할 이슈를 생성해 드릴까요?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth-dialog/model/use-signup-form.ts` around lines 104 - 118, The current email verification flow in use-signup-form.ts uses a setTimeout and window.alert as a demo; replace this placeholder in the function containing the try/catch (where setIsVerifying, setIsEmailVerified, setFieldErrors, and window.alert are used) with a real async call to your email verification API (await verifyEmail(tokenOrCode)), properly handle success by calling setIsEmailVerified(true) and removing field errors, handle failure by parsing and passing server error messages into setError, ensure setIsVerifying(false) runs in finally, and remove window.alert in favor of in-app UI feedback (toast or state); if you can't implement it now, add a TODO comment and create/tracking issue instead.
29-62:setFormData콜백 내에서setFieldErrors호출은 예상치 못한 동작을 유발할 수 있습니다.React의 state 업데이트 콜백 내에서 다른 state를 업데이트하면 클로저 문제나 비동기 배칭으로 인한 예상치 못한 동작이 발생할 수 있습니다. 상태 업데이트를 분리하는 것이 더 안전합니다.
♻️ 상태 업데이트 분리 예시
const handleChange = (field: SignupFieldName) => (e: React.ChangeEvent<HTMLInputElement>) => { const value = e.target.value + const nextFormData = { + ...formData, + [field]: value + } - setFormData((prev) => { - const nextFormData = { - ...prev, - [field]: value - } + setFormData(nextFormData) - if (field === 'email') { - setIsEmailVerified(false) - } + if (field === 'email') { + setIsEmailVerified(false) + } - setFieldErrors((prevErrors) => { - // ... validation logic - }) + if (touchedFields[field]) { + const nextErrors = { ...fieldErrors } + const message = validateSignupField(field, value, nextFormData) + if (message) nextErrors[field] = message + else delete nextErrors[field] - return nextFormData - }) + if (field === 'password' && touchedFields.confirmPassword) { + const confirmMessage = validateSignupField('confirmPassword', nextFormData.confirmPassword, nextFormData) + if (confirmMessage) nextErrors.confirmPassword = confirmMessage + else delete nextErrors.confirmPassword + } + setFieldErrors(nextErrors) + } if (error) { setError('') } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth-dialog/model/use-signup-form.ts` around lines 29 - 62, Inside the setFormData updater (the callback passed to setFormData) you must not call setFieldErrors; instead compute the new form data first (using the same merge logic currently in the callback) and call setFormData with that result, then separately call setFieldErrors (using its functional updater) after setFormData returns, reusing validateSignupField, touchedFields, and checking the password/confirmPassword logic and setIsEmailVerified(field==='email') outside the setFormData callback so there are no nested state updates or closure/batching issues.src/features/auth-dialog/ui/login-dialog.tsx (3)
73-81: 접근성 개선을 위해<label>요소 추가를 권장합니다.현재 입력 필드에
placeholder만 있고<label>요소가 없습니다. 스크린 리더 사용자를 위해 라벨을 추가하고, 에러 메시지는aria-describedby로 연결하는 것이 좋습니다.♻️ 예시 (닉네임 필드)
<div> + <label htmlFor="nickname" className="sr-only">닉네임</label> <input + id="nickname" type="text" placeholder="닉네임" required value={formData.nickname} onChange={handleChange('nickname')} onBlur={handleBlur('nickname')} className={inputClassName('nickname')} + aria-describedby={fieldErrors.nickname ? 'nickname-error' : undefined} /> {fieldErrors.nickname ? ( - <p className="mt-1 text-sm text-red-500">{fieldErrors.nickname}</p> + <p id="nickname-error" className="mt-1 text-sm text-red-500" role="alert">{fieldErrors.nickname}</p> ) : null} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth-dialog/ui/login-dialog.tsx` around lines 73 - 81, Add a proper <label> for the nickname input and connect any error text via aria-describedby: update the JSX around the input with a <label> element referencing the input (use a stable id like "nickname-input") and set the input's id to match; keep the existing props (value={formData.nickname}, onChange={handleChange('nickname')}, onBlur={handleBlur('nickname')}, className={inputClassName('nickname')}) but add aria-describedby pointing to the error element's id (e.g., "nickname-error") so screen readers announce validation messages.
166-181: OAuth 버튼에 로딩 상태가 없습니다.
handleOAuth는 비동기 함수이지만 버튼에 로딩 상태가 표시되지 않습니다. 사용자가 여러 번 클릭하거나 URL 로딩 중임을 인지하지 못할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth-dialog/ui/login-dialog.tsx` around lines 166 - 181, The Google and Kakao buttons call the async handleOAuth but have no loading state or disabling, so add a local loading state (e.g., oauthLoading or oauthLoadingProvider) in the login-dialog component and wrap the calls to handleOAuth with a setter: set loading true before awaiting handleOAuth(provider) and set false in finally; use that state to disable the two button elements and render a loading indicator or change button text (e.g., "Signing in..." or a spinner) while loading to prevent double clicks and indicate progress; update references in the two button elements that currently call handleOAuth('google') and handleOAuth('kakao') to call the new wrapper function that manages the loading state.
10-10: 컴포넌트 이름과 실제 기능 간의 불일치가 있습니다.컴포넌트 이름은
LoginDialog이고 트리거 버튼 텍스트는 "로그인"이지만, 실제 폼은 회원가입 기능을 수행합니다. 사용자 경험과 코드 가독성을 위해 이름을 일치시키는 것이 좋습니다.-function LoginDialog() { +function SignupDialog() {또는 트리거 버튼 텍스트를 "회원가입"으로 변경하세요:
- 로그인 + 회원가입Also applies to: 38-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth-dialog/ui/login-dialog.tsx` at line 10, The component name LoginDialog mismatches its behavior (it renders a signup form) and its trigger text; update either the component name or the UI text to match intent: rename the component (e.g., SignupDialog) and adjust exported/used references, or change the trigger button label from "로그인" to "회원가입" (also update the similar trigger at the other occurrence referenced around lines 38-39); ensure the component's props/handlers (e.g., submit handlers inside LoginDialog) and any external imports/usages are renamed/updated accordingly so names and behavior are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/auth-dialog/model/signup.schema.ts`:
- Around line 18-23: ApiResponse<T> currently requires data but error responses
may omit or set it null; update the ApiResponse<T> type (the interface named
ApiResponse<T> in signup.schema.ts) so its data property is optional or nullable
(e.g., data?: T | null) and then adjust callers like the usage of result.data in
use-signup-form (where result is typed as ApiResponse<...>) to handle possible
undefined/null values before accessing properties.
In `@src/features/auth-dialog/model/use-signup-form.ts`:
- Around line 182-196: In handleOAuth, check the fetch response status before
parsing JSON: verify response.ok and if false throw a descriptive Error (include
status and statusText or body message) instead of throwing an empty Error; also
include that error.message when calling setError so failures show useful
information (refer to the handleOAuth function, the response and result
variables, and the setError call).
---
Nitpick comments:
In `@src/features/auth-dialog/api/signup.ts`:
- Line 18: response.json()가 파싱 실패(예: 500 페이지, HTML 응답) 시 예외를 던질 수 있으니 응답 처리 블록에서
방어적으로 처리하세요: signup API 코드의 response.json() 호출을 try-catch로 감싸고, JSON 파싱에 실패하면
response.text()로 원본문을 읽어 ApiResponse<SignupResponseData | Record<string,string>>
형태의 result에 에러 메시지(예: { error: text })나 적절한 폼으로 할당해 반환하도록 수정하세요; 관련 식별자:
response, result, ApiResponse, SignupResponseData, use-signup-form.ts.
In `@src/features/auth-dialog/model/signup.schema.ts`:
- Around line 52-55: The password validation in the 'password' case of
signup.schema.ts only checks length; update the validator (the switch branch
handling 'password') to also require at least one uppercase, one lowercase, one
digit, and one special character (use regex tests), return clear localized error
messages for each failing rule (or a combined message), and ensure the final
success path still returns an empty string; reference the 'password' case in the
signup validator function to locate and modify the logic.
- Around line 35-68: The current manual validator validateSignupField should be
replaced with a Zod schema to consolidate validation and types: create and
export signupSchema (import { z } from 'zod') that defines nickname, email,
password, confirmPassword, and optional inviteCode with the exact messages and
constraints from the existing logic, add a .refine to ensure password ===
confirmPassword with the '비밀번호가 일치하지 않습니다.' message, and export SignupFormData
as z.infer<typeof signupSchema>; then remove or deprecate validateSignupField
and ensure any callers use the Zod parse/safeParse results (or map Zod errors to
the previous UI messages) instead of relying on emailRegex or per-field switch
logic.
In `@src/features/auth-dialog/model/use-signup-form.ts`:
- Around line 104-118: The current email verification flow in use-signup-form.ts
uses a setTimeout and window.alert as a demo; replace this placeholder in the
function containing the try/catch (where setIsVerifying, setIsEmailVerified,
setFieldErrors, and window.alert are used) with a real async call to your email
verification API (await verifyEmail(tokenOrCode)), properly handle success by
calling setIsEmailVerified(true) and removing field errors, handle failure by
parsing and passing server error messages into setError, ensure
setIsVerifying(false) runs in finally, and remove window.alert in favor of
in-app UI feedback (toast or state); if you can't implement it now, add a TODO
comment and create/tracking issue instead.
- Around line 29-62: Inside the setFormData updater (the callback passed to
setFormData) you must not call setFieldErrors; instead compute the new form data
first (using the same merge logic currently in the callback) and call
setFormData with that result, then separately call setFieldErrors (using its
functional updater) after setFormData returns, reusing validateSignupField,
touchedFields, and checking the password/confirmPassword logic and
setIsEmailVerified(field==='email') outside the setFormData callback so there
are no nested state updates or closure/batching issues.
In `@src/features/auth-dialog/ui/login-dialog.tsx`:
- Around line 73-81: Add a proper <label> for the nickname input and connect any
error text via aria-describedby: update the JSX around the input with a <label>
element referencing the input (use a stable id like "nickname-input") and set
the input's id to match; keep the existing props (value={formData.nickname},
onChange={handleChange('nickname')}, onBlur={handleBlur('nickname')},
className={inputClassName('nickname')}) but add aria-describedby pointing to the
error element's id (e.g., "nickname-error") so screen readers announce
validation messages.
- Around line 166-181: The Google and Kakao buttons call the async handleOAuth
but have no loading state or disabling, so add a local loading state (e.g.,
oauthLoading or oauthLoadingProvider) in the login-dialog component and wrap the
calls to handleOAuth with a setter: set loading true before awaiting
handleOAuth(provider) and set false in finally; use that state to disable the
two button elements and render a loading indicator or change button text (e.g.,
"Signing in..." or a spinner) while loading to prevent double clicks and
indicate progress; update references in the two button elements that currently
call handleOAuth('google') and handleOAuth('kakao') to call the new wrapper
function that manages the loading state.
- Line 10: The component name LoginDialog mismatches its behavior (it renders a
signup form) and its trigger text; update either the component name or the UI
text to match intent: rename the component (e.g., SignupDialog) and adjust
exported/used references, or change the trigger button label from "로그인" to
"회원가입" (also update the similar trigger at the other occurrence referenced
around lines 38-39); ensure the component's props/handlers (e.g., submit
handlers inside LoginDialog) and any external imports/usages are renamed/updated
accordingly so names and behavior are consistent.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7fdafef3-a16f-4249-a035-df6f0763f547
📒 Files selected for processing (4)
src/features/auth-dialog/api/signup.tssrc/features/auth-dialog/model/signup.schema.tssrc/features/auth-dialog/model/use-signup-form.tssrc/features/auth-dialog/ui/login-dialog.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/auth-dialog/model/use-signup-mutation.ts`:
- Around line 5-8: The signup mutation currently returns useMutation({
mutationFn: signup }) but related error handling in use-signup-form.ts only
checks axios.isAxiosError(error), which misses ApiError instances produced by
base-api.ts; update the catch/error handling in use-signup-form.ts to also check
for error instanceof ApiError (importing ApiError from shared/api/base-api) and
branch accordingly to read error.code, error.message and validation payloads,
falling back to the existing axios.isAxiosError(error) handling for other cases
so structured ApiError details are preserved.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 54564781-3cd4-4798-8029-83fa6d18d23c
📒 Files selected for processing (4)
package.jsonsrc/features/auth-dialog/api/signup.tssrc/features/auth-dialog/model/use-signup-form.tssrc/features/auth-dialog/model/use-signup-mutation.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/features/auth-dialog/api/signup.ts
- src/features/auth-dialog/model/use-signup-form.ts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/features/auth-dialog/model/use-signup-form.ts (1)
279-280:⚠️ Potential issue | 🟡 MinorOAuth 실패 원인이 UI에서 다시 숨겨집니다.
위에서 만든 상세 오류 메시지를
catch {}가 버려서, 결국 사용자에게는 항상 같은 문구만 보입니다. 에러를 받아error.message를 함께 노출해야 문제 원인을 추적하기 쉽습니다.🔧 Proposed change
- } catch { - setError(`${provider} 로그인 URL을 가져오는 데 실패했습니다.`) + } catch (error) { + const message = error instanceof Error ? error.message : '알 수 없는 오류입니다.' + setError(`${provider} 로그인 URL을 가져오는 데 실패했습니다. ${message}`) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth-dialog/model/use-signup-form.ts` around lines 279 - 280, The catch block in use-signup-form.ts swallows the thrown error and always sets a generic message; update the catch to accept the error (e) and include its message when calling setError so users/logs see the real cause (e.g., setError(`${provider} 로그인 URL을 가져오는 데 실패했습니다: ${ (e as Error)?.message ?? String(e) }`)). Locate the try/catch around the OAuth URL fetch in the function where setError and provider are used, capture the exception variable, and include its message (or a safe stringified fallback) in the setError call.
🧹 Nitpick comments (2)
src/features/auth-dialog/model/use-signup-form.ts (1)
59-97: state updater 안에서 다른setState를 호출하지 마세요.
setFormDataupdater 내부에서setIsEmailVerified와setFieldErrors를 같이 호출하고 있습니다. React는 updater를 순수 함수로 가정하므로, 특히 Strict Mode에서 이 로직이 중복 평가되면 검증과 에러 갱신이 두 번씩 실행될 수 있습니다. 다음 form 값을 먼저 계산한 뒤, 다른 상태 갱신은 updater 바깥으로 분리하는 편이 안전합니다.♻️ Possible refactor
const handleChange = (field: SignupFieldName) => (e: React.ChangeEvent<HTMLInputElement>) => { const value = e.target.value + const nextFormData = { + ...formData, + [field]: value + } - setFormData((prev) => { - const nextFormData = { - ...prev, - [field]: value - } - // ... - return nextFormData - }) + setFormData(nextFormData) + // setIsEmailVerified / setFieldErrors는 nextFormData 기준으로 여기서 처리🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth-dialog/model/use-signup-form.ts` around lines 59 - 97, The updater passed to setFormData currently calls other state setters inside it (setIsEmailVerified and setFieldErrors), which can run multiple times in Strict Mode; instead compute nextFormData purely inside the setFormData updater (using the prev value and the [field]: value merge) and return it, then immediately after that setFormData call perform the other updates: if field === 'email' call setIsEmailVerified(false), and recompute field errors using validateSignupField and touchedFields against the same nextFormData and call setFieldErrors with the new errors (including revalidating confirmPassword when field === 'password' and touchedFields.confirmPassword); keep all logic that derives values but not the setFormData return outside the updater and use the same nextFormData for consistency.src/shared/api/base-api.ts (1)
28-33:ApiError.status는 payload보다 HTTP 응답에서 읽는 편이 안전합니다.지금은
error.response.data.status를 그대로 넣고 있어서, 에러 payload에status가 없거나 실제 HTTP 상태와 다른 값을 담으면ApiError.status가 틀어질 수 있습니다.status는error.response.status에서 읽고, payload에서는code/message/data만 꺼내는 쪽이 더 견고합니다.🔧 Proposed change
- const { status, code, message, data } = error.response.data + const { status } = error.response + const { code, message, data } = error.response.data // 서버 표준 에러 포맷일 경우 ApiError로 래핑 if (code && message) { return Promise.reject(new ApiError(status, code, message, data ?? null)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/api/base-api.ts` around lines 28 - 33, The ApiError is being constructed using status from the response payload which can be missing or inconsistent; update the axios error handling (the block guarded by axios.isAxiosError(error) in base-api.ts) to read HTTP status from error.response.status and only extract code, message, and data from error.response.data, then pass that HTTP status into the ApiError constructor (the class/constructor named ApiError) while keeping code/message/data from the payload (use data ?? null for the data arg).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/auth-dialog/model/use-signup-form.ts`:
- Around line 263-270: The fetch call in useSignupForm that directly uses
fetch(`/api/auth/${provider}/url`) bypasses the shared VITE_API_BASE_URL; change
it to use the same base URL logic from the shared API (e.g., import and use the
VITE_API_BASE_URL constant or the baseApi URL builder from
shared/api/base-api.ts) so the request becomes
`${VITE_API_BASE_URL}/api/auth/${provider}/url` (or use the shared helper
function) and preserve the existing response.ok/json handling; update the import
and the fetch call inside the function that builds the OAuth URL accordingly.
- Around line 230-233: The server error-to-form mapping currently uses "key in
initialSignupFormData" which accepts inherited properties; change this to an
own-property check (e.g., use
Object.prototype.hasOwnProperty.call(initialSignupFormData, key)) before
assigning into mappedErrors so only actual form fields are mapped; update the
block around Object.entries(serverErrors) that assigns to mappedErrors and keep
the type cast to SignupFieldName as before.
---
Duplicate comments:
In `@src/features/auth-dialog/model/use-signup-form.ts`:
- Around line 279-280: The catch block in use-signup-form.ts swallows the thrown
error and always sets a generic message; update the catch to accept the error
(e) and include its message when calling setError so users/logs see the real
cause (e.g., setError(`${provider} 로그인 URL을 가져오는 데 실패했습니다: ${ (e as
Error)?.message ?? String(e) }`)). Locate the try/catch around the OAuth URL
fetch in the function where setError and provider are used, capture the
exception variable, and include its message (or a safe stringified fallback) in
the setError call.
---
Nitpick comments:
In `@src/features/auth-dialog/model/use-signup-form.ts`:
- Around line 59-97: The updater passed to setFormData currently calls other
state setters inside it (setIsEmailVerified and setFieldErrors), which can run
multiple times in Strict Mode; instead compute nextFormData purely inside the
setFormData updater (using the prev value and the [field]: value merge) and
return it, then immediately after that setFormData call perform the other
updates: if field === 'email' call setIsEmailVerified(false), and recompute
field errors using validateSignupField and touchedFields against the same
nextFormData and call setFieldErrors with the new errors (including revalidating
confirmPassword when field === 'password' and touchedFields.confirmPassword);
keep all logic that derives values but not the setFormData return outside the
updater and use the same nextFormData for consistency.
In `@src/shared/api/base-api.ts`:
- Around line 28-33: The ApiError is being constructed using status from the
response payload which can be missing or inconsistent; update the axios error
handling (the block guarded by axios.isAxiosError(error) in base-api.ts) to read
HTTP status from error.response.status and only extract code, message, and data
from error.response.data, then pass that HTTP status into the ApiError
constructor (the class/constructor named ApiError) while keeping
code/message/data from the payload (use data ?? null for the data arg).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fab8462a-c356-4523-92c6-14748120d3a6
📒 Files selected for processing (3)
src/features/auth-dialog/model/signup.schema.tssrc/features/auth-dialog/model/use-signup-form.tssrc/shared/api/base-api.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/auth-dialog/model/use-signup-form.ts`:
- Around line 55-57: When the email field changes and when any async
email-verification completes, tie both to a single request token/id so only the
latest verification result can update isEmailVerified; create a mutable
latestEmailVerificationId (ref or state) that you increment/replace when field
=== 'email' (alongside setIsEmailVerified(false)), attach the current id to each
outgoing verifyEmail request, and in the async response handler (the place(s)
that currently call setIsEmailVerified(true/false) — search for
setIsEmailVerified and the email verification request handlers in
use-signup-form.ts, including the block around lines 109-147) only apply the
result if the response id matches latestEmailVerificationId, ignoring/stale
responses otherwise.
- Around line 241-244: In use-signup-form.ts, update the OAuth popup flow to
explicitly handle popup-blocked cases by checking the return value of
window.open after calling it (the code block that currently calls
window.open(url, 'oauth_popup', 'width=600,height=700') inside the try). If
window.open returns null, call setError with a user-facing message like
`${provider} 로그인 팝업이 차단되었습니다.` (or similar), and avoid treating the flow as
successful; otherwise continue with the existing success logic. Ensure you
reference the same setError function and the existing provider/url variables so
behavior and messaging are consistent.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c592166c-ae64-42f4-a9f9-4b18e4b76c2d
📒 Files selected for processing (1)
src/features/auth-dialog/model/use-signup-form.ts
Comment on lines
+55
to
+57
| if (field === 'email') { | ||
| setIsEmailVerified(false) | ||
| } |
There was a problem hiding this comment.
이메일 인증 결과가 최신 이메일과 분리되어 적용되는 레이스를 막아주세요.
현재는 인증 요청이 진행 중일 때 이메일을 바꿔도, 이전 요청이 늦게 끝나면 isEmailVerified가 true로 덮여 최신 이메일이 미인증인데도 통과될 수 있습니다. 요청 무효화 토큰(또는 request id)로 최신 요청만 반영되게 처리하는 게 안전합니다.
🛠️ 제안 수정
-import { useState } from 'react'
+import { useRef, useState } from 'react'
@@
export function useSignupForm({ onSuccess }: UseSignupFormParams) {
+ const emailVerifyRequestIdRef = useRef(0)
@@
// 이메일이 바뀌면 기존 인증 상태 초기화
if (field === 'email') {
setIsEmailVerified(false)
+ emailVerifyRequestIdRef.current += 1 // 진행 중 인증 요청 무효화
}
@@
const handleVerifyEmail = async () => {
+ const requestId = ++emailVerifyRequestIdRef.current
const emailError = validateSignupField('email', formData.email, formData)
@@
try {
// TODO: 실제 이메일 인증 API로 교체
await new Promise((resolve) => setTimeout(resolve, 1500))
+ if (requestId !== emailVerifyRequestIdRef.current) return
setIsEmailVerified(true)
@@
} catch {
+ if (requestId !== emailVerifyRequestIdRef.current) return
setError('이메일 인증 중 오류가 발생했습니다.')
} finally {
- setIsVerifying(false)
+ if (requestId === emailVerifyRequestIdRef.current) {
+ setIsVerifying(false)
+ }
}
}Also applies to: 109-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/auth-dialog/model/use-signup-form.ts` around lines 55 - 57, When
the email field changes and when any async email-verification completes, tie
both to a single request token/id so only the latest verification result can
update isEmailVerified; create a mutable latestEmailVerificationId (ref or
state) that you increment/replace when field === 'email' (alongside
setIsEmailVerified(false)), attach the current id to each outgoing verifyEmail
request, and in the async response handler (the place(s) that currently call
setIsEmailVerified(true/false) — search for setIsEmailVerified and the email
verification request handlers in use-signup-form.ts, including the block around
lines 109-147) only apply the result if the response id matches
latestEmailVerificationId, ignoring/stale responses otherwise.
Contributor
Author
There was a problem hiding this comment.
이메일 인증 구현 후 반영 예정
There was a problem hiding this comment.
The PR description image shows the wefin signup dialog with fields for nickname, email (with an "인증하기" verification button), password, password confirmation, and optional invite code, along with Google/Kakao OAuth buttons.
@kkhhmm3103, 이해했습니다! 실제 이메일 인증 API 연동 시 레이스 컨디션이 발생할 수 있으니, 그때 emailVerifyRequestIdRef를 활용한 요청 무효화 처리를 함께 반영해 주시면 좋을 것 같습니다. 추후 잊지 않도록 별도 이슈로 등록해 드릴까요?
gaeunnlee
approved these changes
Apr 1, 2026
gaeunnlee
left a comment
gaeunnlee
left a comment
Member
There was a problem hiding this comment.
계층 분리가 깔끔하여 구조를 파악하기 쉬웠습니다! 구현 수고하셨습니다😊
Comment on lines
+8
to
+17
| export const signupResponseSchema = z.object({ | ||
| status: z.number(), | ||
| code: z.string(), | ||
| message: z.string(), | ||
| data: z.object({ | ||
| userId: z.string(), | ||
| email: z.string(), | ||
| nickname: z.string() | ||
| }) | ||
| }) |
Member
There was a problem hiding this comment.
현재 signupResponseSchema가 백엔드 공통 응답 포맷과 맞지 않아 정상 응답이 반환되더라도 Zod 검증 단계에서 실패하면서 화면에서는 에러로 표시됩니다!
이미 공통 응답을 처리하는 apiResponseSchema 헬퍼가 있으니 개별 API에서 응답 구조를 다시 정의하기보다 해당 헬퍼를 재사용하는 방식으로 통일하면 좋을 것 같습니다!
export const signupResponseSchema = apiResponseSchema(
z.object({
userId: z.string(),
email: z.string(),
nickname: z.string()
})
)
Contributor
Author
There was a problem hiding this comment.
apiResponse를 사용하도록 수정했습니다!
Comment on lines
+33
to
+35
| export async function signup(request: SignupRequest) { | ||
| const response = await baseApi.post('/api/auth/signup', request) | ||
|
|
Member
There was a problem hiding this comment.
baseApi의 baseURL에 이미 /api가 포함되어 있어서 실제 요청이 /api/api/auth/signup으로 나갑니다! /auth/signup으로 변경하면 될 것 같아요!
Contributor
Author
There was a problem hiding this comment.
baseURL과 중복되지 않도록 /auth/signup로 수정했습니다!! 감사합니다~
Comment on lines
+10
to
+13
| export class ApiError<T = unknown> extends Error { | ||
| readonly status: number | ||
| readonly code: string | ||
| readonly data: T | null |
Member
There was a problem hiding this comment.
ApiError에 data 제네릭을 추가해서 필드별 서버 에러를 폼에 매핑하는 구조가 좋네요!👍
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
2 participants
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.
📌 PR 설명
회원가입 모달의 입력 폼, 검증 로직, API 연동까지 전체 흐름을 구현.
✅ 완료한 기능 명세
📸 스크린샷
💭 고민과 해결과정
단순 submit 시 검증만으로는 UX가 부족하다고 판단하여 실시간(onChange), blur, submit 시점을 모두 고려한 구조로 설계.
백엔드에서 내려오는 validation 에러를 단순 메시지로 출력하지 않고 각 필드에 매핑하여 사용자에게 명확하게 보여주도록 구현.
Summary by CodeRabbit
새로운 기능
테스트
잡무
개선