[페이먼츠 3단계 - MSW & Async & Testing] 콘티(조건형) 미션 제출합니다.#551
Open
iftype wants to merge 22 commits into
Open
Conversation
- 카드번호 엔티티는 자신의 무결성을 지켜야한다 - 참조값은 기능의 함수(model)로 전부 분리
- 포맷은 중복되더라도 기능별로 들고있는다( 입력과 프리뷰의 포맷이 다를 수 있기때문)
- 엔티티는 무결성을 지키기위헤 에러메세지를 제거
- handleChange => onChange - 오탈자 수정
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (12)
src/pages/result/Result.module.css-18-24 (1)
18-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
messageBox클래스가 없어 메시지 묶음 스타일이 적용되지 않습니다.
Result.tsx에서styles.messageBox를 사용하고 있어, 여기에도 해당 클래스를 정의해 레이아웃 의도를 맞춰주세요.수정 예시
+.messageBox { + display: flex; + flex-direction: column; + gap: 4px; +} + .message { font-size: 25px; font-weight: 700;🤖 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/pages/result/Result.module.css` around lines 18 - 24, The CSS file is missing the .messageBox class referenced as styles.messageBox in Result.tsx, so add a .messageBox selector to src/pages/result/Result.module.css that provides the intended layout (e.g., container width, padding, margin, display/alignments) to wrap .message; update or match any existing naming (ensure .message remains) so Result.tsx's styles.messageBox works correctly.src/entities/card/model/expiryDate.ts-11-15 (1)
11-15:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
isValidMonth의 기본 반환값이 함수 의미와 어긋납니다.Line 11-15에서 길이가 1/2가 아니면
true를 반환해 빈 문자열이나 3자리 문자열도 유효 월로 판단됩니다. 기본 반환값을false로 두는 쪽이 함수 의미와 일치합니다.🤖 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/entities/card/model/expiryDate.ts` around lines 11 - 15, The isValidMonth function currently returns true for any month string whose length is not 1 or 2, which wrongly treats empty or 3+ char strings as valid; change its default behavior so that if month.length is neither 1 nor 2 the function returns false, and keep the existing regex checks for length 1 (allow "0" or "1") and length 2 (allow "01".."12") in the isValidMonth function to ensure only valid month inputs pass.src/entities/card/model/password.ts-3-4 (1)
3-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win비밀번호 검증이 길이만 확인합니다.
Line 3-4는 숫자 여부를 확인하지 않아 문자 2자리도 유효 처리될 수 있습니다. 숫자 문자열 조건을 함께 검증해 주세요.
🤖 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/entities/card/model/password.ts` around lines 3 - 4, validatePassword currently only checks length (PASSWORD_LENGTH) and therefore allows non-numeric strings; update validatePassword to ensure the input is numeric and exactly PASSWORD_LENGTH long by verifying both password.length === PASSWORD_LENGTH and that the string contains only digits (e.g., using a digit-only check/regex) before returning true.src/entities/card/model/cardNumber.ts-60-64 (1)
60-64:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win카드번호 검증에 숫자 형식 확인이 빠져 있습니다.
Line 60-64는 브랜드/길이만 확인해서 숫자가 아닌 문자가 섞여도 유효 처리될 수 있습니다.
validateCardNumber에서 숫자 문자열 조건도 함께 확인해 주세요.🤖 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/entities/card/model/cardNumber.ts` around lines 60 - 64, validateCardNumber currently only checks brand and length and will accept non-digit characters; update validateCardNumber to first assert the input is a numeric string (e.g. match only digits) and return false if not, then proceed to call getCardBrand and compare length against BRAND_RULES[brand]; ensure you reference validateCardNumber, getCardBrand, and BRAND_RULES when adding the numeric-only check so non-digit characters are rejected before brand/length validation.src/entities/card/model/cvc.ts-3-4 (1)
3-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCVC 검증이 길이만 확인합니다.
Line 3-4는 숫자 여부를 확인하지 않아
'abc'같은 값도 통과할 수 있습니다. 숫자 문자열 검증을 함께 넣는 게 안전합니다.🤖 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/entities/card/model/cvc.ts` around lines 3 - 4, validateCvc currently only checks length and will accept non-numeric strings; update validateCvc to verify both that cvc.length === CVC_LENGTH and that the string contains only digits (e.g., use a numeric regex match like /^\d+$/ or a length-specific /^\d{CVC_LENGTH}$/) before returning true, keeping reference to the validateCvc function and the CVC_LENGTH constant.src/features/cardPreview/CardPreview.tsx-45-50 (1)
45-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win은행 미선택 시
bankClassName이 빈 문자열로 인해className에"undefined"가 포함됩니다.
bank가 없는 경우bankClassName이 빈 문자열('')이 되고,bankStyles['']는 정의되지 않아undefined로 평가됩니다. 이는 DOM의 className 속성에 문자열"undefined"를 추가하게 됩니다.CSS 모듈(
bank.module.css)에 이미.unknown클래스가 정의되어 있으므로, Line 45의 기본값을'unknown'으로 변경하는 것을 권장합니다:const bankClassName = bank !== undefined ? BANK_RULES[bank].className : 'unknown';🤖 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/cardPreview/CardPreview.tsx` around lines 45 - 50, The bankClassName default is set to empty string which makes bankStyles[bankClassName] evaluate to undefined and inject "undefined" into the DOM className; update the logic that sets bankClassName (in the CardPreview component where BANK_RULES and bankStyles are used) to use 'unknown' as the fallback when bank is undefined and ensure the JSX uses bankStyles[bankClassName] (or a safe lookup) so the '.unknown' CSS module class is applied instead of an empty/undefined value.src/core/styles/fonts.css-1-1 (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win폰트 import URL이 잘못되어 실제 폰트가 로드되지 않습니다.
Line 1의
https://googleapis.com은 Google Fonts CSS 제공 주소가 아니라서index.css의Noto Sans가 적용되지 않고 폴백 폰트로 동작할 가능성이 큽니다.🤖 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/core/styles/fonts.css` at line 1, The `@import` in src/core/styles/fonts.css uses an incorrect URL (https://googleapis.com) so Google Fonts aren't loaded; replace the `@import` target with the correct Google Fonts stylesheet URL (fonts.googleapis.com) for Noto Sans (e.g., the css2 URL including the Noto+Sans family and desired weights and display parameter) so the Noto Sans declaration in index.css actually loads and applies.src/core/components/field/Field.module.css-19-26 (1)
19-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win잘못된 CSS 속성 값을 수정해주세요.
Line 21의
font-style: Bold;는 두 가지 문제가 있습니다:
font-style의 유효한 값은normal,italic,oblique이며,Bold는 유효하지 않습니다.- 굵기를 표현하려면
font-weight속성을 사용해야 하는데, line 20에 이미font-weight: 700이 정의되어 있습니다.Line 21을 제거하거나, 다른 의도가 있었다면 올바른
font-style값으로 변경해주세요.🔧 수정 제안
.title { font-weight: 700; - font-style: Bold; font-size: 18px; line-height: 100%; letter-spacing: 0%; vertical-align: middle; }🤖 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/core/components/field/Field.module.css` around lines 19 - 26, In the .title CSS rule remove the invalid property "font-style: Bold;" (or replace it with a valid font-style value such as "font-style: normal" or "font-style: italic" if italic/oblique was intended); note that weight is already set via "font-weight: 700" so do not use font-style to express boldness—update the .title rule accordingly.README.md-70-70 (1)
70-70:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win오타를 수정해주세요.
문서에서 다음 오타가 발견되었습니다:
- 70, 79번 줄: "실기간" → "실시간" (real-time의 올바른 번역)
- 88번 줄: "카드 branch" → "카드 brand" (브랜드의 올바른 영문 표기)
📝 수정 제안
- - [ ] 유효하지 않은 번호 입력시 실기간 피드백을 제공한다 + - [ ] 유효하지 않은 번호 입력시 실시간 피드백을 제공한다- - [ ] 유효하지 않은 월 입력시 실기간 피드백을 제공한다 + - [ ] 유효하지 않은 월 입력시 실시간 피드백을 제공한다- - [ ] 구분한 카드 branch에 따라 이미지를 보여준다 + - [ ] 구분한 카드 brand에 따라 이미지를 보여준다Also applies to: 79-79, 88-88
🤖 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 `@README.md` at line 70, Fix typos in the README by replacing the incorrect Korean term "실기간" with the correct "실시간" at the two occurrences that match the sentence fragment "유효하지 않은 번호 입력시 실기간 피드백을 제공한다" (and its duplicate), and change "카드 branch" to "카드 brand" for the occurrence that contains that phrase; update those exact strings in README.md so the document uses "실시간" and "카드 brand".README.md-39-63 (1)
39-63:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win폴더 구조 예시를 컨벤션에 맞게 수정해주세요.
폴더 구조 템플릿에서 다음 문제가 발견되었습니다:
- 컨벤션 위반 (54-55번 줄):
/카드번호,/유효기간등 한글 폴더명이 21번 줄에 명시된 "폴더명: camelCase" 컨벤션을 위반합니다.- 플레이스홀더 (59-60번 줄):
degueAPI.tsx,asdsad같은 의미 없는 이름은 제거하거나 실제 예시로 교체해야 합니다.📝 수정 제안
/components - /CVC - /카드번호 - /유효기간 + /cvc + /cardNumber + /expiryDate /hooks /usePayments.tsx /api - /degueAPI.tsx - /asdsad + /cardApi.ts🤖 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 `@README.md` around lines 39 - 63, The folder-structure sample violates the project's camelCase convention and contains placeholder names: rename Korean-named folders `/카드번호` and `/유효기간` to camelCase equivalents such as `cardNumber` and `expirationDate`, keep `CVC` as-is or change to `cvc` for consistency, replace meaningless placeholders like `degueAPI.tsx` and `asdsad` with realistic names (e.g., `paymentsApi.tsx` or `degueApi.tsx` for API modules) or remove them, and ensure hook filenames like `usePayments.tsx` remain camelCase; update any references in `/features/payments` and `/components` to match these new names.src/features/registerCard/lib/validateFieldData.ts-11-11 (1)
11-11:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
== undefined비교 의도를 확인해 주세요.Line 11은 느슨한 비교라
null도 통과합니다. 이 함수에서 성공 기준이 정말undefined만이라면=== undefined로 맞추는 편이 안전합니다.null도 성공으로 보려는 의도였을까요?🤖 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/registerCard/lib/validateFieldData.ts` at line 11, The loose equality check in the isValidMonth assignment uses validateExpiryMonth(expiryDate.month) == undefined which will treat null as valid too; update this to a strict comparison validateExpiryMonth(expiryDate.month) === undefined if the only success case is undefined, or explicitly allow null by checking for both (e.g., value === undefined || value === null) depending on the intended validation; locate the isValidMonth variable and validateExpiryMonth call to make the change.src/features/submitButton/SubmitButton.module.css-3-14 (1)
3-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win중복 선언된
bottom속성을 하나 제거해 주세요.Line 3과 Line 14가 동일 선언이라 lint 에러가 발생합니다.
최소 수정 예시
.submitButton { position: fixed; bottom: 0; width: 100%; left: 50%; background-color: `#333333`; color: `#ffffff`; font-size: 16px; font-weight: 700; cursor: pointer; border: none; border-radius: 2px; z-index: 100; - bottom: 0; padding: 10px; transform: translateX(-50%); }🤖 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/submitButton/SubmitButton.module.css` around lines 3 - 14, The CSS block in SubmitButton.module.css contains a duplicate bottom declaration; remove one of the two identical "bottom: 0;" entries (either the first or the last) so the rule set only declares bottom once to resolve the lint error and keep the rest of the styles (width, left, background-color, etc.) unchanged.
🧹 Nitpick comments (10)
eslint.config.js (1)
12-12: 💤 Low value
storybook-static디렉터리도 무시 목록에 추가하는 것을 고려해보세요.
.gitignore에storybook-static이 포함되어 있는데, ESLint가 이 디렉터리를 검사하지 않도록globalIgnores배열에 추가하면 불필요한 린팅 작업을 줄일 수 있습니다.현재 프로젝트에서 Storybook 빌드 산출물을 어떻게 관리하고 있는지 확인해보시고, 필요하다면 추가를 고려해보세요.
💡 제안하는 변경
- globalIgnores(['dist']), + globalIgnores(['dist', 'storybook-static']),🤖 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 `@eslint.config.js` at line 12, Add "storybook-static" to the ESLint ignore list so build artifacts aren’t linted: update the globalIgnores call in eslint.config.js (the globalIgnores([...]) invocation) to include 'storybook-static' alongside 'dist', verifying your project actually produces a storybook-static output before committing the change.package.json (1)
13-13: ⚖️ Poor tradeoffdeploy 스크립트의 크로스 플랫폼 호환성을 개선할 수 있습니다.
현재
touch dist/.nojekyll명령어는 Unix/Linux/macOS에서만 동작하며, Windows 환경에서는 실행되지 않을 수 있습니다.크로스 플랫폼 호환성을 위해 다음 방법들을 고려해보세요:
- Node.js 스크립트 사용 (fs.writeFileSync)
- 크로스 플랫폼 패키지 사용 (예:
touch-cli)- Vite 플러그인으로 빌드 시 자동 생성
Windows 환경에서 배포를 진행할 가능성이 있다면 개선을 검토해보세요.
🤖 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 `@package.json` at line 13, The "deploy" npm script uses the Unix-only touch command which breaks on Windows; update the "deploy" script in package.json to create dist/.nojekyll in a cross-platform way—either add a small Node script (e.g., scripts/createNoJekyll.js using fs.writeFileSync) and call it from the "deploy" pipeline before cp and npx gh-pages, or add a cross-platform dependency like touch-cli (and replace touch dist/.nojekyll with npx touch-cli dist/.nojekyll); ensure the chosen change is referenced from the "deploy" script so dist/.nojekyll is always created on all platforms.src/core/components/field/Field.tsx (1)
15-21: ⚡ Quick win옵셔널 props의 조건부 렌더링을 고려해보세요.
subTitle과label이 옵셔널 props인데 값이 없을 때도 항상 빈 요소가 렌더링되고 있습니다. 이로 인해 불필요한 DOM 노드가 생성될 수 있습니다.값이 있을 때만 해당 요소를 렌더링하도록 개선할 수 있는지 검토해보세요. 조건부 렌더링 패턴(예:
&&연산자, 삼항 연산자)을 활용하면 DOM을 간결하게 유지할 수 있습니다.🤖 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/core/components/field/Field.tsx` around lines 15 - 21, The current Field component always renders empty DOM nodes for optional props; update the JSX so subTitle and label are conditionally rendered only when truthy: inside the legend block render the <div className={styles.subTitle}> element only when subTitle is defined (e.g., subTitle && ...), and inside the form wrapper render the <span className={styles.label}> element only when label is defined (e.g., label && ...); keep title and children rendering as-is and use the existing className symbols (styles.legend, styles.title, styles.subTitle, styles.formWrapper, styles.label, styles.children) to locate the elements to change.README.md (1)
41-41: 💤 Low value코드 블록에 언어 지정자를 추가하세요.
Markdown 린터가 코드 블록에 언어 지정자가 없다고 경고하고 있습니다. 언어를 명시하면 구문 강조가 적용되어 가독성이 향상됩니다.
📝 수정 제안
-``` +```plaintext /src /core🤖 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 `@README.md` at line 41, The README contains a fenced code block with no language specifier; update that block (the triple-backtick that wraps the directory tree snippet) to include a language identifier such as plaintext or text (e.g., replace ``` with ```plaintext) so the Markdown linter stops warning and syntax highlighting/readability improve.src/features/registerCard/ui/fields/BankSelectField.tsx (1)
33-33: 💤 Low value변수 이름을 단수형으로 수정하는 것을 고려해보세요.
BANKS.map((banks) => ...)에서banks는 개별 은행을 나타내므로 단수형bank가 더 적절합니다.🤖 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/registerCard/ui/fields/BankSelectField.tsx` at line 33, Rename the plural callback parameter in the BANKS.map call from "banks" to the singular "bank" to reflect that each iteration yields one bank; update the map callback parameter in the component (the BANKS.map(...) expression) and any usages inside that callback (e.g., properties accessed from banks) to use "bank" so naming is consistent and clearer.src/features/registerCard/ui/fields/BankSelectField.module.css (2)
21-28: 💤 Low value중복된 스타일을 하나의 규칙으로 통합하는 방법을 고민해보세요.
.button:hover와.button:focus에서 동일한border-color:#000000`` 스타일이 반복됩니다.어떻게 하면 이 중복을 제거할 수 있을까요?
힌트: CSS 셀렉터를 쉼표로 구분하여 그룹화할 수 있습니다.
🤖 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/registerCard/ui/fields/BankSelectField.module.css` around lines 21 - 28, The .button:hover and .button:focus rules both set border-color: `#000000`; remove the duplication by combining selectors for the shared declaration (use a grouped selector like .button:hover, .button:focus { border-color: `#000000`; }) and keep the focus-only rule for the outline behavior (e.g., .button:focus { outline: none; }) so you preserve the focus-specific styles while eliminating the repeated border-color.
36-36: ⚡ Quick win매직 넘버
99.3%대신 명확한 레이아웃 기법을 고려해보세요.현재 드롭다운 리스트의 너비를
99.3%로 설정하여 보더를 고려하고 있는 것으로 보입니다.
box-sizing: border-box와width: 100%를 함께 사용하거나,calc(100% - 2px)같은 명시적 계산을 사용하면 의도가 더 명확해집니다.🤖 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/registerCard/ui/fields/BankSelectField.module.css` at line 36, The rule setting width: 99.3% in BankSelectField.module.css is using a magic number to account for borders; change it to a clear layout approach by (a) applying box-sizing: border-box to the same selector (or a shared form-control/root selector) and using width: 100%, or (b) using an explicit calc like width: calc(100% - 2px) if you need to subtract a known border/padding; update the selector in BankSelectField.module.css (the one that currently has width: 99.3%) and remove the magic percentage so the intent is explicit and robust.src/features/registerCard/model/registerPassword.ts (1)
4-6: 💤 Low value에러 메시지에서 용어 일관성을 고려해보세요.
현재 "PASSWORD를 전부 채워주세요."처럼 한글 문장에 영어 단어를 혼용하고 있습니다.
다른 필드 에러 메시지들과 일관성을 유지하려면 어떤 용어를 사용하는 것이 좋을지 프로젝트의 문구 가이드라인을 확인해보세요.
🤖 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/registerCard/model/registerPassword.ts` around lines 4 - 6, The PASSWORD_ERROR_MESSAGE uses mixed language ("PASSWORD를 전부 채워주세요.") which breaks terminology consistency; update the message value in PASSWORD_ERROR_MESSAGE to use the project's preferred term (e.g., replace "PASSWORD" with the Korean equivalent "비밀번호" or whatever the project's copy guideline specifies) so it matches other field error messages and adjust any tests or usages that assert this exact text if needed.src/features/registerCard/model/registerCardNumber.ts (2)
63-65: ⚖️ Poor tradeoff카드 번호 전체 오류 시 모든 필드에 동일한 에러 메시지를 표시하는 것이 최선인지 검토해보세요.
현재
cardNumberError가 발생하면 모든 입력 필드에 동일한 "유효하지 않은 카드번호입니다." 메시지가 표시됩니다. 사용자 입장에서는 어느 필드를 수정해야 할지 혼란스러울 수 있습니다.개별 필드 에러(길이 부족 등)와 전체 카드 번호 검증 실패를 구분하여 표시하는 방법을 고민해보세요.
🤖 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/registerCard/model/registerCardNumber.ts` around lines 63 - 65, The current logic assigns the same cardNumberError to every field by setting inputErrors = cardNumberError ? visibleFieldErrors.map(() => cardNumberError) : visibleFieldErrors; change this so overall card-number validation failures and per-field validation failures are handled separately: keep visibleFieldErrors as the per-input array, derive a distinct form- or field-level indicator (e.g., cardNumberGlobalError) for 전체-card validation failures and only apply per-field overrides when there are specific per-index errors; update the consumer of inputErrors to display per-field messages from visibleFieldErrors and show the global cardNumberError via a single shared UI element (or attach it only to the first field) instead of copying it to all entries.
82-110: ⚡ Quick win중복된 카드 번호 파싱 로직을 공통 함수로 분리하는 것을 고려해보세요.
getCardNumberFieldState(82-87라인)와getNextCardNumberFieldState(107-110라인)에서 카드 번호 결합, 브랜드 판별, 포맷 결정 로직이 중복됩니다.이 공통 로직을 별도 함수로 추출하면 유지보수성이 향상됩니다. 어떤 방식으로 중복을 제거할 수 있을지 생각해보세요.
💡 힌트: 공통 로직 추출 방향
카드 번호 배열에서 브랜드와 포맷을 계산하는 부분을 별도 함수로 만들어보세요:
- 입력:
numbers: string[]- 출력:
{ cardNumber: string, brand: CardBrand | undefined, format: number[] }두 함수에서 이 공통 함수를 재사용하면 코드 중복을 제거할 수 있습니다.
🤖 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/registerCard/model/registerCardNumber.ts` around lines 82 - 110, Extract the duplicated card-parsing logic into a new helper that accepts numbers: string[] and returns { cardNumber: string, brand: CardBrand | undefined, format: number[] }; replace the repeated sequences in getCardNumberFieldState and getNextCardNumberFieldState (where numbers.join(''), getCardBrand(cardNumber), and getCardBrandFormat(brand) are used) with calls to this helper, and update subsequent calls to validateCardNumber, isCompleteCardNumber, getCardNumberErrors, etc., to use the returned cardNumber/brand/format values.
🤖 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 @.storybook/main.ts:
- Line 4: The Storybook "stories" glob in .storybook/main.ts only matches
*.stories.* files so files named like Field.story.tsx are missed; update the
stories array entry (the stories property) to include both patterns by adding a
second glob for ../src/**/*.story.@(js|jsx|mjs|ts|tsx) or use a combined glob
that matches both (e.g., ../src/**/*.{story,stories}.@(js|jsx|mjs|ts|tsx)) so
Storybook will detect files named with either .story.* or .stories.*.
In `@src/core/styles/morden-normalize.css`:
- Line 1: Rename the CSS file from morden-normalize.css to modern-normalize.css
and update every import/reference that uses "morden-normalize.css" to the new
"modern-normalize.css" name; search for the symbol "morden-normalize.css" across
the repo (import statements, bundler entries, HTML <link> tags, and any
build/config files) and replace each occurrence so all modules and assets now
reference modern-normalize.css to avoid broken imports.
In `@src/features/registerCard/ui/cardForm/CardForm.tsx`:
- Around line 42-74: The fields are being conditionally rendered in reverse
insertion order which causes layout jumps as step increases; reorder the JSX to
a fixed render order NumberField, BankSelectField, ExpiryDateField, CvcField,
PasswordField (still using the same props: numbersField, bankField, expiryField,
cvcField, passwordField, setStepRef callbacks, onComplate handlers, and STEP
constants) so new fields appear below existing ones instead of above; keep the
existing step-based conditions (e.g., step >= X) or replace them with visibility
logic but do not change prop names or handler targets (toStep, setStepRef,
STEP.*) so behavior and refs remain intact.
In `@src/features/registerCard/ui/fields/BankSelectField.tsx`:
- Around line 17-20: In handleChange in BankSelectField.tsx, the condition if
(value !== undefined) is incorrect because the select returns a string (empty
string for placeholder) not undefined; change the guard to check for a non-empty
string (e.g., if (value !== "" ) or if (Boolean(value))) so onComplate() is only
called for a valid bank selection; update the condition around onComplate() in
the handleChange function accordingly.
- Line 28: The current onChange uses a blind assertion (e.target.value as Bank);
update BankSelectField to validate the incoming value against the known BANKS
list before calling handleChange: check if e.target.value exists in BANKS (e.g.,
find/includes by comparing the option key/value) and only cast/pass it to
handleChange when it matches, otherwise handle the invalid case (no-op, set
error, or log). Reference: the onChange handler, the handleChange function, the
Bank type and the BANKS array.
In `@src/features/registerCard/ui/fields/CardNumberField.tsx`:
- Line 28: Initialize the touched state array dynamically instead of hardcoding
four entries: replace the fixed [false, false, false, false] with a useState
initializer that creates an array of the same length as the source (e.g., format
or numbers) using Array.from or new Array(length).fill(false) inside the lazy
initializer form of useState (e.g., useState(() => Array.from({length:
format.length}, () => false))). Also add a useEffect that watches the length of
format or numbers and resets setTouched to a new false-filled array when that
length changes so touched always matches the current number of input fields.
- Around line 63-66: CardNumberField is calling setStepRef for every input which
overwrites the step ref; change the ref handler so it still calls
setInputRef(node, index) for all inputs but only calls setStepRef(node) when
index === 0 (i.e., the first input), mirroring the pattern used in
ExpiryDateField/CardForm so only a single step ref is registered.
In `@src/pages/payments/Payments.tsx`:
- Around line 33-44: The state passed to navigate from onRegister is wrong and
includes sensitive data; change the object sent to navigate('/result', { state:
... }) so it matches the Result contract and omits secrets: use cardNumbers:
numbers (the string[]), keep expiryDate and bank, and do NOT include cvc or
password (remove numbers.join('') and the cvc/password fields from fieldData
used for routing); update the FieldData->state construction inside onRegister
(and any variable named fieldData) so navigate receives only { cardNumbers:
numbers, expiryDate, bank }.
In `@vite.config.ts`:
- Line 47: The alias entry for '@' currently calls path.resolve(__dirname,
'./src') instead of reusing the already-computed dirname variable; update the
alias to use path.resolve(dirname, 'src') (or path.join(dirname, 'src')) so it
leverages the safe ESM-compatible dirname value defined earlier (the dirname
symbol) and avoid direct use of __dirname in the alias configuration.
---
Minor comments:
In `@README.md`:
- Line 70: Fix typos in the README by replacing the incorrect Korean term "실기간"
with the correct "실시간" at the two occurrences that match the sentence fragment
"유효하지 않은 번호 입력시 실기간 피드백을 제공한다" (and its duplicate), and change "카드 branch" to
"카드 brand" for the occurrence that contains that phrase; update those exact
strings in README.md so the document uses "실시간" and "카드 brand".
- Around line 39-63: The folder-structure sample violates the project's
camelCase convention and contains placeholder names: rename Korean-named folders
`/카드번호` and `/유효기간` to camelCase equivalents such as `cardNumber` and
`expirationDate`, keep `CVC` as-is or change to `cvc` for consistency, replace
meaningless placeholders like `degueAPI.tsx` and `asdsad` with realistic names
(e.g., `paymentsApi.tsx` or `degueApi.tsx` for API modules) or remove them, and
ensure hook filenames like `usePayments.tsx` remain camelCase; update any
references in `/features/payments` and `/components` to match these new names.
In `@src/core/components/field/Field.module.css`:
- Around line 19-26: In the .title CSS rule remove the invalid property
"font-style: Bold;" (or replace it with a valid font-style value such as
"font-style: normal" or "font-style: italic" if italic/oblique was intended);
note that weight is already set via "font-weight: 700" so do not use font-style
to express boldness—update the .title rule accordingly.
In `@src/core/styles/fonts.css`:
- Line 1: The `@import` in src/core/styles/fonts.css uses an incorrect URL
(https://googleapis.com) so Google Fonts aren't loaded; replace the `@import`
target with the correct Google Fonts stylesheet URL (fonts.googleapis.com) for
Noto Sans (e.g., the css2 URL including the Noto+Sans family and desired weights
and display parameter) so the Noto Sans declaration in index.css actually loads
and applies.
In `@src/entities/card/model/cardNumber.ts`:
- Around line 60-64: validateCardNumber currently only checks brand and length
and will accept non-digit characters; update validateCardNumber to first assert
the input is a numeric string (e.g. match only digits) and return false if not,
then proceed to call getCardBrand and compare length against BRAND_RULES[brand];
ensure you reference validateCardNumber, getCardBrand, and BRAND_RULES when
adding the numeric-only check so non-digit characters are rejected before
brand/length validation.
In `@src/entities/card/model/cvc.ts`:
- Around line 3-4: validateCvc currently only checks length and will accept
non-numeric strings; update validateCvc to verify both that cvc.length ===
CVC_LENGTH and that the string contains only digits (e.g., use a numeric regex
match like /^\d+$/ or a length-specific /^\d{CVC_LENGTH}$/) before returning
true, keeping reference to the validateCvc function and the CVC_LENGTH constant.
In `@src/entities/card/model/expiryDate.ts`:
- Around line 11-15: The isValidMonth function currently returns true for any
month string whose length is not 1 or 2, which wrongly treats empty or 3+ char
strings as valid; change its default behavior so that if month.length is neither
1 nor 2 the function returns false, and keep the existing regex checks for
length 1 (allow "0" or "1") and length 2 (allow "01".."12") in the isValidMonth
function to ensure only valid month inputs pass.
In `@src/entities/card/model/password.ts`:
- Around line 3-4: validatePassword currently only checks length
(PASSWORD_LENGTH) and therefore allows non-numeric strings; update
validatePassword to ensure the input is numeric and exactly PASSWORD_LENGTH long
by verifying both password.length === PASSWORD_LENGTH and that the string
contains only digits (e.g., using a digit-only check/regex) before returning
true.
In `@src/features/cardPreview/CardPreview.tsx`:
- Around line 45-50: The bankClassName default is set to empty string which
makes bankStyles[bankClassName] evaluate to undefined and inject "undefined"
into the DOM className; update the logic that sets bankClassName (in the
CardPreview component where BANK_RULES and bankStyles are used) to use 'unknown'
as the fallback when bank is undefined and ensure the JSX uses
bankStyles[bankClassName] (or a safe lookup) so the '.unknown' CSS module class
is applied instead of an empty/undefined value.
In `@src/features/registerCard/lib/validateFieldData.ts`:
- Line 11: The loose equality check in the isValidMonth assignment uses
validateExpiryMonth(expiryDate.month) == undefined which will treat null as
valid too; update this to a strict comparison
validateExpiryMonth(expiryDate.month) === undefined if the only success case is
undefined, or explicitly allow null by checking for both (e.g., value ===
undefined || value === null) depending on the intended validation; locate the
isValidMonth variable and validateExpiryMonth call to make the change.
In `@src/features/submitButton/SubmitButton.module.css`:
- Around line 3-14: The CSS block in SubmitButton.module.css contains a
duplicate bottom declaration; remove one of the two identical "bottom: 0;"
entries (either the first or the last) so the rule set only declares bottom once
to resolve the lint error and keep the rest of the styles (width, left,
background-color, etc.) unchanged.
In `@src/pages/result/Result.module.css`:
- Around line 18-24: The CSS file is missing the .messageBox class referenced as
styles.messageBox in Result.tsx, so add a .messageBox selector to
src/pages/result/Result.module.css that provides the intended layout (e.g.,
container width, padding, margin, display/alignments) to wrap .message; update
or match any existing naming (ensure .message remains) so Result.tsx's
styles.messageBox works correctly.
---
Nitpick comments:
In `@eslint.config.js`:
- Line 12: Add "storybook-static" to the ESLint ignore list so build artifacts
aren’t linted: update the globalIgnores call in eslint.config.js (the
globalIgnores([...]) invocation) to include 'storybook-static' alongside 'dist',
verifying your project actually produces a storybook-static output before
committing the change.
In `@package.json`:
- Line 13: The "deploy" npm script uses the Unix-only touch command which breaks
on Windows; update the "deploy" script in package.json to create dist/.nojekyll
in a cross-platform way—either add a small Node script (e.g.,
scripts/createNoJekyll.js using fs.writeFileSync) and call it from the "deploy"
pipeline before cp and npx gh-pages, or add a cross-platform dependency like
touch-cli (and replace touch dist/.nojekyll with npx touch-cli dist/.nojekyll);
ensure the chosen change is referenced from the "deploy" script so
dist/.nojekyll is always created on all platforms.
In `@README.md`:
- Line 41: The README contains a fenced code block with no language specifier;
update that block (the triple-backtick that wraps the directory tree snippet) to
include a language identifier such as plaintext or text (e.g., replace ``` with
```plaintext) so the Markdown linter stops warning and syntax
highlighting/readability improve.
In `@src/core/components/field/Field.tsx`:
- Around line 15-21: The current Field component always renders empty DOM nodes
for optional props; update the JSX so subTitle and label are conditionally
rendered only when truthy: inside the legend block render the <div
className={styles.subTitle}> element only when subTitle is defined (e.g.,
subTitle && ...), and inside the form wrapper render the <span
className={styles.label}> element only when label is defined (e.g., label &&
...); keep title and children rendering as-is and use the existing className
symbols (styles.legend, styles.title, styles.subTitle, styles.formWrapper,
styles.label, styles.children) to locate the elements to change.
In `@src/features/registerCard/model/registerCardNumber.ts`:
- Around line 63-65: The current logic assigns the same cardNumberError to every
field by setting inputErrors = cardNumberError ? visibleFieldErrors.map(() =>
cardNumberError) : visibleFieldErrors; change this so overall card-number
validation failures and per-field validation failures are handled separately:
keep visibleFieldErrors as the per-input array, derive a distinct form- or
field-level indicator (e.g., cardNumberGlobalError) for 전체-card validation
failures and only apply per-field overrides when there are specific per-index
errors; update the consumer of inputErrors to display per-field messages from
visibleFieldErrors and show the global cardNumberError via a single shared UI
element (or attach it only to the first field) instead of copying it to all
entries.
- Around line 82-110: Extract the duplicated card-parsing logic into a new
helper that accepts numbers: string[] and returns { cardNumber: string, brand:
CardBrand | undefined, format: number[] }; replace the repeated sequences in
getCardNumberFieldState and getNextCardNumberFieldState (where numbers.join(''),
getCardBrand(cardNumber), and getCardBrandFormat(brand) are used) with calls to
this helper, and update subsequent calls to validateCardNumber,
isCompleteCardNumber, getCardNumberErrors, etc., to use the returned
cardNumber/brand/format values.
In `@src/features/registerCard/model/registerPassword.ts`:
- Around line 4-6: The PASSWORD_ERROR_MESSAGE uses mixed language ("PASSWORD를 전부
채워주세요.") which breaks terminology consistency; update the message value in
PASSWORD_ERROR_MESSAGE to use the project's preferred term (e.g., replace
"PASSWORD" with the Korean equivalent "비밀번호" or whatever the project's copy
guideline specifies) so it matches other field error messages and adjust any
tests or usages that assert this exact text if needed.
In `@src/features/registerCard/ui/fields/BankSelectField.module.css`:
- Around line 21-28: The .button:hover and .button:focus rules both set
border-color: `#000000`; remove the duplication by combining selectors for the
shared declaration (use a grouped selector like .button:hover, .button:focus {
border-color: `#000000`; }) and keep the focus-only rule for the outline behavior
(e.g., .button:focus { outline: none; }) so you preserve the focus-specific
styles while eliminating the repeated border-color.
- Line 36: The rule setting width: 99.3% in BankSelectField.module.css is using
a magic number to account for borders; change it to a clear layout approach by
(a) applying box-sizing: border-box to the same selector (or a shared
form-control/root selector) and using width: 100%, or (b) using an explicit calc
like width: calc(100% - 2px) if you need to subtract a known border/padding;
update the selector in BankSelectField.module.css (the one that currently has
width: 99.3%) and remove the magic percentage so the intent is explicit and
robust.
In `@src/features/registerCard/ui/fields/BankSelectField.tsx`:
- Line 33: Rename the plural callback parameter in the BANKS.map call from
"banks" to the singular "bank" to reflect that each iteration yields one bank;
update the map callback parameter in the component (the BANKS.map(...)
expression) and any usages inside that callback (e.g., properties accessed from
banks) to use "bank" so naming is consistent and clearer.
🪄 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: a6c69bed-b6b6-413e-8c04-9250470d1709
⛔ Files ignored due to path filters (10)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/favicon.svgis excluded by!**/*.svgpublic/icons.svgis excluded by!**/*.svgsrc/core/assets/Check.svgis excluded by!**/*.svgsrc/core/assets/DefaultCardImage.pngis excluded by!**/*.pngsrc/entities/card/config/AmericanExpress.svgis excluded by!**/*.svgsrc/entities/card/config/Diners.svgis excluded by!**/*.svgsrc/entities/card/config/Mastercard.svgis excluded by!**/*.svgsrc/entities/card/config/UnionPay.svgis excluded by!**/*.svgsrc/entities/card/config/Visa.svgis excluded by!**/*.svg
📒 Files selected for processing (62)
.editorconfig.github/pull_request_template.md.gitignore.gitmessage.txt.prettierrc.storybook/main.ts.storybook/preview.tsREADME.mdeslint.config.jsindex.htmlpackage.jsonsrc/App.tsxsrc/core/components/field/Field.module.csssrc/core/components/field/Field.story.tsxsrc/core/components/field/Field.tsxsrc/core/components/input/Input.module.csssrc/core/components/input/Input.stories.tsxsrc/core/components/input/Input.tsxsrc/core/hooks/useInputFocus.tssrc/core/styles/fonts.csssrc/core/styles/index.csssrc/core/styles/morden-normalize.csssrc/core/utils/validator.tssrc/entities/card/config/bank.module.csssrc/entities/card/config/brandSvgMap.tssrc/entities/card/model/bank.tssrc/entities/card/model/cardNumber.tssrc/entities/card/model/cvc.tssrc/entities/card/model/expiryDate.tssrc/entities/card/model/password.tssrc/features/cardPreview/CardPreview.module.csssrc/features/cardPreview/CardPreview.stories.tsxsrc/features/cardPreview/CardPreview.tsxsrc/features/cardPreview/model/cardPreview.tssrc/features/registerCard/hooks/usePaymentsStep.tssrc/features/registerCard/lib/validateFieldData.tssrc/features/registerCard/model/payments.tssrc/features/registerCard/model/registerCardNumber.tssrc/features/registerCard/model/registerCvc.tssrc/features/registerCard/model/registerExpiryDate.tssrc/features/registerCard/model/registerPassword.tssrc/features/registerCard/ui/cardForm/CardForm.module.csssrc/features/registerCard/ui/cardForm/CardForm.tsxsrc/features/registerCard/ui/fields/BankSelectField.module.csssrc/features/registerCard/ui/fields/BankSelectField.tsxsrc/features/registerCard/ui/fields/CardNumberField.tsxsrc/features/registerCard/ui/fields/CvcField.tsxsrc/features/registerCard/ui/fields/ExpiryDateField.tsxsrc/features/registerCard/ui/fields/PasswordField.tsxsrc/features/submitButton/SubmitButton.module.csssrc/features/submitButton/SubmitButton.tsxsrc/main.tsxsrc/pages/payments/Payments.module.csssrc/pages/payments/Payments.tsxsrc/pages/result/Result.module.csssrc/pages/result/Result.tsxsrc/svg.d.tstsconfig.app.jsontsconfig.jsontsconfig.node.jsonvite.config.tsvitest.shims.d.ts
💤 Files with no reviewable changes (1)
- .github/pull_request_template.md
| import type { StorybookConfig } from '@storybook/react-vite'; | ||
|
|
||
| const config: StorybookConfig = { | ||
| stories: ['../src/**/*.mdx', '../src/**/*.stories.@(js|jsx|mjs|ts|tsx)'], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Storybook 스토리 파일 패턴 확인
echo "=== .story.tsx 패턴 파일 ==="
fd -e tsx '.story.tsx$' src/
echo -e "\n=== .stories.tsx 패턴 파일 ==="
fd -e tsx '.stories.tsx$' src/
echo -e "\n=== 모든 story 관련 파일 ==="
fd 'story|stories' src/ --type fRepository: woowacourse/react-payments
Length of output: 417
Storybook 스토리 파일 감지 누락 문제를 해결하세요.
프로젝트에서 사용 중인 스토리 파일의 명명 규칙이 일관되지 않습니다. Field.story.tsx와 같이 .story.tsx 패턴을 사용하는 파일이 존재하지만, 현재 .storybook/main.ts 설정은 .stories.* 패턴만 매칭하므로 이 파일들이 감지되지 않습니다.
모든 스토리 파일을 포함하도록 패턴 수정
- stories: ['../src/**/*.mdx', '../src/**/*.stories.@(js|jsx|mjs|ts|tsx)'],
+ stories: ['../src/**/*.mdx', '../src/**/*.{story,stories}.@(js|jsx|mjs|ts|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.
Suggested change
| stories: ['../src/**/*.mdx', '../src/**/*.stories.@(js|jsx|mjs|ts|tsx)'], | |
| stories: ['../src/**/*.mdx', '../src/**/*.{story,stories}.@(js|jsx|mjs|ts|tsx)'], |
🤖 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 @.storybook/main.ts at line 4, The Storybook "stories" glob in
.storybook/main.ts only matches *.stories.* files so files named like
Field.story.tsx are missed; update the stories array entry (the stories
property) to include both patterns by adding a second glob for
../src/**/*.story.@(js|jsx|mjs|ts|tsx) or use a combined glob that matches both
(e.g., ../src/**/*.{story,stories}.@(js|jsx|mjs|ts|tsx)) so Storybook will
detect files named with either .story.* or .stories.*.
| @@ -0,0 +1,211 @@ | |||
| /*! modern-normalize v3.0.1 | MIT License | https://github.com/sindresorhus/modern-normalize */ | |||
There was a problem hiding this comment.
파일명 오타를 수정해주세요.
파일명이 morden-normalize.css로 되어 있는데, 정확한 라이브러리 이름은 modern-normalize입니다. 파일명을 modern-normalize.css로 변경하고, 이 파일을 임포트하는 모든 경로도 함께 업데이트해야 합니다.
🤖 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/core/styles/morden-normalize.css` at line 1, Rename the CSS file from
morden-normalize.css to modern-normalize.css and update every import/reference
that uses "morden-normalize.css" to the new "modern-normalize.css" name; search
for the symbol "morden-normalize.css" across the repo (import statements,
bundler entries, HTML <link> tags, and any build/config files) and replace each
occurrence so all modules and assets now reference modern-normalize.css to avoid
broken imports.
Comment on lines
+42
to
+74
| {step >= 4 && ( | ||
| <PasswordField | ||
| passwordField={passwordField} | ||
| setStepRef={(node) => setStepRef(node, STEP.PASSWORD)} | ||
| onComplate={() => {}} | ||
| /> | ||
| )} | ||
| {step >= 3 && ( | ||
| <CvcField | ||
| cvcField={cvcField} | ||
| setStepRef={(node) => setStepRef(node, STEP.CVC)} | ||
| onComplate={() => toStep(STEP.PASSWORD)} | ||
| /> | ||
| )} | ||
| {step >= 2 && ( | ||
| <ExpiryDateField | ||
| expiryField={expiryField} | ||
| setStepRef={(node) => setStepRef(node, STEP.EXPIRY)} | ||
| onComplate={() => toStep(STEP.CVC)} | ||
| /> | ||
| )} | ||
| {step >= 1 && ( | ||
| <BankSelectField | ||
| bankField={bankField} | ||
| setStepRef={(node) => setStepRef(node, STEP.BANK)} | ||
| onComplate={() => toStep(STEP.EXPIRY)} | ||
| /> | ||
| )} | ||
| <NumberField | ||
| numbersField={numbersField} | ||
| setStepRef={(node) => setStepRef(node, STEP.NUMBERS)} | ||
| onComplate={() => toStep(STEP.BANK)} | ||
| /> |
There was a problem hiding this comment.
단계별 필드가 역순으로 쌓여 입력 흐름이 흔들립니다.
Line 42~74 구조에서는 step이 올라갈수록 새 필드가 위에 삽입되어 레이아웃이 점프합니다. 번호→은행→유효기간→CVC→비밀번호 순으로 렌더 순서를 고정하는 쪽이 UX 안정성이 높습니다.
정렬 방향만 바로잡는 최소 변경 예시
return (
<form className={styles.form} id={formId} onSubmit={handleSubmit}>
- {step >= 4 && (
- <PasswordField
- passwordField={passwordField}
- setStepRef={(node) => setStepRef(node, STEP.PASSWORD)}
- onComplate={() => {}}
- />
- )}
- {step >= 3 && (
- <CvcField
- cvcField={cvcField}
- setStepRef={(node) => setStepRef(node, STEP.CVC)}
- onComplate={() => toStep(STEP.PASSWORD)}
- />
- )}
- {step >= 2 && (
- <ExpiryDateField
- expiryField={expiryField}
- setStepRef={(node) => setStepRef(node, STEP.EXPIRY)}
- onComplate={() => toStep(STEP.CVC)}
- />
- )}
- {step >= 1 && (
- <BankSelectField
- bankField={bankField}
- setStepRef={(node) => setStepRef(node, STEP.BANK)}
- onComplate={() => toStep(STEP.EXPIRY)}
- />
- )}
<NumberField
numbersField={numbersField}
setStepRef={(node) => setStepRef(node, STEP.NUMBERS)}
onComplate={() => toStep(STEP.BANK)}
/>
+ {step >= 1 && (
+ <BankSelectField
+ bankField={bankField}
+ setStepRef={(node) => setStepRef(node, STEP.BANK)}
+ onComplate={() => toStep(STEP.EXPIRY)}
+ />
+ )}
+ {step >= 2 && (
+ <ExpiryDateField
+ expiryField={expiryField}
+ setStepRef={(node) => setStepRef(node, STEP.EXPIRY)}
+ onComplate={() => toStep(STEP.CVC)}
+ />
+ )}
+ {step >= 3 && (
+ <CvcField
+ cvcField={cvcField}
+ setStepRef={(node) => setStepRef(node, STEP.CVC)}
+ onComplate={() => toStep(STEP.PASSWORD)}
+ />
+ )}
+ {step >= 4 && (
+ <PasswordField
+ passwordField={passwordField}
+ setStepRef={(node) => setStepRef(node, STEP.PASSWORD)}
+ onComplate={() => {}}
+ />
+ )}
</form>
);📝 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
| {step >= 4 && ( | |
| <PasswordField | |
| passwordField={passwordField} | |
| setStepRef={(node) => setStepRef(node, STEP.PASSWORD)} | |
| onComplate={() => {}} | |
| /> | |
| )} | |
| {step >= 3 && ( | |
| <CvcField | |
| cvcField={cvcField} | |
| setStepRef={(node) => setStepRef(node, STEP.CVC)} | |
| onComplate={() => toStep(STEP.PASSWORD)} | |
| /> | |
| )} | |
| {step >= 2 && ( | |
| <ExpiryDateField | |
| expiryField={expiryField} | |
| setStepRef={(node) => setStepRef(node, STEP.EXPIRY)} | |
| onComplate={() => toStep(STEP.CVC)} | |
| /> | |
| )} | |
| {step >= 1 && ( | |
| <BankSelectField | |
| bankField={bankField} | |
| setStepRef={(node) => setStepRef(node, STEP.BANK)} | |
| onComplate={() => toStep(STEP.EXPIRY)} | |
| /> | |
| )} | |
| <NumberField | |
| numbersField={numbersField} | |
| setStepRef={(node) => setStepRef(node, STEP.NUMBERS)} | |
| onComplate={() => toStep(STEP.BANK)} | |
| /> | |
| <NumberField | |
| numbersField={numbersField} | |
| setStepRef={(node) => setStepRef(node, STEP.NUMBERS)} | |
| onComplate={() => toStep(STEP.BANK)} | |
| /> | |
| {step >= 1 && ( | |
| <BankSelectField | |
| bankField={bankField} | |
| setStepRef={(node) => setStepRef(node, STEP.BANK)} | |
| onComplate={() => toStep(STEP.EXPIRY)} | |
| /> | |
| )} | |
| {step >= 2 && ( | |
| <ExpiryDateField | |
| expiryField={expiryField} | |
| setStepRef={(node) => setStepRef(node, STEP.EXPIRY)} | |
| onComplate={() => toStep(STEP.CVC)} | |
| /> | |
| )} | |
| {step >= 3 && ( | |
| <CvcField | |
| cvcField={cvcField} | |
| setStepRef={(node) => setStepRef(node, STEP.CVC)} | |
| onComplate={() => toStep(STEP.PASSWORD)} | |
| /> | |
| )} | |
| {step >= 4 && ( | |
| <PasswordField | |
| passwordField={passwordField} | |
| setStepRef={(node) => setStepRef(node, STEP.PASSWORD)} | |
| onComplate={() => {}} | |
| /> | |
| )} |
🤖 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/registerCard/ui/cardForm/CardForm.tsx` around lines 42 - 74, The
fields are being conditionally rendered in reverse insertion order which causes
layout jumps as step increases; reorder the JSX to a fixed render order
NumberField, BankSelectField, ExpiryDateField, CvcField, PasswordField (still
using the same props: numbersField, bankField, expiryField, cvcField,
passwordField, setStepRef callbacks, onComplate handlers, and STEP constants) so
new fields appear below existing ones instead of above; keep the existing
step-based conditions (e.g., step >= X) or replace them with visibility logic
but do not change prop names or handler targets (toStep, setStepRef, STEP.*) so
behavior and refs remain intact.
Comment on lines
+17
to
+20
| const handleChange = (value: Bank) => { | ||
| onChange(value); | ||
| if (value !== undefined) onComplate(); | ||
| }; |
There was a problem hiding this comment.
value !== undefined 조건이 의도대로 작동하지 않습니다.
handleChange의 value 매개변수는 select 요소에서 가져온 문자열이므로 절대 undefined가 될 수 없습니다.
비활성화된 옵션(placeholder)이 선택되면 value는 빈 문자열("")이 됩니다. 따라서 현재 조건문은 빈 문자열 선택 시에도 onComplate()를 호출합니다.
어떻게 하면 유효한 은행 선택 시에만 onComplate()를 호출할 수 있을까요?
힌트: 빈 문자열을 체크하는 조건을 생각해보세요.
🤖 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/registerCard/ui/fields/BankSelectField.tsx` around lines 17 -
20, In handleChange in BankSelectField.tsx, the condition if (value !==
undefined) is incorrect because the select returns a string (empty string for
placeholder) not undefined; change the guard to check for a non-empty string
(e.g., if (value !== "" ) or if (Boolean(value))) so onComplate() is only called
for a valid bank selection; update the condition around onComplate() in the
handleChange function accordingly.
| ref={setStepRef} | ||
| className={styles.button} | ||
| value={bank ?? ''} | ||
| onChange={(e) => handleChange(e.target.value as Bank)} |
There was a problem hiding this comment.
as Bank 타입 단언의 안전성을 검증하는 방법을 고민해보세요.
현재 e.target.value를 Bank 타입으로 단언하고 있습니다. 만약 예상치 못한 값이 들어온다면 타입 시스템을 우회하게 됩니다.
런타임에 실제로 Bank 타입의 값인지 검증하려면 어떤 방법을 사용할 수 있을까요?
힌트: BANKS 배열에 해당 값이 포함되어 있는지 확인하는 방법을 생각해보세요.
🤖 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/registerCard/ui/fields/BankSelectField.tsx` at line 28, The
current onChange uses a blind assertion (e.target.value as Bank); update
BankSelectField to validate the incoming value against the known BANKS list
before calling handleChange: check if e.target.value exists in BANKS (e.g.,
find/includes by comparing the option key/value) and only cast/pass it to
handleChange when it matches, otherwise handle the invalid case (no-op, set
error, or log). Reference: the onChange handler, the handleChange function, the
Bank type and the BANKS array.
| export const NumberField = ({ numbersField, setStepRef, onComplate }: CardNumberFieldProps) => { | ||
| const { numbers, onChange } = numbersField; | ||
| const { setInputRef, focusNext } = useInputFocus(); | ||
| const [touched, setTouched] = useState([false, false, false, false]); |
There was a problem hiding this comment.
touched 배열의 길이를 동적으로 초기화하는 방법을 고민해보세요.
현재 [false, false, false, false]로 하드코딩되어 있어 카드사에 따라 입력 필드 개수가 달라질 때 문제가 발생할 수 있습니다 (예: AMEX는 마지막 필드가 3자리).
format 또는 numbers 배열의 길이를 기반으로 touched 상태를 초기화하려면 어떻게 해야 할까요?
힌트: useState의 초기값으로 배열을 동적으로 생성하는 방법을 생각해보세요.
🤖 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/registerCard/ui/fields/CardNumberField.tsx` at line 28,
Initialize the touched state array dynamically instead of hardcoding four
entries: replace the fixed [false, false, false, false] with a useState
initializer that creates an array of the same length as the source (e.g., format
or numbers) using Array.from or new Array(length).fill(false) inside the lazy
initializer form of useState (e.g., useState(() => Array.from({length:
format.length}, () => false))). Also add a useEffect that watches the length of
format or numbers and resets setTouched to a new false-filled array when that
length changes so touched always matches the current number of input fields.
Comment on lines
+63
to
+66
| ref={(node) => { | ||
| setInputRef(node, index); | ||
| setStepRef(node); | ||
| }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# setStepRef의 정의와 사용처를 찾아 의도를 파악합니다.
rg -nP -C5 'setStepRef' --type=ts --type=tsxRepository: woowacourse/react-payments
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Find setStepRef definition and usage - try without specific file type
rg -nP -C5 'setStepRef' .Repository: woowacourse/react-payments
Length of output: 16353
🏁 Script executed:
# Also read the CardNumberField.tsx file to see full context
wc -l src/features/registerCard/ui/fields/CardNumberField.tsxRepository: woowacourse/react-payments
Length of output: 129
🏁 Script executed:
# List files in the directory to understand structure
fd -t f . src/features/registerCard/ui/fields/Repository: woowacourse/react-payments
Length of output: 404
🏁 Script executed:
cat -n src/features/registerCard/hooks/usePaymentsStep.tsRepository: woowacourse/react-payments
Length of output: 916
🏁 Script executed:
# Also check if there are any other uses of setInputRef to understand the pattern
rg -nP 'setInputRef' src/Repository: woowacourse/react-payments
Length of output: 989
🏁 Script executed:
cat -n src/features/registerCard/ui/fields/CardNumberField.tsx | head -80Repository: woowacourse/react-payments
Length of output: 2936
🏁 Script executed:
# Check the useInputFocus implementation to fully understand setInputRef
cat -n src/core/hooks/useInputFocus.tsRepository: woowacourse/react-payments
Length of output: 1170
🏁 Script executed:
# Check ExpiryDateField more carefully - see if it has multiple inputs and how it handles setStepRef
sed -n '74,100p' src/features/registerCard/ui/fields/ExpiryDateField.tsxRepository: woowacourse/react-payments
Length of output: 720
CardNumberField의 모든 입력 요소에서 setStepRef를 호출하지 마세요.
setStepRef를 4개의 입력 필드 각각에서 호출하면, 마지막 입력만의 ref가 유지되고 이전 inputs의 ref는 덮어써집니다.
단계별 포커스 관리를 위해서는 첫 번째 입력 필드(index 0)에서만 setStepRef(node)를 호출하거나, CardForm의 다른 필드들처럼 단일 입력 필드에서만 ref를 설정해야 합니다. 현재 패턴은 step 추적 시스템에 4번의 덮어쓰기를 초래합니다.
참고로 ExpiryDateField도 동일한 패턴을 가지고 있습니다(첫 번째 입력에서 setStepRef(node) 호출).
🤖 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/registerCard/ui/fields/CardNumberField.tsx` around lines 63 -
66, CardNumberField is calling setStepRef for every input which overwrites the
step ref; change the ref handler so it still calls setInputRef(node, index) for
all inputs but only calls setStepRef(node) when index === 0 (i.e., the first
input), mirroring the pattern used in ExpiryDateField/CardForm so only a single
step ref is registered.
Comment on lines
+33
to
+44
| const fieldData: FieldData = { | ||
| numbers: numbers.join(''), | ||
| expiryDate, | ||
| cvc, | ||
| password, | ||
| bank, | ||
| }; | ||
|
|
||
| const onRegister = (fieldData: FieldData) => { | ||
| navigate('/result', { | ||
| state: fieldData, | ||
| }); |
There was a problem hiding this comment.
결과 페이지로 넘기는 state 계약을 맞추고 민감정보 전달을 제거해주세요.
현재 fieldData를 그대로 전달하면 Result가 기대하는 cardNumbers: string[] 형태와 불일치하고(numbers는 단일 문자열), 동시에 cvc/password까지 라우팅 state에 포함됩니다. 이 상태로는 결과 화면 런타임 오류 가능성과 개인정보/보안 리스크가 함께 발생합니다.
수정 예시
- const onRegister = (fieldData: FieldData) => {
+ const onRegister = () => {
navigate('/result', {
- state: fieldData,
+ state: {
+ cardNumbers: numbers,
+ bank,
+ },
});
};
@@
- onRegister={() => onRegister(fieldData)}
+ onRegister={onRegister}🤖 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/pages/payments/Payments.tsx` around lines 33 - 44, The state passed to
navigate from onRegister is wrong and includes sensitive data; change the object
sent to navigate('/result', { state: ... }) so it matches the Result contract
and omits secrets: use cardNumbers: numbers (the string[]), keep expiryDate and
bank, and do NOT include cvc or password (remove numbers.join('') and the
cvc/password fields from fieldData used for routing); update the
FieldData->state construction inside onRegister (and any variable named
fieldData) so navigate receives only { cardNumbers: numbers, expiryDate, bank }.
| }, | ||
| resolve: { | ||
| alias: { | ||
| '@': path.resolve(__dirname, './src'), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Node.js ESM 환경에서 __dirname 사용 가능 여부와, Vite config(ESM)에서 fileURLToPath(import.meta.url) 기반 dirname 사용이 권장되는지 공식 문서 기준으로 확인해줘.
💡 Result:
Node.js의 ESM(ECMAScript Modules) 환경에서는 CommonJS에서 제공되던 dirname 및 filename 전역 변수를 기본적으로 사용할 수 없습니다 [1][2][3]. 1. Node.js ESM 환경에서의 dirname 사용 Node.js v20.11.0 및 v21.2.0부터는 import.meta.dirname 및 import.meta.filename 속성이 도입되어, 이를 통해 별도의 변환 없이 현재 모듈의 디렉토리 경로와 파일 경로를 직접 얻을 수 있습니다 [4][1]. 따라서 최신 Node.js 환경에서는 이 속성들을 사용하는 것이 공식적으로 권장됩니다 [1][2]. 2. Vite config (ESM)에서의 권장 사항 Vite 설정 파일(vite.config.js 등)이 ESM 모듈로 작성된 경우, 과거에는 fileURLToPath(import.meta.url)을 사용하여 dirname을 직접 구현하는 방식이 널리 사용되었습니다 [5][6]. 그러나 현재는 다음과 같은 관점에서 접근하는 것이 좋습니다: - Node.js 환경과 동일하게 import.meta.dirname을 사용하는 것이 가장 간결하고 현대적인 방법입니다 [1]. - 만약 하위 호환성이나 특정 빌드 도구의 동작을 고려해야 하는 경우, 여전히 fileURLToPath(import.meta.url) 기반의 구현이 안정적인 대안으로 사용됩니다 [4][5]. - Vite는 설정 파일 내에서 dirname 등을 사용할 수 있도록 지원하는 기능을 포함하고 있으나 [7], ESM 환경에서는 표준적인 import.meta.dirname 또는 fileURLToPath 방식을 사용하는 것이 명확하며 권장됩니다 [1][6]. 결론적으로, 최신 Node.js 환경에서는 import.meta.dirname을 사용하는 것이 가장 권장되며, 이전 버전과의 호환성이 필요하거나 관습적인 방식을 선호한다면 fileURLToPath(import.meta.url)을 사용하는 방식도 여전히 유효합니다 [4][1][5].
Citations:
- 1: https://www.sonarsource.com/blog/dirname-node-js-es-modules
- 2: https://nodejs.org/docs/latest/api/esm.html
- 3: https://nodejs.org/api/esm.html
- 4: https://stackoverflow.com/questions/46745014/alternative-for-dirname-in-node-js-when-using-es6-modules/65895043
- 5: https://humanwhocodes.com/snippets/2023/01/mimicking-dirname-filename-nodejs-esm/
- 6: __dirname in dependency not being handled by Vite vitejs/vite#8482
- 7: feat: better config
__dirnamesupport vitejs/vite#8442
🏁 Script executed:
# Read vite.config.ts to see the context around lines 11-12 and 47
cat -n vite.config.ts | head -60Repository: woowacourse/react-payments
Length of output: 1887
🏁 Script executed:
# Check package.json for Node.js version requirements
cat package.json | grep -A 5 -B 5 'engines\|node'Repository: woowacourse/react-payments
Length of output: 478
Lines 11-12에서 이미 계산한 dirname 변수를 Line 47의 alias에도 사용하세요.
Lines 11-12에서 ESM 환경의 호환성을 고려하여 dirname을 정의하고 있는데, Line 47의 path.resolve(__dirname, './src')는 이 안전한 패턴을 따르지 않습니다.
다음을 생각해보세요:
- Lines 11-12에서
typeof __dirname !== 'undefined' ? __dirname : ...패턴을 사용하는 이유가 무엇일까요? - Line 26에서 이미 정의된
dirname변수가 Line 47에서도 사용될 수 있다면, 같은 문제를 피할 수 있지 않을까요?
이미 계산된 dirname을 재사용하도록 Line 47을 정리해보세요.
🤖 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 `@vite.config.ts` at line 47, The alias entry for '@' currently calls
path.resolve(__dirname, './src') instead of reusing the already-computed dirname
variable; update the alias to use path.resolve(dirname, 'src') (or
path.join(dirname, 'src')) so it leverages the safe ESM-compatible dirname value
defined earlier (the dirname symbol) and avoid direct use of __dirname in the
alias configuration.
eastroots92
requested changes
May 20, 2026
eastroots92
left a comment
eastroots92
left a comment
There was a problem hiding this comment.
안녕하세요 콘티
전반적으로 아주 잘 구현해주셨네요! 👍
소소하게 고민해보시면 좋을 내용들은 리뷰 남겨두었어요!
질문에 대해 답변 드릴게요
1. validate 위치 (꼭 Props로 내려줘야하나?)
저는 꼭 그럴 필요는 없다고 생각해요.
validate하는 위치가 일관되고, validate하는 함수가 잘 추상화 되어있다면 굳이 부모가 아닌 validate 값을 필요로 하는 쪽에서 함수를 호출해서 쓰는 것도 괜찮다고 생각해요!
보통 아래 두가지 방식 중 하나를 고려해서 쓰는 것 같아요.
- useForm 처럼 상위에서 validate까지 한번에 관리.
- 사용하는 쪽에서 직접 Validate 함수를 호출해서 대응. (보통 input 입력시, submit 시 두 곳에서 관리)
이 부분에 대해선 정답은 없고, 저의 경우 보통 2번을 많이 사용하나 특별한 이유가 있진 않아요. (머쓱..)
굳이 이야기 하면 validate 에러는 동일하지만 에러를 표현하는 방식이 달라서 그렇게 했던 것 같아요.
2. 에러메세지는 어디인가요?
이 부분은 제가 온전히 이해하지 못한 것 같아요. 기능이 feature를 이야기 하는 것일까요?
3. 도메인 훅이 정말 필요없는가?
이번 미션에서 UI 상태를 컴포넌트에 둔 것은 단순히 규모가 작아서라기보단 규모도 규모지만 해당 상태가 컴포넌트 내부에서 관리해도 충분했기 때문이라고 생각해요.
결과적으로 추상화 정도를 어느정도로 할 것인가 문제라고 생각하는데요.
좋은 추상화는 코드를 파악하고 읽는데 어려움이 없어야 한다고 생각해요.
코드를 읽는데 너무 과도한 정보가 있어서 읽기 어렵다면 추상화를 고려할 수도 있고, 반대로 과도한 추상화로 정보가 없어 내용을 파악하기 힘든 경우도 있어요.
저의 경우 UI 상태를 추상화 할때 보통 저는 재사용 여지가 많으면 추상화 하고, 아닌 경우 그대로 두는 경우가 많이 있어요. 이미 해당 컴포넌트의 책임 역할 단위가 적절해서, UI와 비즈니스 로직이 어느정도 공존해도 유지보수나 가독성이 문제가 없는 경우가 많이 있었기 떄문이에요.
혹시 추가로 궁금한 점이 있다면 편하게 이야기 해주세요!
고생하셨습니다!
| RegisterCardResponse, | ||
| } from '@/entities/card/model/card'; | ||
|
|
||
| export const isRegisterCardErrorResponse = (error: unknown): error is RegisterCardErrorResponse => { |
There was a problem hiding this comment.
P5;
코드 내용을 보았을 때 RegisterCardError로 판단하기엔 모호한 것 같단 생각이 들었어요!
요 코드는 어떤 목적으로 만든 것일까요?
Comment on lines
+29
to
+34
| if (response.status === 400) { | ||
| const error = (await response.json()) as RegisterCardErrorResponse; | ||
| throw error; | ||
| } | ||
|
|
||
| throw new Error('카드 등록에 실패했습니다.'); |
There was a problem hiding this comment.
P3;
API 응답에서 에러 메시지를 내려주는 것으로 이해했는데요! 이렇게 하드코딩하는 방식이 아닌 서버에서 내려준 message를 활용하는 방식으로 대응 해볼 수 있을까요?
Comment on lines
+52
to
+54
| if (response.status !== 204) { | ||
| throw new Error('카드 삭제에 실패했습니다.'); | ||
| } |
There was a problem hiding this comment.
P4;
204가 아닌경우 무조건 에러를 발생시키는게 정말 좋을까? 하는 생각이 들었어요!
물론 스펙상 204로 내려주기로 약속했으니 큰 문제는 아니라고 생각해요!
|
|
||
| const onRegister = async () => { | ||
| const { cardInfo } = paymentsForm; | ||
| await registerCard(toRequestData(cardInfo)); |
Comment on lines
+14
to
+24
| if (status === 'idle') return <CardListLoading />; | ||
| if (status === 'loading') return <CardListLoading />; | ||
| if (status === 'error') return <CardListError onRetry={() => void refetch()} />; | ||
| if (cards.length === 0) return <CardListEmpty onAddCard={handleAddCard} />; | ||
|
|
||
| return ( | ||
| <CardList | ||
| cards={cards} | ||
| onAddCard={handleAddCard} | ||
| onDelete={(cardId) => void removeCard(cardId)} | ||
| /> |
|
|
||
| export const CardListPage = () => { | ||
| const navigate = useNavigate(); | ||
| const { cards, status, refetch, removeCard } = useCardList(); |
There was a problem hiding this comment.
P2;
removeCard는 사용하는 쪽에서 호출해서 사용해도 될 것 같은데 여기서 관리하는 이유가 있을까요?
hooks 이름에도 부합되지 않고, 굳이 여기서 할 이유가 없을 것 같아 여쭈어봐요!
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.
🎯 페이먼츠
3단계
idle | loading | success | error네 가지로 명시적으로 관리한다.🕵️ 셀프 리뷰(Self-Review)
제출 전 체크 리스트
리뷰 요청 & 논의하고 싶은 내용
1) 이번 단계에서 가장 많이 고민했던 문제와 해결 과정에서 배운 점
사용자 인터렉션 로직과 모델로직의 분리
이번단계에서 가장 고민했던건 touched입니다.
필드들을 카드인포(하나의 객체)로 묶는 것부터 고민이 시작됐습니다. 등록페이지에서 제출 버튼이 렌더링되는 조건은 모든 필드가 유효할때이니 상태를 가지고 있는 곳에서 에러를 반환해야겠다고 생각했습니다.
그렇게 데이터만 다루는 로직(상태와 유효성 검증)을 훅으로 분리하고 사용자가 직접 반응하는 UI적인 부분(touched, focus)은 필드 컴포넌트에 두게 되었습니다.
그 후 API 명세를 보니 제출 실패 시 서버에서 에러메세지를 주고 해당 에러가 난 부분으로 포커스를 이동하고 메세지를 띄워줘야됐습니다.
이대로 구현하면 좋겠다고 생각했습니다, 하지만..
이 명세를 보고나니 필드 내부에서 계산하는 것이 아니라, 폼이나 관련 훅에서 어떤 것을 렌더링할지 데이터만 필드에게 전달해주고 필드 컴포넌트를 멍청하게(Dumb) 만들면 로직이 편해질 것이라 생각했습니다.(리액트 훅 폼도 이벤트를 최상단에서 관리하더라고요)
하지만 그렇게 전부 끌어올리려면 스텝과 터치, UI적인요소 전부 하나의 훅에서 관리되는 구조(유저의 이벤트 순서에따른 처리때문에 이렇게 생각했습니다)가 된다고 생각하여 아래와 같은 결론이 났습니다.
2) 이번 리뷰를 통해 논의하고 싶은 부분
validate
훅에서 isValid 같은 파생값을 내려줘도
handleChange때 계산할 수 없어 validate를 내려줘서 계산하게 했는데사실은 필드 컴포넌트에서 import해서 사용하면 되지않나? 고민했습니다. (완료 조건(validate)를 props로 내려줄 필요성이 있나? 생각했습니다)
에러메세지는 어디인가요?
엔티티라는 계층에선 도메인 모델의 유효성검사와 타입을 정의하고 카드 등록같은 기능에서 에러메세지를 정의해 사용했습니다.
그런데 에러메세지란 여러곳에서 쓰일 수 있어서 어느 계층에 둘지 고민했습니다
지금은 기능내에서만 사용한다고 생각하여 기능/모델에 넣었습니다.
프리뷰의 위치
FSD 질문입니다.
카드 프리뷰의 경우 세 가지 경우로 생각했습니다
처음엔 카드가 표현라는 ui니 엔티티에 넣으려했으나 UI로 표현하는 것들이 너무 많았습니다.
그래서 기능과 페이지중 고민하다 페이지로 이동시켰습니다 (렌더링만을 책임지기때문)
가장 궁금한 점
가장 궁금한점은 도메인 훅이 정말 필요없는가였습니다.
이번 미션이 규모가 작기때문에 만들지 않은 것인지, 파일이 커지게된다면 각 컴포넌트를 헤드리스하게 만들고 렌더링만을 담당하게해야하는것이 아닌지 의문이 들어 자꾸 ui훅도 만들어 상태를 끌어올리려는 욕구에 휩쌓였습니다.이번 미션에서 규모가 작기때문에 ui상태를 컴포넌트에 둔것인가요? 나중에 규모가 커지면 인터렉션로직도 전부 훅으로 분리하게 되나요 아니면 그대로 컴포넌트에 드러나게 두나요?
✅ 리뷰어 체크 포인트
1. Form 상태 관리 & Custom Hook 분리
2. 입력 UI 흐름과 UX
3. 컴포넌트 구조 및 재사용성
4. 상태 기반 유효성 검사 및 확인 버튼 활성화
5. 비동기 상태 · 네트워크 경계 · 통합 테스트
idle | loading | success | error네 가지로 명시적으로 관리하고,isLoading/error를 별도 boolean으로 쪼개지 않았는가?POST/GET/DELETE /cards와 400 시나리오까지 포함하여 네트워크 경계에서 동작하는가?fetch·axios를 모킹하지 않고, MSW + RTL로 사용자 관점에서 작성되었는가?getByRole → getByText → getByLabelText → getByTestId우선순위를 따르고, 비동기 요소에findBy*를 사용했는가?