[페이먼츠 2단계 - hooks & state] 지오(장주형) 미션 제출합니다. #536
Open
JuHyeong424 wants to merge 69 commits into
Open
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Walkthrough이 PR은 카드 결제 입력 폼의 아키텍처를 포괄적으로 리팩토링했다. 기존의 CardInfo 컴포넌트가 지역 상태 관리(useState)를 통해 카드번호, 유효기간, CVC를 처리하던 방식에서 벗어나, 각 입력 필드마다 독립적인 Context(CardNumberContext, ExpireDateContext, CvcContext, CardBrandContext, PasswordContext)를 구성하고 PaymentProvider로 통합했다. 동시에 카드 입력 필드들의 렌더링 순서를 조건부 검증(canShowCardBrand, canShowExpireDate 등)으로 제어하는 방식으로 전환했다. Diners, AMEX, UnionPay 브랜드 지원을 추가하고, BrowserRouter를 이용한 클라이언트 라우팅을 도입하여 완료 화면(Enrollment)을 별도 경로로 분리했으며, TypeScript strict 모드와 ESNext 타겟 설정도 적용했다. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
24-68:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win체크리스트 표기가 GitHub Task List 문법과 다릅니다.
[v]는 체크박스로 렌더링되지 않아서, 문서 가독성을 위해[x](완료) /[ ](미완료) 형식으로 바꾸는 게 좋습니다.✅ 제안 수정안 (예시)
-- [v] 숫자(0~9)만 입력 가능 +- [x] 숫자(0~9)만 입력 가능전체 항목에 동일 규칙으로 일괄 적용해 주세요.
🤖 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 24 - 68, Replace all checklist markers using the non-standard token "[v]" with GitHub Task List syntax "[x]" for completed items and use "[ ]" for any incomplete items; specifically update every instance of "[v]" in the given README sections (card number, 유효기간 입력, CVC 입력, 실시간 프리뷰, 동적 입력 UI 구현, 카드사 선택, 폼 제출 및 상태 관리, 카드 등록 완료 및 네비게이션) so they render as GitHub checkboxes (e.g., change "- [v] 숫자(0~9)만 입력 가능" to "- [x] 숫자(0~9)만 입력 가능"), keeping the rest of the text unchanged.
🧹 Nitpick comments (27)
src/components/cardPreview/CardPreviewInfo.tsx (1)
12-13: 💤 Low value의미 있는 prop으로 매핑하면 호출부 가독성이 좋아집니다.
여기서
"10px"/"0px"를 직접 넘기고 있는데, 만일CardPreviewNumber측에서variant/spaced같은 의미 단위 prop으로 받게 바꾼다면, 이 호출부도 “이 줄은 카드번호 영역이라 띄움이 있고, 저 줄은 만료일 영역이라 안 띄운다”는 의도가 코드로 자연스럽게 드러납니다. (CardPreviewNumber 쪽 코멘트와 함께 보세요.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/cardPreview/CardPreviewInfo.tsx` around lines 12 - 13, 현재 CardPreviewInfo 호출부는 CardPreviewNumber에 gap="10px"/"0px" 같은 스타일 값을 직접 전달해 의도가 불분명합니다; CardPreviewNumber 컴포넌트 API를 의미 기반 prop(예: variant="number" | "expiry" 또는 spaced={true|false})으로 변경하고, CardPreviewNumber를 사용하는 곳에서 gap 대신 해당 의미 있는 prop을 전달하도록 maskCardNumber(cardNumber) 호출 줄과 formatExpireDate(expireDate) 호출 줄을 각각 업데이트하세요; 이때 CardPreviewNumber 내부에서 variant/spaced를 받아 기존 gap 로직을 매핑하도록 유지해 하위 컴포넌트만 수정하면 호출부 의도가 명확해집니다.src/components/cardInfo/cardNumber/constants.ts (1)
1-18: 💤 Low value구성이 카드사 표준과 일치합니다 — 타입화만 한 번 고민해 보세요.
Diners(14자리: 4-6-4)와 AMEX(15자리: 4-6-5) 분할 모두 표준과 맞습니다. 한 가지만 짚어드리면, 세 상수가 모두
{ name, placeholder, maxLength }[]형태로 동일한 모양을 갖는데,src/types/types.ts의InputConfig를 활용해const ... : InputConfig = [...]로 명시적으로 타입을 붙여두면 어떤 이점이 있을까요? (힌트: 향후 필드 추가/변경 시 컴파일 타임 보호,name이 단순string이 아닌 유한 집합이라면as const또는 리터럴 유니언 활용)
name값을 자유로운string대신 의미 있는 리터럴 유니언("first" | "second" | ...)으로 좁히면 어떤 유즈케이스에서 도움이 될지 한 번 생각해 보세요.As per coding guidelines: "Ask leading questions and provide hints instead of just telling the answer."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/cardInfo/cardNumber/constants.ts` around lines 1 - 18, The three exported constants (INPUT_CARD_NUMBER_CONFIG, DINERS_INPUT_CARD_NUMBER_CONFIG, AMEX_INPUT_CARD_NUMBER_CONFIG) share the same shape and should be explicitly typed with the existing InputConfig type from src/types/types.ts to get compile-time protection; update each declaration to use : InputConfig and import the type, and consider narrowing the `name` property to a literal union (or use `as const`) so `name` isn't just `string`—this will enforce correct field names and catch future shape changes at compile time (hint: change the exports to typed constants and/or apply `as const` to the arrays to preserve literal types).src/components/cardPreview/CardPreviewNumber.tsx (1)
4-13: 💤 Low value
gapprop의 타입을 더 좁혀볼 여지가 있습니다.
id: "cardNumber" | "expireNumber"디스크리미네이터에서 표현형gap: string으로 바꾸면서 사용처가 단순해진 점은 좋습니다. 다만 현재 사용처가"10px"/"0px"두 가지뿐인 만큼,gap을 그대로 임의의 CSS 문자열로 열어두는 대신
- 리터럴 유니언(
gap: "10px" | "0px")으로 좁히거나,- CSS 의존을 끊고 의미 있는 prop(
variant: "number" | "expire"또는spaced: boolean)으로 매핑하는 방식중 어느 쪽이 호출부 가독성과 디자인 일관성 측면에서 더 나을지 한 번 고민해 보시면 좋겠습니다. 외부에 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/components/cardPreview/CardPreviewNumber.tsx` around lines 4 - 13, CardPreviewNumber's CardNumberProps currently types gap as string; tighten it by changing gap to a literal union or a semantic prop: either set gap: "10px" | "0px" (narrow literal union) or replace gap with a semantic prop like variant: "number" | "expire" or spaced: boolean and map that to the appropriate $gap value inside CardPreviewNumber (update the CardNumberWrapper usage and any callers of CardPreviewNumber accordingly) so callers don't pass arbitrary CSS strings and design tokens remain easier to migrate.src/components/sendButton/SendButton.tsx (1)
9-9: ⚡ Quick win경로 prefix
/react-payments/의 하드코딩.GitHub Pages 배포용 basename을 라우터 레벨에서 한 번 설정(
<BrowserRouter basename="/react-payments">)하면 컴포넌트 안에서는navigate("/enrollment")만 사용해도 됩니다. 현재처럼 컴포넌트마다 prefix를 손으로 적게 되면 배포 경로가 바뀌었을 때 누락이 생기기 쉽습니다. 라우트 상수나 enum으로 추출하는 것도 좋은 대안입니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/sendButton/SendButton.tsx` at line 9, The Button in SendButton.tsx hardcodes the "/react-payments/enrollment" path in its onClick handler (navigate call), which duplicates the router basename and risks broken links on deployment; change the navigate invocation to use the basename-free route (e.g., navigate("/enrollment")) or reference a central route constant/enum instead so the component uses navigate("/enrollment") (or ROUTES.ENROLLMENT) and relies on BrowserRouter basename to prepend "/react-payments".src/context/cvc/CvcContext.ts (2)
1-15: 💤 Low value사소한 정돈 두 가지와 표현 관례 하나만 같이 점검해 보세요.
import { createContext } from "react";와import { useContext } from "react";가 두 줄로 분리되어 있는데, 같은 모듈에서 가져오는 식별자는 한 줄로 묶는 편이 일반적입니다. 어떤 이점이 있을까요?throw Error("...")도 동작은 하지만, JavaScript 관례적으로throw new Error("...")형태가 더 널리 쓰입니다. 두 형식의 차이가 실제로 있는지 한 번 확인해 보시면 좋습니다(힌트:Error는 함수로 호출해도new와 동등하게 동작하도록 명세에 정의되어 있습니다 — 그럼에도 관례를 따르는 이유는 무엇일까요?).- 에러 메시지가 한국어 메시지인데, 사용자 노출이 아닌 “개발자에게 잘못된 사용을 알리는 invariant 메시지”라면 영문으로 일관되게 두는 편이 도구 친화적이고 검색 가능성도 좋습니다. 팀 컨벤션에 비추어 결정해 보세요.
As per coding guidelines: "Ask leading questions and provide hints instead of just telling the answer."
🤖 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/context/cvc/CvcContext.ts` around lines 1 - 15, The file exports CvcContext and useCvcContext; please tidy imports by merging the two React imports into one (e.g., import { createContext, useContext } from "react") to follow common style, change the thrown error in useCvcContext from throw Error(...) to throw new Error(...) to match JS conventions, and consider replacing the Korean invariant message with an English message (or confirm team convention) so developer-facing errors are consistent and searchable — you can find these changes in the CvcContext and useCvcContext symbols.
5-15: ⚡ Quick win다섯 개의 Context가 동일한 보일러플레이트를 반복합니다 — 추상화 가능성을 검토해 보세요.
CardNumberContext,ExpireDateContext,CvcContext,CardBrandContext,PasswordContext모두 정확히 같은 패턴을 따릅니다:
createContext<Type | null>(null)use*Context훅에서 null 체크 후 Error throw추가로 import 스타일(한 줄 vs 두 줄)과 에러 메시지(띄어쓰기, 동사 차이)도 일관되지 않습니다.
동일한 패턴을 다섯 번 반복하는 대신 제네릭 헬퍼로 통합했을 때:
- 가독성과 유지보수성은 어떻게 변할까요?
- 에러 메시지의 일관성 이점이 있을까요?
- 반대로, 각 도메인의 특수성 때문에 현재 형태가 더 나은 경우는 어떤 상황일까요?
- 향후 새 도메인을 추가할 때의 비용은 줄어들까요?
이 과정을 통해 모듈화와 추상화 의사결정 감각을 다질 수 있을 것 같습니다.
🤖 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/context/cvc/CvcContext.ts` around lines 5 - 15, Extract the repeated pattern of createContext<Type | null>(null) + null-checking hook into a generic helper (e.g., createNullableContext<T>(label?: string) returning { Context, useContextHook }) and replace the five duplications (CardNumberContext, ExpireDateContext, CvcContext, CardBrandContext, PasswordContext and their use*Context hooks) to use this helper; ensure the helper standardizes the thrown Error message (consistent wording/spacing) and import style, and update references like useCvcContext to call the generated hook from the helper so behavior and typings remain identical.src/types/types.ts (1)
18-63: ⚡ Quick win컨텍스트 값 타입의 일관성과 표현력을 한 번 점검해 보세요.
두 가지 정도 검토해 보시면 좋겠습니다.
- 핸들러 네이밍 일관성:
CvcContextValue.handleBlur,PasswordContextValue.handleBlur는 제네릭한 이름인 반면,ExpireDateContextValue는handleMonthBlur/handleYearBlur로 구체적입니다. 컨텍스트 안에서handleBlur라는 이름만 봤을 때 어떤 필드의 블러인지 즉시 식별 가능한가요? 도메인 이름을 접두/접미로 통일하면(handleCvcBlur,handlePasswordBlur) 사용 측 가독성에 어떤 변화가 있을지 생각해 보세요.cardType: string | null: 지원 브랜드가 한정되어 있다면("VISA" | "MASTER" | "AMEX" | "DINERS" | "UNIONPAY" | ...) 어떤 이점이 있을까요? 다운스트림에서 카드 타입 분기 로직을 작성할 때 컴파일러가 어떻게 도와줄 수 있을지,selectCardType유틸의 반환 타입과 연결지어 한 번 검토해 보시면 좋습니다.힌트: 브랜드 상수를 단일 출처(SSOT)로 두고 거기에서
typeof BRANDS[number]같은 패턴으로 유니언 타입을 파생시키는 방식도 있습니다.As per coding guidelines: "Promote modular thinking—breaking problems into reusable components" and "Ask leading questions and provide hints instead of just telling the answer."
🤖 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/types/types.ts` around lines 18 - 63, Summary: Context handler names and cardType typing are inconsistent and should be made explicit and type-safe. Change generic handleBlur properties to field-specific names (e.g., CardNumberContextValue.handleBlur -> handleCardNumberBlur, CvcContextValue.handleBlur -> handleCvcBlur, PasswordContextValue.handleBlur -> handlePasswordBlur; keep ExpireDateContextValue.handleMonthBlur/handleYearBlur as-is) and update all usages accordingly; replace cardType: string | null in CardNumberContextValue with a discriminated union derived from a single-source-of-truth BRANDS constant (or enum) so cardType has type like CardBrand | null and align/selectCardType’s return type to that union; ensure any consumers, tests, and InputConfig/selectCardType signatures are updated to the new names and types.src/hooks/useFocusFirstInput.ts (1)
6-14: ⚡ Quick win의존성 배열 동작과 입력 후보 선정에 대해 한 번 점검해 보세요.
몇 가지 짚어볼 만한 지점이 있습니다.
useRef가 반환하는 ref 객체는 컴포넌트 생애주기 동안 동일한 참조를 유지합니다. 그렇다면[formRef]를 의존성으로 두는 것과[]를 두는 것이 실제로 어떻게 다를까요? 카드 브랜드가 바뀌어 입력 칸 개수/구성이 달라지는 경우(예: AMEX로 전환)에도 이 effect가 다시 실행되길 기대하시는지 한 번 의도를 정리해 보시면 좋겠습니다.querySelector("input")은disabled/hidden/type="hidden"요소도 매칭합니다. 만약 첫 번째 input이 비활성 상태라면 어떤 동작을 기대하시나요? CSS 선택자에서:not(:disabled)또는:enabled같은 가상 클래스를 활용하면 포커스 가능한 첫 번째 요소를 식별할 수 있습니다. 이런 방식이 케이스에 맞을지 생각해 보세요.관련해서 React 19에서 ref 콜백 형태도 변경되었으니, 추후 ref 콜백을 쓰는 경우 반환값이 있으면 TypeScript 오류가 발생한다는 점도 알아두면 좋습니다.
🤖 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/hooks/useFocusFirstInput.ts` around lines 6 - 14, The effect in useFocusFirstInput should run when the actual form element changes (not the ref object) and should pick a truly focusable input; update the dependency from [formRef] to [formRef.current] (or accept an external deps array if you need re-run on card-brand changes) and change the selector to something like 'input:not(:disabled):not([type="hidden"]):not([hidden])' to avoid disabled/hidden inputs; finally guard the focus call by ensuring the node is an HTMLElement and call .focus() safely (e.g., const firstInput = formRef.current?.querySelector(...) as HTMLElement | null; if (firstInput && typeof firstInput.focus === "function") firstInput.focus()).package.json (1)
22-22: 💤 Low value
react-router-domv7 마이그레이션 가이드 검토 추천v7은 v6 대비 패키지 통합으로 인한 메이저 업그레이드입니다. 현재 코드에서 사용 중인 API(
useNavigate,BrowserRouter,MemoryRouter,Route,Routes)는 v7에서도 완벽하게 지원되며, react-router-dom 패키지는 v7에서도 react-router의 재수출로 유지되어 호환성에 문제가 없습니다.다만, v7 마이그레이션 모범 사례로는 다음을 고려해볼 수 있습니다:
- 임포트 경로를
react-router-dom에서react-router(또는 DOM 코드의 경우react-router/dom)로 업데이트하는 것이 권장됩니다.- 필요시 Future Flags를 단계적으로 활성화하여 증분 마이그레이션 가능합니다.
현재 코드가 정상 작동하므로 긴급한 변경은 아니지만, 향후 라우팅 기능을 확장할 때 v7의 마이그레이션 가이드를 참고하면 좋겠습니다.
🤖 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 22, 패키지 업데이트 관련: 현재 package.json에 "react-router-dom": "^7.15.0"을 사용 중인 것을 확인했으니, v7 마이그레이션 권장사항을 적용하려면 코드베이스에서 라우터 관련 임포트(예: useNavigate, BrowserRouter, MemoryRouter, Route, Routes)를 점검하고 필요한 경우 import 경로를 "react-router" 또는 DOM 전용 코드인 경우 "react-router/dom"으로 변경하고, 단계적 마이그레이션이 필요하면 v7의 Future Flags를 검토하여 점진적으로 활성화하는 변경을 적용하세요.src/utils/validators.ts (1)
17-23: ⚡ Quick win불리언 반환을 명시해 조건 함수 의미를 고정해 주세요.
Line [17]~Line [23]의
canShowExpireDate는 현재string | false가 반환될 수 있습니다. predicate 함수라면 항상boolean이 되도록 정리하는 편이 이후 조건식/타입 추론이 덜 흔들립니다.🤖 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/utils/validators.ts` around lines 17 - 23, canShowExpireDate currently can return a non-boolean (string | false) because it returns values like selectedItem.brand directly; change the function to explicitly return a boolean (e.g., annotate return type as boolean) and coerce each truthy check to boolean (use !! or Boolean(...)) so the expression inside canShowExpireDate always yields a boolean; update the export const canShowExpireDate(signature) and the return expression referencing cardBrandContext.isOpen, cardBrandContext.selectedItem.brand, and cardBrandContext.selectedItem.color accordingly.src/components/cardInfo/password/Password.stories.tsx (1)
17-25: ⚡ Quick win스토리가 실제 컴포넌트를 직접 렌더링하지 않아 동기화가 깨질 수 있습니다.
Line 17-25는
Password와 동일 UI를 스토리 내부에서 다시 작성하고 있습니다. 이렇게 두면 실제 컴포넌트 변경이 스토리에 누락될 수 있어, Provider 안에서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/components/cardInfo/password/Password.stories.tsx` around lines 17 - 25, 현재 스토리가 Password와 동일한 UI를 다시 작성하고 있어 동기화가 깨질 위험이 있습니다; PasswordProvider 내부에서 직접 Password 컴포넌트를 렌더링하도록 변경하세요: PasswordProvider로 감싸진 Wrapper/Header/SubHeader/Label/Input의 중복된 마크업을 제거하고 대신 Password 컴포넌트를 호출하도록 바꿔(참조 대상: PasswordProvider, Password, Wrapper, Header, SubHeader, Label, Input), 필요한 경우 Password가 받는 props를 스토리에서 전달하여 기존 시나리오를 재현하도록 보장하세요.src/context/password/PasswordProvider.tsx (1)
5-5: ⚡ Quick win의미와 다른 변수명으로 컨텍스트 의도가 흐려집니다.
Line 5의
cvc는 실제로 비밀번호 상태 훅 결과를 담고 있어서 읽는 사람이 쉽게 오해합니다. 의미가 맞는 이름으로 맞춰 두는 게 이후 유지보수에 안전합니다.🤖 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/context/password/PasswordProvider.tsx` at line 5, The local variable cvc is misnamed and should be renamed to a clear identifier that reflects the hook result from usePasswordInput (e.g., passwordState, passwordInput, or passwordContext) to match intent; change the declaration const cvc = usePasswordInput() to const passwordState = usePasswordInput() (or a chosen name) and update every usage in PasswordProvider (and any exported/context value names like PasswordProvider or PasswordContext) to the new identifier so references remain consistent and typings/props continue to work.src/components/cardInfo/expireDate/Input.tsx (2)
26-26: 💤 Low value화살표 함수 래퍼 간소화 고려
onBlur={() => handleMonthBlur()}와onBlur={() => handleYearBlur()}를 각각onBlur={handleMonthBlur},onBlur={handleYearBlur}로 간소화할 수 있습니다.앞서 CVC Input에서 언급한 것과 동일한 패턴입니다. blur 핸들러들이 이벤트 객체를 인자로 받지 않는다면 직접 전달해도 됩니다.
Also applies to: 42-42
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/cardInfo/expireDate/Input.tsx` at line 26, Replace the arrow-function wrappers on the onBlur props with the handler references directly: change occurrences of onBlur={() => handleMonthBlur()} and onBlur={() => handleYearBlur()} to onBlur={handleMonthBlur} and onBlur={handleYearBlur} respectively (in the Input component where these handlers are used) — only do this if handleMonthBlur and handleYearBlur do not require the synthetic event argument or binding.
21-21: ⚡ Quick winform 요소 구조에 대해 재고해보세요
월(month)과 연도(year) 입력이 각각 별도의 form 요소(FieldWrapper)로 감싸져 있습니다.
다음 질문들을 고려해보세요:
- 월과 연도가 함께 하나의 유효기간을 구성하는데, 왜 별도의 form으로 분리되어 있을까요?
- 첫 번째 form에만 useFocusFirstInput이 적용되는데, 이것이 의도한 동작인가요?
- 만약 form 요소가 실제 제출 기능 없이 스타일링/구조화 목적이라면 div 등의 다른 요소를 사용하는 것은 어떨까요?
생각해볼 대안:
- 월/년도 input을 하나의 form 또는 div로 감싸기
- 아니면 form을 사용하지 않고 상위 컨테이너만 사용하기
HTML 시맨틱 측면에서 form 요소는 제출 가능한 폼을 나타내는데, 현재 용도에 적합한지 검토해보세요.
Also applies to: 35-35, 69-69
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/cardInfo/expireDate/Input.tsx` at line 21, 현재 코드가 month/year 각각을 별도의 FieldWrapper(form)로 감싸고 있어 시맨틱에 맞지 않으므로 FieldWrapper 요소 구조를 재구성하세요: month와 year 입력을 하나의 상위 컨테이너(선택: 하나의 form 또는 단순한 div)로 묶고 불필요한 중복 form 태그를 제거하며 FieldWrapper 컴포넌트가 실제 폼 제출이 필요 없다면 form이 아닌 div로 렌더링하도록 변경하세요; 또한 useFocusFirstInput이 적용되는 위치(useFocusFirstInput, formRef, 첫 번째 입력)를 확인해 동일한 컨테이너 안에서 첫 입력에만 포커스 훅이 적용되도록 조정하세요.src/components/cardInfo/cvc/Input.tsx (2)
18-18: 💤 Low value화살표 함수 래퍼 간소화 고려
onBlur={() => handleBlur()}를onBlur={handleBlur}로 간소화할 수 있습니다.다만 다음을 확인해보세요:
- handleBlur가 이벤트 객체를 인자로 받지 않는다면 직접 전달해도 됩니다
- 만약 이벤트 전파를 막거나 다른 로직이 필요하다면 현재 방식이 맞습니다
19번 라인의 경우
e.target.value를 추출해야 하므로 화살표 함수가 필요합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/cardInfo/cvc/Input.tsx` at line 18, The onBlur prop currently wraps handleBlur in an unnecessary arrow function; replace onBlur={() => handleBlur()} with onBlur={handleBlur} in the Input component if the handleBlur handler does not require the event argument, keeping the arrow wrapper only where you need to access e.target.value (as noted for the other case on line 19); locate the onBlur usage and the handleBlur function reference to make this substitution to avoid the extra closure.
8-9: 💤 Low valueform ref와 useFocusFirstInput의 필요성을 고려해보세요
현재 단일 input 요소만 있는데 form 요소를 만들고 useFocusFirstInput을 적용하고 있습니다.
다음을 생각해보세요:
- useFocusFirstInput 훅은 여러 input이 있을 때 첫 번째 input을 찾아 포커스하는 용도인가요?
- 단일 input의 경우 input에 직접 ref를 연결하고 useEffect로 포커스하는 것과 비교했을 때 어떤 장단점이 있을까요?
- form 요소가 실제로 form 제출 기능을 제공하나요, 아니면 단순히 스타일링 컨테이너인가요?
현재 구조가 다른 input 컴포넌트들과의 일관성을 위한 것이라면 합리적일 수 있습니다.
Also applies to: 13-13
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/cardInfo/cvc/Input.tsx` around lines 8 - 9, The formRef + useFocusFirstInput setup is unnecessary for a single input: remove the form element and the useFocusFirstInput call, attach a ref directly to the input element (e.g., inputRef) inside the Input component and implement a small useEffect to focus it on mount; if you must keep a form for consistency, leave formRef/useFocusFirstInput but confirm the form actually provides submit behavior—otherwise remove the form wrapper entirely and use the direct inputRef+useEffect approach to simplify and clarify behavior.src/components/cardInfo/cvc/Cvc.tsx (1)
7-8: 💤 Low valueHeader와 Label의 텍스트 중복을 확인해주세요
7번과 8번 라인에 동일한 텍스트 "CVC 번호를 입력해 주세요"가 사용되고 있습니다.
다음을 고려해보세요:
- Header(h1)와 Label이 같은 내용을 표시하는 것이 의도된 디자인인가요?
- 일반적으로 Label은 더 짧거나 구체적인 텍스트(예: "CVC 번호")를 사용합니다
- 접근성 측면에서 스크린 리더 사용자에게 중복된 정보가 제공될 수 있습니다
디자인 명세를 확인하고, 필요하다면 하나는 제거하거나 텍스트를 다르게 구성하는 것을 고려해보세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/cardInfo/cvc/Cvc.tsx` around lines 7 - 8, Header 컴포넌트와 Label 컴포넌트가 Cvc.tsx에서 동일한 텍스트 "CVC 번호를 입력해 주세요"를 사용해 중복되어 있으니 디자인/접근성 의도에 맞게 하나를 수정하거나 제거하세요; 구체적으로 Cvc.tsx의 Header는 페이지/섹션 제목으로 유지하고 Label은 더 짧은 식별자(예: "CVC 번호" 또는 "CVC")로 변경하거나 Label을 제거해 화면 읽기 도구에 중복 정보를 주지 않도록 수정하십시오.src/hooks/useExpireDateInput.ts (2)
21-57: ⚖️ Poor tradeoff월(month) 입력 검증 로직의 구조를 재검토해보세요
handleMonthChange 함수의 유효성 검사 로직이 복잡합니다. 다음 사항들을 고려해보세요:
검증 순서와 구조
- 라인 31, 36, 41의 if 문들이 각각 독립적으로 실행됩니다
- 이들이 if-else-if 체인으로 구성되어야 할까요, 아니면 현재처럼 독립적이어야 할까요?
- 여러 조건이 동시에 true가 될 수 있다면 어떤 에러 메시지가 최종적으로 표시될까요?
엣지 케이스 테스트
다음 입력값들에 대해 어떻게 동작하는지 확인해보세요:
- "0"만 입력했을 때 (한 글자)
- "00"을 입력했을 때
- "13"을 입력했을 때
- "a1"을 입력했을 때
firstDigit 추출 위치
- 라인 24에서
value.substring(0, 1)을 사용하는데, value는 아직 정제되지 않은 원본입니다- 라인 22의 onlyNumbers를 사용하는 것과 어떤 차이가 있을까요?
디버깅 팁: console.log를 추가하여 각 조건이 언제 실행되는지 확인해보세요.
🤖 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/hooks/useExpireDateInput.ts` around lines 21 - 57, handleMonthChange has overlapping independent ifs that read uncleaned value and can overwrite errors; change validation to operate on the cleaned onlyNumbers (use onlyNumbers and sliceValue for firstDigit and numeric checks), convert the validation branches into a single if-else-if chain so only one setExpireDateError runs, and treat the single-character "0" case explicitly (e.g., show numeric-range error until two digits or allow entry but validate on blur). Also keep the existing yearInputRef.focus() behavior but base it on onlyNumbers.length === 2, and ensure you still call setExpireDate with sliceValue at the end.
81-95: ⚡ Quick winBlur 핸들러의 유효성 검사를 개선할 수 있습니다
handleMonthBlur와 handleYearBlur가 길이만 체크하고 있습니다.
다음을 생각해보세요:
- 이미 change 핸들러에서 에러가 설정된 경우 blur에서도 다시 메시지를 덮어써야 할까요?
- "완전히 입력해 주세요"라는 메시지가 "숫자를 입력해주세요"보다 우선순위가 높은가요?
- blur 시점에 기존 에러가 있다면 그대로 유지하는 것이 더 나은 UX일 수 있습니다
힌트: blur 핸들러에서 에러 메시지를 설정하기 전에 현재 에러 상태를 확인하는 조건을 추가해보세요.
🤖 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/hooks/useExpireDateInput.ts` around lines 81 - 95, handleMonthBlur and handleYearBlur currently overwrite any existing error with a length-only message; update both to first check the current error state (expireDateError.month and expireDateError.year) and only set "완전히 입력해 주세요." when length < 2 AND there is no existing error that should be preserved (e.g., a numeric-format error set by the change handler). Concretely, in handleMonthBlur and handleYearBlur, read expireDateError.month/year and only call setExpireDateError(...) to set the "완전히 입력해 주세요." message if expireDate.month.length < 2 AND expireDateError.month (or .year) is falsy (or not a higher-priority message), otherwise leave the existing error unchanged.src/components/conditional/ConditionalPassword.tsx (1)
12-21: ⚡ Quick winJSX 구조를 단순화할 수 있습니다.
중첩된 Fragment가 불필요해 보입니다. 현재 구조를 보면:
<> {validation && ( <> {children} <Password /> </> )} </>바깥쪽 Fragment를 제거하는 방법을 생각해보세요. Fragment는 언제 필요하고 언제 생략할 수 있을까요?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/conditional/ConditionalPassword.tsx` around lines 12 - 21, The JSX currently renders an unnecessary outer Fragment in ConditionalPassword's return; simplify by returning the conditional fragment directly—use the validation check to return either the inner Fragment containing {children} and <Password /> or null/false, removing the redundant outer <>...</>; update the return in the ConditionalPassword component so it directly returns (validation && (<>{children}<Password /></>)) or the equivalent ternary expression.src/components/cardInfo/cardNumber/CardNumber.stories.tsx (1)
31-63: ⚡ Quick win스타일 컴포넌트 중복이 발견되었습니다.
ExpireDate.stories.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 `@src/components/cardInfo/cardNumber/CardNumber.stories.tsx` around lines 31 - 63, Wrapper, Header, SubHeader and Label are duplicated styled-components across multiple stories (including CardNumber.stories and ExpireDate.stories); extract these shared styled components into a single module (e.g., export named components from a new shared story-styles file) and replace the local definitions in CardNumber.stories.tsx and ExpireDate.stories.tsx with imports of those named exports, keeping the same component names (Wrapper, Header, SubHeader, Label) so existing usage requires minimal changes and remove the duplicated definitions in each story file.src/components/cardInfo/expireDate/ExpireDate.stories.tsx (1)
31-64: ⚡ Quick win중복된 스타일 컴포넌트를 발견했습니다.
Wrapper,Header,SubHeader,Label스타일 컴포넌트가CardNumber.stories.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 `@src/components/cardInfo/expireDate/ExpireDate.stories.tsx` around lines 31 - 64, The styles Wrapper, Header, SubHeader and Label are duplicated across stories; extract them into a shared styled-components module (e.g., export const Wrapper, Header, SubHeader, Label from a new file) and update the stories to import those named exports instead of redefining them; ensure the new module re-exports the same component names and adjust any imports in ExpireDate.stories (and CardNumber.stories) to use the shared exports so there is a single source-of-truth for these styled components.src/components/conditional/ConditionalExpireDate.tsx (1)
13-22: ⚡ Quick winJSX 구조 단순화를 고려해보세요.
ConditionalPassword와 동일한 패턴으로, 중첩된 Fragment를 단순화할 수 있습니다.이러한 조건부 렌더링 컴포넌트들이 여러 개 있는 것 같은데, 일관된 구조로 리팩토링하면 코드베이스의 일관성이 향상될 것입니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/conditional/ConditionalExpireDate.tsx` around lines 13 - 22, ConditionalExpireDate currently uses nested fragments for conditional rendering; simplify it to match the ConditionalPassword pattern by removing the extra Fragment and returning the conditional expression directly (use the validation prop to render children and the ExpireDate component only when truthy). Locate the ConditionalExpireDate component and update its return to directly render children and <ExpireDate /> under the single conditional using the validation prop and children identifiers.src/hooks/useCardNumberInput.ts (2)
34-38: ⚡ Quick win입력 유효성 검증 로직을 다시 살펴보세요.
현재
isNumeric(value)검증만 수행하고 있는데, 빈 문자열인 경우에도 고려가 필요할 수 있습니다.다음 시나리오를 생각해보세요:
- 사용자가 숫자를 입력한 후 모두 지운다면?
isNumeric("")의 결과는 무엇일까요?- 이 경우 에러 메시지가 적절할까요?
유효성 검증 함수(
src/utils/validators.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 `@src/hooks/useCardNumberInput.ts` around lines 34 - 38, The numeric validation currently only calls isNumeric(value) and will mis-handle empty strings depending on validators.ts behavior; update the check in useCardNumberInput (the block that mutates newError[index]) to explicitly handle empty values first (e.g., if value === "" then set newError[index] = "숫자를 입력해주세요." or the intended “required” message), else run isNumeric(value) and set/clear newError[index] accordingly; confirm this aligns with src/utils/validators.ts behavior and adjust the message/logic if empty input is meant to be allowed (in which case treat "" as valid and clear the error).
48-57: 💤 Low value에러 상태 관리의 완전성을 고려해보세요.
handleBlur에서 입력이 불완전할 때 에러를 설정하지만, 입력이 완전할 때 에러를 명시적으로 제거하지 않습니다.다음 상황을 생각해보세요:
- 사용자가 불완전한 입력으로 blur → 에러 표시
- 사용자가 돌아와서 완전히 입력 → blur
- 이전 에러가 남아있을까요?
handleCardNumberChange에서 에러를 지우고 있지만, blur 시점에서도 명시적으로 처리하는 것이 일관성 있는 에러 관리인지 고민해보세요.🤖 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/hooks/useCardNumberInput.ts` around lines 48 - 57, handleBlur currently sets an error when cardNumber[index].length !== maxLength but never clears an existing error when the field is complete; update handleBlur (in useCardNumberInput) to explicitly clear the error for that index when cardNumber[index].length === maxLength by copying cardNumberError into newError, setting newError[index] = "" and calling setCardNumberError(newError); keep the existing early-return behavior only for the error case and reference inputConfig, cardNumber, cardNumberError, and setCardNumberError when making the change.src/components/cardInfo/cardNumber/Input.tsx (1)
67-71: 💤 Low value스타일 조건부 적용 방식을 개선할 수 있습니다.
템플릿 리터럴로 조건부 CSS를 적용하고 있는데, Emotion의 더 간결한 방법을 고려해보셨나요?
다음 두 가지 접근을 비교해보세요:
- 현재: 템플릿 리터럴 내부에 조건부 CSS 문자열
- 대안: 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/components/cardInfo/cardNumber/Input.tsx` around lines 67 - 71, The conditional styling in Input.tsx uses a template literal to inject CSS when props.$hasError is true—replace that pattern with Emotion's object-style conditional return to improve readability and type-safety: inside the styled component (where ${(props) => props.$hasError && `border: 1px solid red;`} appears), return a CSS object or merge a conditional object (e.g., props.$hasError ? { border: '1px solid red' } : {}) so the styled Input component and the $hasError prop are handled via plain objects rather than string interpolation.src/components/PaymentWidget.stories.tsx (1)
34-37: ⚡ Quick win스토리에서 내부 구현 복제 대신
PaymentWidget을 직접 렌더링하세요.Line 34-37은 실제 컴포넌트 구조를 스토리에서 다시 조합하고 있어,
PaymentWidget변경 시 스토리와 동작이 어긋나기 쉽습니다.♻️ 제안 수정
export const Default: Story = { render: () => { return ( <MemoryRouter> <PaymentProvider> - <CardPaymentWidgetWrapper> - <CardPreview /> - <TotalInfo /> - </CardPaymentWidgetWrapper> + <PaymentWidget /> </PaymentProvider> </MemoryRouter> ); }, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/PaymentWidget.stories.tsx` around lines 34 - 37, The story is manually composing CardPaymentWidgetWrapper with CardPreview and TotalInfo instead of rendering the actual PaymentWidget, which will drift when PaymentWidget changes; update the story to import and render PaymentWidget directly (replace the CardPaymentWidgetWrapper/ CardPreview/ TotalInfo composition with a single <PaymentWidget /> render), ensure any props or context consumed by PaymentWidget are provided via the story's args or decorators, and remove the duplicated internal structure so the story always reflects the real component behavior.
🤖 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 `@README.md`:
- Line 8: The README contains a typo in the learning objective sentence: the
string "hooks API를 이용하여 상태 관리 로직을 구현하한다." should be corrected to "hooks API를
이용하여 상태 관리 로직을 구현한다."; update that exact sentence in README.md to replace
"구현하한다" with "구현한다".
In `@src/components/cardInfo/cardBrand/CardBrand.stories.tsx`:
- Around line 16-24: The story's meta.component is CardBrand but the Default
story's render returns Dropdown directly, so regressions to CardBrand won't be
caught; update the Default story (the Story named Default) to render the
CardBrand component (e.g., wrap Dropdown inside or replace Dropdown with
<CardBrand />) while preserving the CardBrandProvider/Wrapper/Header/SubHeader
structure so the story actually exercises the CardBrand component declared in
meta.component.
In `@src/components/cardInfo/cardBrand/Dropdown.tsx`:
- Around line 11-37: The dropdown options are not keyboard-accessible: add
proper ARIA roles and keyboard handlers to Button, Ul, and Li so assistive tech
and keyboard users can operate the list. On the toggle control (Button /
cardBrandContext.toggleDropdown) add aria-expanded reflecting
cardBrandContext.isOpen, and ensure it is focusable; make the options container
(Ul) role="listbox" and each option (Li) role="option" with tabIndex={-1} (or 0
for the first/active) and aria-selected mapping to
cardBrandContext.selectedItem; implement onKeyDown handlers on the toggle and
each option to handle Enter/Space to select (invoking
cardBrandContext.handleItemClick) and Arrow Up/Down to move focus between
options, and ensure options get unique ids so the active/selected option can be
referenced by aria-activedescendant if you prefer roving focus.
In `@src/components/cardInfo/cvc/Cvc.stories.tsx`:
- Around line 17-27: The Default story in Cvc.stories.tsx is reconstructing the
internal markup (Wrapper, Header, Label, Input) instead of rendering the actual
component; replace the custom JSX inside the render function with a minimal
wrapper that renders the Cvc component within CvcProvider (i.e., use CvcProvider
and render <Cvc />), remove direct references to Wrapper/Header/Label/Input from
the story, and ensure the story imports and uses the Cvc and CvcProvider symbols
so that future UI/structure changes in Cvc.tsx are reflected automatically.
In `@src/components/cardInfo/password/Input.tsx`:
- Line 13: The Container element is a styled.form but has no onSubmit handler,
so pressing Enter can trigger an unintended submit; either change Container to a
non-form wrapper (e.g., styled.div) if no submit behaviour is required, or add
an onSubmit handler to the component that calls event.preventDefault() to block
native submission. Locate the Container declaration and the usage where formRef
is passed in Input.tsx and implement one of these two fixes consistently (update
the styled component type or add the onSubmit prop on the Container and ensure
the handler is bound in the component).
In `@src/components/cardInfo/password/Password.tsx`:
- Around line 41-48: The Label styled component is not wired to its input which
hurts accessibility; update the Password component to accept or generate a
stable id prop (e.g., passwordId) and pass that id to the input element and to
Label via its htmlFor prop (use the Label component where currently rendered and
set htmlFor={passwordId}), ensuring the input element has id={passwordId}; if
the component is reused without an id prop, generate a unique id (e.g., with
useId or a simple fallback) so every Label–input pair is properly linked.
In `@src/components/Enrollment.stories.tsx`:
- Around line 16-34: The story currently renders a duplicated UI instead of the
real Enrollment component; replace the inline JSX in the Default story and
render the actual Enrollment component (e.g., import and use Enrollment in the
Default.render) and wrap stories with the runtime context providers suggested in
meta.decorators (add a decorator that wraps Story with MemoryRouter and
PaymentProvider) so Enrollment receives navigation and payment context; ensure
to remove the faux UI in Default and pass any required props to Enrollment if
needed.
In `@src/components/sendButton/SendButton.tsx`:
- Around line 4-14: SendButton currently renders a Button without an explicit
type, so its default HTML behavior is "submit" which can trigger unintended form
submissions; update the JSX in the SendButton component (the Button element
inside function SendButton) to include type="button" as a prop (i.e., <Button
type="button" ...>) so clicks won't submit a parent form inadvertently.
In `@src/utils/selectCardType.ts`:
- Around line 35-37: selectCardType currently assumes cardNumber has at least
two entries and directly calls substring on cardNumber[1], which can throw if
the array is shorter; add a defensive guard at the start of selectCardType (or
immediately before computing firstSixNumber/firstThirdNumber/firstFourthNumber)
to ensure cardNumber.length >= 2 (or supply safe default empty strings) and use
conditional access when building firstSixNumber (e.g., use cardNumber[0] and
(cardNumber[1] ?? '') ), so compute firstSixNumber, firstThirdNumber, and
firstFourthNumber only from validated/non-null strings to avoid runtime crashes.
In `@src/utils/validators.ts`:
- Around line 25-38: The current canShowCvc, canShowPassword, and
canShowSendButton functions only check input lengths and should also ensure
there are no validation errors; update canShowCvc to require both
expireDate.month/year lengths and that expireDateContext.expireDate has no error
flag (e.g., !expireDateContext.expireDate.error), update canShowPassword to
require cvcContext.cvc.length === 3 && !cvcContext.cvc.error, and update
canShowSendButton to require the password length check plus
!passwordContext.password.error so progression only occurs when length and "no
error" conditions are met.
In `@tsconfig.app.json`:
- Line 7: The tsconfig's "lib" currently lists ["ES5","DOM"], which omits modern
runtime type definitions like Promise/Map/Set and causes broad type errors;
update the "lib" setting to at least "ES2015" (recommended "ES2023" or "ESNext")
instead of "ES5" so async/await and modern collections are typed
correctly—locate the "lib" entry (the line reading "lib": ["ES5", "DOM"]) and
replace the ES5 entry with a newer ES version.
---
Outside diff comments:
In `@README.md`:
- Around line 24-68: Replace all checklist markers using the non-standard token
"[v]" with GitHub Task List syntax "[x]" for completed items and use "[ ]" for
any incomplete items; specifically update every instance of "[v]" in the given
README sections (card number, 유효기간 입력, CVC 입력, 실시간 프리뷰, 동적 입력 UI 구현, 카드사 선택, 폼
제출 및 상태 관리, 카드 등록 완료 및 네비게이션) so they render as GitHub checkboxes (e.g., change
"- [v] 숫자(0~9)만 입력 가능" to "- [x] 숫자(0~9)만 입력 가능"), keeping the rest of the text
unchanged.
---
Nitpick comments:
In `@package.json`:
- Line 22: 패키지 업데이트 관련: 현재 package.json에 "react-router-dom": "^7.15.0"을 사용 중인 것을
확인했으니, v7 마이그레이션 권장사항을 적용하려면 코드베이스에서 라우터 관련 임포트(예: useNavigate, BrowserRouter,
MemoryRouter, Route, Routes)를 점검하고 필요한 경우 import 경로를 "react-router" 또는 DOM 전용
코드인 경우 "react-router/dom"으로 변경하고, 단계적 마이그레이션이 필요하면 v7의 Future Flags를 검토하여 점진적으로
활성화하는 변경을 적용하세요.
In `@src/components/cardInfo/cardNumber/CardNumber.stories.tsx`:
- Around line 31-63: Wrapper, Header, SubHeader and Label are duplicated
styled-components across multiple stories (including CardNumber.stories and
ExpireDate.stories); extract these shared styled components into a single module
(e.g., export named components from a new shared story-styles file) and replace
the local definitions in CardNumber.stories.tsx and ExpireDate.stories.tsx with
imports of those named exports, keeping the same component names (Wrapper,
Header, SubHeader, Label) so existing usage requires minimal changes and remove
the duplicated definitions in each story file.
In `@src/components/cardInfo/cardNumber/constants.ts`:
- Around line 1-18: The three exported constants (INPUT_CARD_NUMBER_CONFIG,
DINERS_INPUT_CARD_NUMBER_CONFIG, AMEX_INPUT_CARD_NUMBER_CONFIG) share the same
shape and should be explicitly typed with the existing InputConfig type from
src/types/types.ts to get compile-time protection; update each declaration to
use : InputConfig and import the type, and consider narrowing the `name`
property to a literal union (or use `as const`) so `name` isn't just
`string`—this will enforce correct field names and catch future shape changes at
compile time (hint: change the exports to typed constants and/or apply `as
const` to the arrays to preserve literal types).
In `@src/components/cardInfo/cardNumber/Input.tsx`:
- Around line 67-71: The conditional styling in Input.tsx uses a template
literal to inject CSS when props.$hasError is true—replace that pattern with
Emotion's object-style conditional return to improve readability and
type-safety: inside the styled component (where ${(props) => props.$hasError &&
`border: 1px solid red;`} appears), return a CSS object or merge a conditional
object (e.g., props.$hasError ? { border: '1px solid red' } : {}) so the styled
Input component and the $hasError prop are handled via plain objects rather than
string interpolation.
In `@src/components/cardInfo/cvc/Cvc.tsx`:
- Around line 7-8: Header 컴포넌트와 Label 컴포넌트가 Cvc.tsx에서 동일한 텍스트 "CVC 번호를 입력해 주세요"를
사용해 중복되어 있으니 디자인/접근성 의도에 맞게 하나를 수정하거나 제거하세요; 구체적으로 Cvc.tsx의 Header는 페이지/섹션 제목으로
유지하고 Label은 더 짧은 식별자(예: "CVC 번호" 또는 "CVC")로 변경하거나 Label을 제거해 화면 읽기 도구에 중복 정보를 주지
않도록 수정하십시오.
In `@src/components/cardInfo/cvc/Input.tsx`:
- Line 18: The onBlur prop currently wraps handleBlur in an unnecessary arrow
function; replace onBlur={() => handleBlur()} with onBlur={handleBlur} in the
Input component if the handleBlur handler does not require the event argument,
keeping the arrow wrapper only where you need to access e.target.value (as noted
for the other case on line 19); locate the onBlur usage and the handleBlur
function reference to make this substitution to avoid the extra closure.
- Around line 8-9: The formRef + useFocusFirstInput setup is unnecessary for a
single input: remove the form element and the useFocusFirstInput call, attach a
ref directly to the input element (e.g., inputRef) inside the Input component
and implement a small useEffect to focus it on mount; if you must keep a form
for consistency, leave formRef/useFocusFirstInput but confirm the form actually
provides submit behavior—otherwise remove the form wrapper entirely and use the
direct inputRef+useEffect approach to simplify and clarify behavior.
In `@src/components/cardInfo/expireDate/ExpireDate.stories.tsx`:
- Around line 31-64: The styles Wrapper, Header, SubHeader and Label are
duplicated across stories; extract them into a shared styled-components module
(e.g., export const Wrapper, Header, SubHeader, Label from a new file) and
update the stories to import those named exports instead of redefining them;
ensure the new module re-exports the same component names and adjust any imports
in ExpireDate.stories (and CardNumber.stories) to use the shared exports so
there is a single source-of-truth for these styled components.
In `@src/components/cardInfo/expireDate/Input.tsx`:
- Line 26: Replace the arrow-function wrappers on the onBlur props with the
handler references directly: change occurrences of onBlur={() =>
handleMonthBlur()} and onBlur={() => handleYearBlur()} to
onBlur={handleMonthBlur} and onBlur={handleYearBlur} respectively (in the Input
component where these handlers are used) — only do this if handleMonthBlur and
handleYearBlur do not require the synthetic event argument or binding.
- Line 21: 현재 코드가 month/year 각각을 별도의 FieldWrapper(form)로 감싸고 있어 시맨틱에 맞지 않으므로
FieldWrapper 요소 구조를 재구성하세요: month와 year 입력을 하나의 상위 컨테이너(선택: 하나의 form 또는 단순한
div)로 묶고 불필요한 중복 form 태그를 제거하며 FieldWrapper 컴포넌트가 실제 폼 제출이 필요 없다면 form이 아닌 div로
렌더링하도록 변경하세요; 또한 useFocusFirstInput이 적용되는 위치(useFocusFirstInput, formRef, 첫 번째
입력)를 확인해 동일한 컨테이너 안에서 첫 입력에만 포커스 훅이 적용되도록 조정하세요.
In `@src/components/cardInfo/password/Password.stories.tsx`:
- Around line 17-25: 현재 스토리가 Password와 동일한 UI를 다시 작성하고 있어 동기화가 깨질 위험이 있습니다;
PasswordProvider 내부에서 직접 Password 컴포넌트를 렌더링하도록 변경하세요: PasswordProvider로 감싸진
Wrapper/Header/SubHeader/Label/Input의 중복된 마크업을 제거하고 대신 Password 컴포넌트를 호출하도록
바꿔(참조 대상: PasswordProvider, Password, Wrapper, Header, SubHeader, Label, Input),
필요한 경우 Password가 받는 props를 스토리에서 전달하여 기존 시나리오를 재현하도록 보장하세요.
In `@src/components/cardPreview/CardPreviewInfo.tsx`:
- Around line 12-13: 현재 CardPreviewInfo 호출부는 CardPreviewNumber에 gap="10px"/"0px"
같은 스타일 값을 직접 전달해 의도가 불분명합니다; CardPreviewNumber 컴포넌트 API를 의미 기반 prop(예:
variant="number" | "expiry" 또는 spaced={true|false})으로 변경하고, CardPreviewNumber를
사용하는 곳에서 gap 대신 해당 의미 있는 prop을 전달하도록 maskCardNumber(cardNumber) 호출 줄과
formatExpireDate(expireDate) 호출 줄을 각각 업데이트하세요; 이때 CardPreviewNumber 내부에서
variant/spaced를 받아 기존 gap 로직을 매핑하도록 유지해 하위 컴포넌트만 수정하면 호출부 의도가 명확해집니다.
In `@src/components/cardPreview/CardPreviewNumber.tsx`:
- Around line 4-13: CardPreviewNumber's CardNumberProps currently types gap as
string; tighten it by changing gap to a literal union or a semantic prop: either
set gap: "10px" | "0px" (narrow literal union) or replace gap with a semantic
prop like variant: "number" | "expire" or spaced: boolean and map that to the
appropriate $gap value inside CardPreviewNumber (update the CardNumberWrapper
usage and any callers of CardPreviewNumber accordingly) so callers don't pass
arbitrary CSS strings and design tokens remain easier to migrate.
In `@src/components/conditional/ConditionalExpireDate.tsx`:
- Around line 13-22: ConditionalExpireDate currently uses nested fragments for
conditional rendering; simplify it to match the ConditionalPassword pattern by
removing the extra Fragment and returning the conditional expression directly
(use the validation prop to render children and the ExpireDate component only
when truthy). Locate the ConditionalExpireDate component and update its return
to directly render children and <ExpireDate /> under the single conditional
using the validation prop and children identifiers.
In `@src/components/conditional/ConditionalPassword.tsx`:
- Around line 12-21: The JSX currently renders an unnecessary outer Fragment in
ConditionalPassword's return; simplify by returning the conditional fragment
directly—use the validation check to return either the inner Fragment containing
{children} and <Password /> or null/false, removing the redundant outer
<>...</>; update the return in the ConditionalPassword component so it directly
returns (validation && (<>{children}<Password /></>)) or the equivalent ternary
expression.
In `@src/components/PaymentWidget.stories.tsx`:
- Around line 34-37: The story is manually composing CardPaymentWidgetWrapper
with CardPreview and TotalInfo instead of rendering the actual PaymentWidget,
which will drift when PaymentWidget changes; update the story to import and
render PaymentWidget directly (replace the CardPaymentWidgetWrapper/
CardPreview/ TotalInfo composition with a single <PaymentWidget /> render),
ensure any props or context consumed by PaymentWidget are provided via the
story's args or decorators, and remove the duplicated internal structure so the
story always reflects the real component behavior.
In `@src/components/sendButton/SendButton.tsx`:
- Line 9: The Button in SendButton.tsx hardcodes the
"/react-payments/enrollment" path in its onClick handler (navigate call), which
duplicates the router basename and risks broken links on deployment; change the
navigate invocation to use the basename-free route (e.g.,
navigate("/enrollment")) or reference a central route constant/enum instead so
the component uses navigate("/enrollment") (or ROUTES.ENROLLMENT) and relies on
BrowserRouter basename to prepend "/react-payments".
In `@src/context/cvc/CvcContext.ts`:
- Around line 1-15: The file exports CvcContext and useCvcContext; please tidy
imports by merging the two React imports into one (e.g., import { createContext,
useContext } from "react") to follow common style, change the thrown error in
useCvcContext from throw Error(...) to throw new Error(...) to match JS
conventions, and consider replacing the Korean invariant message with an English
message (or confirm team convention) so developer-facing errors are consistent
and searchable — you can find these changes in the CvcContext and useCvcContext
symbols.
- Around line 5-15: Extract the repeated pattern of createContext<Type |
null>(null) + null-checking hook into a generic helper (e.g.,
createNullableContext<T>(label?: string) returning { Context, useContextHook })
and replace the five duplications (CardNumberContext, ExpireDateContext,
CvcContext, CardBrandContext, PasswordContext and their use*Context hooks) to
use this helper; ensure the helper standardizes the thrown Error message
(consistent wording/spacing) and import style, and update references like
useCvcContext to call the generated hook from the helper so behavior and typings
remain identical.
In `@src/context/password/PasswordProvider.tsx`:
- Line 5: The local variable cvc is misnamed and should be renamed to a clear
identifier that reflects the hook result from usePasswordInput (e.g.,
passwordState, passwordInput, or passwordContext) to match intent; change the
declaration const cvc = usePasswordInput() to const passwordState =
usePasswordInput() (or a chosen name) and update every usage in PasswordProvider
(and any exported/context value names like PasswordProvider or PasswordContext)
to the new identifier so references remain consistent and typings/props continue
to work.
In `@src/hooks/useCardNumberInput.ts`:
- Around line 34-38: The numeric validation currently only calls
isNumeric(value) and will mis-handle empty strings depending on validators.ts
behavior; update the check in useCardNumberInput (the block that mutates
newError[index]) to explicitly handle empty values first (e.g., if value === ""
then set newError[index] = "숫자를 입력해주세요." or the intended “required” message),
else run isNumeric(value) and set/clear newError[index] accordingly; confirm
this aligns with src/utils/validators.ts behavior and adjust the message/logic
if empty input is meant to be allowed (in which case treat "" as valid and clear
the error).
- Around line 48-57: handleBlur currently sets an error when
cardNumber[index].length !== maxLength but never clears an existing error when
the field is complete; update handleBlur (in useCardNumberInput) to explicitly
clear the error for that index when cardNumber[index].length === maxLength by
copying cardNumberError into newError, setting newError[index] = "" and calling
setCardNumberError(newError); keep the existing early-return behavior only for
the error case and reference inputConfig, cardNumber, cardNumberError, and
setCardNumberError when making the change.
In `@src/hooks/useExpireDateInput.ts`:
- Around line 21-57: handleMonthChange has overlapping independent ifs that read
uncleaned value and can overwrite errors; change validation to operate on the
cleaned onlyNumbers (use onlyNumbers and sliceValue for firstDigit and numeric
checks), convert the validation branches into a single if-else-if chain so only
one setExpireDateError runs, and treat the single-character "0" case explicitly
(e.g., show numeric-range error until two digits or allow entry but validate on
blur). Also keep the existing yearInputRef.focus() behavior but base it on
onlyNumbers.length === 2, and ensure you still call setExpireDate with
sliceValue at the end.
- Around line 81-95: handleMonthBlur and handleYearBlur currently overwrite any
existing error with a length-only message; update both to first check the
current error state (expireDateError.month and expireDateError.year) and only
set "완전히 입력해 주세요." when length < 2 AND there is no existing error that should be
preserved (e.g., a numeric-format error set by the change handler). Concretely,
in handleMonthBlur and handleYearBlur, read expireDateError.month/year and only
call setExpireDateError(...) to set the "완전히 입력해 주세요." message if
expireDate.month.length < 2 AND expireDateError.month (or .year) is falsy (or
not a higher-priority message), otherwise leave the existing error unchanged.
In `@src/hooks/useFocusFirstInput.ts`:
- Around line 6-14: The effect in useFocusFirstInput should run when the actual
form element changes (not the ref object) and should pick a truly focusable
input; update the dependency from [formRef] to [formRef.current] (or accept an
external deps array if you need re-run on card-brand changes) and change the
selector to something like
'input:not(:disabled):not([type="hidden"]):not([hidden])' to avoid
disabled/hidden inputs; finally guard the focus call by ensuring the node is an
HTMLElement and call .focus() safely (e.g., const firstInput =
formRef.current?.querySelector(...) as HTMLElement | null; if (firstInput &&
typeof firstInput.focus === "function") firstInput.focus()).
In `@src/types/types.ts`:
- Around line 18-63: Summary: Context handler names and cardType typing are
inconsistent and should be made explicit and type-safe. Change generic
handleBlur properties to field-specific names (e.g.,
CardNumberContextValue.handleBlur -> handleCardNumberBlur,
CvcContextValue.handleBlur -> handleCvcBlur, PasswordContextValue.handleBlur ->
handlePasswordBlur; keep ExpireDateContextValue.handleMonthBlur/handleYearBlur
as-is) and update all usages accordingly; replace cardType: string | null in
CardNumberContextValue with a discriminated union derived from a
single-source-of-truth BRANDS constant (or enum) so cardType has type like
CardBrand | null and align/selectCardType’s return type to that union; ensure
any consumers, tests, and InputConfig/selectCardType signatures are updated to
the new names and types.
In `@src/utils/validators.ts`:
- Around line 17-23: canShowExpireDate currently can return a non-boolean
(string | false) because it returns values like selectedItem.brand directly;
change the function to explicitly return a boolean (e.g., annotate return type
as boolean) and coerce each truthy check to boolean (use !! or Boolean(...)) so
the expression inside canShowExpireDate always yields a boolean; update the
export const canShowExpireDate(signature) and the return expression referencing
cardBrandContext.isOpen, cardBrandContext.selectedItem.brand, and
cardBrandContext.selectedItem.color accordingly.
🪄 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: f9bda64a-130b-42b9-9eb9-e23ef47a4408
⛔ Files ignored due to path filters (4)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/AmericanExpress.svgis excluded by!**/*.svgsrc/assets/ChinaUnionPay.svgis excluded by!**/*.svgsrc/assets/DinersClub.svgis excluded by!**/*.svg
📒 Files selected for processing (63)
README.mdpackage.jsonsrc/App.stories.tsxsrc/App.tsxsrc/components/Enrollment.stories.tsxsrc/components/Enrollment.tsxsrc/components/PaymentWidget.stories.tsxsrc/components/PaymentWidget.tsxsrc/components/cardInfo/CardInfo.stories.tsxsrc/components/cardInfo/CardInfo.styles.tssrc/components/cardInfo/CardInfo.tsxsrc/components/cardInfo/TotalInfo.tsxsrc/components/cardInfo/cardBrand/CardBrand.stories.tsxsrc/components/cardInfo/cardBrand/CardBrand.tsxsrc/components/cardInfo/cardBrand/Dropdown.tsxsrc/components/cardInfo/cardNumber/CardNumber.stories.tsxsrc/components/cardInfo/cardNumber/CardNumber.tsxsrc/components/cardInfo/cardNumber/Input.tsxsrc/components/cardInfo/cardNumber/constants.tssrc/components/cardInfo/constants.tssrc/components/cardInfo/cvc/Cvc.stories.tsxsrc/components/cardInfo/cvc/Cvc.tsxsrc/components/cardInfo/cvc/Input.tsxsrc/components/cardInfo/expireDate/ExpireDate.stories.tsxsrc/components/cardInfo/expireDate/ExpireDate.tsxsrc/components/cardInfo/expireDate/Input.tsxsrc/components/cardInfo/password/Input.tsxsrc/components/cardInfo/password/Password.stories.tsxsrc/components/cardInfo/password/Password.tsxsrc/components/cardPreview/CardPreview.stories.tsxsrc/components/cardPreview/CardPreview.tsxsrc/components/cardPreview/CardPreviewInfo.tsxsrc/components/cardPreview/CardPreviewNumber.tsxsrc/components/common/CardInfoHeader.tsxsrc/components/common/CardInfoInput.tsxsrc/components/conditional/ConditionalCardBrand.tsxsrc/components/conditional/ConditionalCvc.tsxsrc/components/conditional/ConditionalExpireDate.tsxsrc/components/conditional/ConditionalPassword.tsxsrc/components/conditional/ConditionalSendButton.tsxsrc/components/sendButton/SendButton.tsxsrc/context/PaymentProvider.tsxsrc/context/cardBrand/CardBrandContext.tssrc/context/cardBrand/CardBrandProvider.tsxsrc/context/cardNumber/CardNumberContext.tssrc/context/cardNumber/CardNumberProvider.tsxsrc/context/cvc/CvcContext.tssrc/context/cvc/CvcProvider.tsxsrc/context/expireDate/ExpireDateContext.tssrc/context/expireDate/ExpireDateProvider.tsxsrc/context/password/PasswordContext.tssrc/context/password/PasswordProvider.tsxsrc/hooks/useCardBrand.tssrc/hooks/useCardNumberInput.tssrc/hooks/useCvcNumberInput.tssrc/hooks/useExpireDateInput.tssrc/hooks/useFocusFirstInput.tssrc/hooks/usePasswordInput.tssrc/types/types.tssrc/utils/selectCardType.tssrc/utils/validators.tstsconfig.app.jsontsconfig.json
💤 Files with no reviewable changes (6)
- src/components/cardInfo/CardInfo.stories.tsx
- src/components/common/CardInfoHeader.tsx
- src/components/cardInfo/CardInfo.styles.ts
- src/components/common/CardInfoInput.tsx
- src/components/cardInfo/CardInfo.tsx
- src/components/cardInfo/constants.ts
Comment on lines
+16
to
+24
| export const Default: Story = { | ||
| render: () => { | ||
| return ( | ||
| <CardBrandProvider> | ||
| <Wrapper> | ||
| <Header>카드사를 선택해 주세요</Header> | ||
| <SubHeader>현재 국내 카드사만 가능합니다.</SubHeader> | ||
| <Dropdown /> | ||
| </Wrapper> |
There was a problem hiding this comment.
스토리 대상 컴포넌트와 실제 렌더 대상이 다릅니다.
Line 16-24에서 meta.component는 CardBrand인데, 스토리는 Dropdown만 직접 렌더링하고 있습니다. 이 상태에서는 CardBrand 변경에 대한 회귀를 스토리에서 잡기 어렵습니다.
🔧 제안 수정
-import Dropdown from "./Dropdown";
-import styled from "@emotion/styled";
@@
export const Default: Story = {
render: () => {
return (
<CardBrandProvider>
- <Wrapper>
- <Header>카드사를 선택해 주세요</Header>
- <SubHeader>현재 국내 카드사만 가능합니다.</SubHeader>
- <Dropdown />
- </Wrapper>
+ <CardBrand />
</CardBrandProvider>
);
},
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/cardInfo/cardBrand/CardBrand.stories.tsx` around lines 16 -
24, The story's meta.component is CardBrand but the Default story's render
returns Dropdown directly, so regressions to CardBrand won't be caught; update
the Default story (the Story named Default) to render the CardBrand component
(e.g., wrap Dropdown inside or replace Dropdown with <CardBrand />) while
preserving the CardBrandProvider/Wrapper/Header/SubHeader structure so the story
actually exercises the CardBrand component declared in meta.component.
Author
There was a problem hiding this comment.
CardBrand 안에 있는 Dropdown 변경이 가능합니다.
Comment on lines
+16
to
+34
| export const Default: Story = { | ||
| render: () => { | ||
| const cardNumber = [1234, 1234, 1234, 1234]; | ||
| const brand = "국민카드"; | ||
| const handleClick = () => {}; | ||
|
|
||
| return ( | ||
| <MemoryRouter> | ||
| <Wrapper> | ||
| <CheckIcon /> | ||
| <Phrase> | ||
| <span>{cardNumber[0]}로 시작하는</span> | ||
| <span>{brand}가 등록되었어요.</span> | ||
| </Phrase> | ||
| <Button onClick={() => handleClick()}>확인</Button> | ||
| </Wrapper> | ||
| </MemoryRouter> | ||
| ); | ||
| }, |
There was a problem hiding this comment.
스토리가 실제 Enrollment 컴포넌트를 렌더링하지 않습니다.
현재 스토리는 UI를 복제해서 그려서, 실제 Enrollment의 컨텍스트/네비게이션 동작 회귀를 잡지 못합니다. 스토리에서 Enrollment 자체를 렌더링하도록 바꾸는 게 좋습니다.
💡 제안 수정안
export const Default: Story = {
render: () => {
- const cardNumber = [1234, 1234, 1234, 1234];
- const brand = "국민카드";
- const handleClick = () => {};
-
- return (
- <MemoryRouter>
- <Wrapper>
- <CheckIcon />
- <Phrase>
- <span>{cardNumber[0]}로 시작하는</span>
- <span>{brand}가 등록되었어요.</span>
- </Phrase>
- <Button onClick={() => handleClick()}>확인</Button>
- </Wrapper>
- </MemoryRouter>
- );
+ return <Enrollment />;
},
};// 예시: meta.decorators에 실제 런타임 컨텍스트 주입
// (PaymentProvider import 추가 필요)
decorators: [
(Story) => (
<MemoryRouter>
<PaymentProvider>
<Story />
</PaymentProvider>
</MemoryRouter>
),
],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Enrollment.stories.tsx` around lines 16 - 34, The story
currently renders a duplicated UI instead of the real Enrollment component;
replace the inline JSX in the Default story and render the actual Enrollment
component (e.g., import and use Enrollment in the Default.render) and wrap
stories with the runtime context providers suggested in meta.decorators (add a
decorator that wraps Story with MemoryRouter and PaymentProvider) so Enrollment
receives navigation and payment context; ensure to remove the faux UI in Default
and pass any required props to Enrollment if needed.
Author
There was a problem hiding this comment.
스토리북 수정은 스텝3에서 전체적으로 수정하겠습니다.
지금은 스토리북보다는 코드 설계에 집중하고 싶습니다.
Comment on lines
+35
to
+37
| const firstSixNumber = cardNumber[0] + cardNumber[1].substring(0, 2); | ||
| const firstThirdNumber = cardNumber[0].substring(0, 3); | ||
| const firstFourthNumber = cardNumber[0].substring(0, 4); |
There was a problem hiding this comment.
배열 길이 가정으로 런타임 에러가 날 수 있습니다.
Line [35]에서 cardNumber[1].substring(...)을 바로 호출하고 있어서, 입력 초기 상태나 예외 케이스에서 cardNumber[1]이 없으면 즉시 크래시합니다.
selectCardType에 들어오는 cardNumber의 최소 길이가 항상 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/utils/selectCardType.ts` around lines 35 - 37, selectCardType currently
assumes cardNumber has at least two entries and directly calls substring on
cardNumber[1], which can throw if the array is shorter; add a defensive guard at
the start of selectCardType (or immediately before computing
firstSixNumber/firstThirdNumber/firstFourthNumber) to ensure cardNumber.length
>= 2 (or supply safe default empty strings) and use conditional access when
building firstSixNumber (e.g., use cardNumber[0] and (cardNumber[1] ?? '') ), so
compute firstSixNumber, firstThirdNumber, and firstFourthNumber only from
validated/non-null strings to avoid runtime crashes.
Author
There was a problem hiding this comment.
cardNumber[1]이 없다면 해당 조건을 만족하지 못해서 카드 브랜드 사진이 보이지 않게 합니다. 입력 값에 따라서 보이는 브랜드 사진이 다르므로 cardNumber[0] + cardNumber[1]이 "" 이 되더라도 괜찮습니다.
따라서 방어 로직을 작성할 필요까지는 없다고 생각합니다.
Comment on lines
+25
to
+38
| export const canShowCvc = (expireDateContext: ExpireDateContextValue) => { | ||
| return ( | ||
| expireDateContext.expireDate.month.length === 2 && | ||
| expireDateContext.expireDate.year.length === 2 | ||
| ); | ||
| }; | ||
|
|
||
| export const canShowPassword = (cvcContext: CvcContextValue) => { | ||
| return cvcContext.cvc.length === 3; | ||
| }; | ||
|
|
||
| export const canShowSendButton = (passwordContext: PasswordContextValue) => { | ||
| return passwordContext.password.length === 2; | ||
| }; |
There was a problem hiding this comment.
길이만으로 다음 단계 노출을 결정하면 검증 우회가 발생할 수 있습니다.
Line [25]~Line [38]은 길이 조건만 통과하면 다음 필드/버튼이 열립니다. 이러면 형식 오류(예: 잘못된 월/년, 비정상 CVC)가 있어도 진행될 수 있습니다.
canShow* 계열에서 “길이 + 에러 없음”을 함께 보도록 조건을 재점검해 보실래요?
🤖 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/utils/validators.ts` around lines 25 - 38, The current canShowCvc,
canShowPassword, and canShowSendButton functions only check input lengths and
should also ensure there are no validation errors; update canShowCvc to require
both expireDate.month/year lengths and that expireDateContext.expireDate has no
error flag (e.g., !expireDateContext.expireDate.error), update canShowPassword
to require cvcContext.cvc.length === 3 && !cvcContext.cvc.error, and update
canShowSendButton to require the password length check plus
!passwordContext.password.error so progression only occurs when length and "no
error" conditions are met.
Author
There was a problem hiding this comment.
현재 validator로 input 에러를 검사하고 있다고 생각했었는데, 모든 입력 후에도 문자를 입력하면 에러 문구가 떠 지는 것을 확인했습니다.
다음 단계 노출의 안정성을 위해 모든 input 입력과 에러 메시지가 없을 때, 다음 단계가 보이도록 수정하였습니다.
| "target": "ESNext", | ||
| "strict": true, | ||
| "forceConsistentCasingInFileNames": true, | ||
| "lib": ["ES5", "DOM"], |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TypeScript tsconfig에서 "lib": ["ES5","DOM"] 설정 시 Promise 타입이 포함되는지, 그리고 React 앱에서 권장 lib 설정이 무엇인지 공식 문서 기준으로 확인해줘.
💡 Result:
"lib": ["ES5", "DOM"] 설정 시 Promise 타입은 포함되지 않습니다. 공식 TypeScript 문서에 따르면 ES5는 ES5 기능의 코어 정의만 포함하며, Promise는 ES2015에 포함됩니다[1]. 여러 소스에서 ES5 + DOM만으로는 Promise가 없어 async/await 등에서 오류가 발생하며, "es2015" 또는 "es2015.promise"를 추가해야 한다고 확인됩니다[2][3][4].
Citations:
- 1: https://www.typescriptlang.org/tsconfig/lib.html
- 2: https://stackoverflow.com/questions/41461797/where-does-the-lib-come-from-in-typecsript
- 3: https://runebook.dev/en/docs/typescript/tsconfig/index/lib-config
- 4: https://stackoverflow.com/questions/43555378/typescript-compiler-says-an-async-function-or-method-in-es5-es3-requires-the-p
lib 설정을 ES5로 두면 필수 타입이 누락됩니다.
["ES5", "DOM"]는 명시적 override로 작동하여 Promise와 Map, Set 등 최신 타입이 빠집니다. async/await를 사용하는 React 앱에서 광범위한 타입 오류를 일으킵니다. 최소 ES2015 이상으로 설정해야 합니다(권장: ES2023 또는 ESNext).
제안 수정안
- "lib": ["ES5", "DOM"],
+ "lib": ["ES2023", "DOM"],📝 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
| "lib": ["ES5", "DOM"], | |
| "lib": ["ES2023", "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 `@tsconfig.app.json` at line 7, The tsconfig's "lib" currently lists
["ES5","DOM"], which omits modern runtime type definitions like Promise/Map/Set
and causes broad type errors; update the "lib" setting to at least "ES2015"
(recommended "ES2023" or "ESNext") instead of "ES5" so async/await and modern
collections are typed correctly—locate the "lib" entry (the line reading "lib":
["ES5", "DOM"]) and replace the ES5 entry with a newer ES version.
Author
There was a problem hiding this comment.
["DOM", "DOM.Iterable", "ESNext"]로 수정하여 DOM API와 최신 모듈을 받을 수 있도록 수정하였습니다.
degurii
requested changes
May 15, 2026
degurii
left a comment
degurii
left a comment
There was a problem hiding this comment.
안녕하세요 지오. 우선 PR 리뷰가 늦어진 점 정말 죄송합니다.
화상 미팅으로 주요 질문들은 답변을 드렸고, 그외 코드레벨에서 보이는 인라인 코멘트들 몇개 남겨드렸습니다. 확인 후에 다시 리뷰 요청 부탁드리겠습니다.
고생 많으셨습니다!
| - 하나의 input에 4자리가 모두 입력되면 카드 프리뷰를 업데이트한다 | ||
| - 유효하지 않은 번호 입력 시 에러 피드백 제공 | ||
| | Diners | `36`으로 시작하는 14자리 숫자 | | ||
| | AMEX | `36`, `37`로 시작하는 15자리 숫자 | |
There was a problem hiding this comment.
[필수]
AMEX 식별 번호는 34, 37이 요구사항인데, 정리해주신 명세는 다른 것 같아요. 기능은 잘 구현해주셔서 요부분만 수정해주시면 좋을듯합니다.
| import styled from "@emotion/styled"; | ||
| import { type CardPreviewProps } from "../../types/types"; | ||
| import { maskCardNumber, formatExpireDate } from "../../utils/cardFormatters"; | ||
| import CardPreviewNumber from "../common/CardPreviewNumber"; |
There was a problem hiding this comment.
[필수]
CardPreviewInfo.tsx가 어디서도 import되지 않아요. 아마 요 역할이 CardPreview 컴포넌트로 넘어가면서, 제거가 누락된 컴포넌트가 아닌가 싶어요. 한번 확인해보시고 데드 코드라면 제거해주면 좋을 것 같습니다.
Author
There was a problem hiding this comment.
CardPreviewInfo.tsx를 CardPreview.tsx에 합쳤는데, 이전 코드를 삭제하지 않았습니다.
삭제하였습니다!
|
|
||
| return ( | ||
| <Wrapper> | ||
| <Button |
There was a problem hiding this comment.
[권장]
styled로 넘기는 prop에 $hasError와 같이 $ prefix를 넣어주는 패턴이 보이는데, 여기서는 prefix 없이 styled에 전달하고 있어요. 요 컨벤션은 통일되는 편이 좋아보입니다.
Author
There was a problem hiding this comment.
emotion style에서 $를 주면 dom 태그에 전달해주는 대신 스타일에만 사용하는 속성이라는 것을 확인했습니다.
수정하였습니다!
| setExpireDateError(initialState.expireDateError); | ||
| }; | ||
|
|
||
| const handleMonthChange = (value: string) => { |
There was a problem hiding this comment.
[제안]
handleMonthChange의 분기들이 어느 쪽이든 마지막에 값을 반영하고 에러 메시지만 다르게 세팅하는 형태예요. CVC/Password도 같은 패턴입니다.
1단계 마무리 리뷰에서 month 에러 시 잘못된 값이 state에 저장될 것 같다고 짚었었는데, 여기서도 동일하게 값의 유효성과 관계 없이 항상 state에 저장하는 것 같아요. 그래서 13을 입력하면 카드 프리뷰의 월에 13이 그대로 표시되는 동작이 남아 있습니다.
이렇게 잘못된 값이더라도 state에 저장하는 것이 허용 가능하고, 이를 프리뷰에 표시하는 것이 의도된 정책일지, 아니면 작업자의 실수일지 코드를 읽는 입장에서 좀 헷갈리는 감이 있습니다. 어느 방향이든, 좀 더 코드에 그 의도가 잘 드러날 수 있도록 구조를 개선해보아도 좋을 것 같아요.
Author
There was a problem hiding this comment.
에러가 발생해도 입력 상태를 업데이트하도록 설계한 이유는 에러가 어디서 발생했는지 정확히 알도록 하기 위함입니다. 사용자가 자신이 어떤 값을 잘못 입력했는지 입력창에서 직접 보고 인지한 뒤 수정할 수 있게 하려는 의도였습니다.
또한, 에러 상태가 활성화되면 어차피 다음 스텝으로 넘어가지 못하도록 방어하고 있어서 잘못된 값이 상태에 저장되더라도 전체 흐름에 문제가 없다고 생각했습니다.
다른 에러와 다르게 숫자 이외의 문자 입력을 완전히 막은 이유는 구현 조건에서 숫자 이외의 문자는 입력되지 않도록 제시하여서 입력을 막았습니다.
위와 같은 이유로 에러 발생 입력이라도 입력 상태 값을 변경하여 입력이 되도록 하는 것이 좋다고 생각합니다. 하지만, 입력 값이 잘못 되었는데 프리뷰가 업데이트 되면 옳은 값이라고 판단할 수 있기 때문에, 이 상황에서 프리뷰에 해당 값으로 업데이트하는 것은 좋지 않은 판단이라고 생각합니다.
그래서, 에러 상황에서도 입력 상태 값 변화는 막지 않으면서 프리뷰 업데이트를 막는 방법은 어떤지 궁금합니다.
JuHyeong424
commented
May 15, 2026
Author
JuHyeong424
left a comment
JuHyeong424
left a comment
There was a problem hiding this comment.
안녕하세요!
어제 화상 회의에서 좋은 말씀 주셔서 감사합니다.
리뷰에 대한 답글을 달았는데 확인해 주시면 감사하겠습니다.
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.
🎯 페이먼츠
이번 미션을 통해 다음과 같은 학습 경험들을 쌓는 것을 목표로 합니다.
1단계
2단계
3단계
idle | loading | success | error네 가지로 명시적으로 관리한다.🕵️ 셀프 리뷰(Self-Review)
제출 전 체크 리스트
리뷰 요청 & 논의하고 싶은 내용
안녕하세요. 지오입니다.
이번 미션을 하면서 설계를 어떻게 하면 좋을지, 리팩토링을 어떻게 하면 좋을지 등에 대한 고민이 많이 생겼습니다.
이번 미션도 잘 부탁드립니다.
1) 이번 단계에서 가장 많이 고민했던 문제와 해결 과정에서 배운 점
불필요한 리렌더링 방어와 Context API를 활용한 상태 관리 최적화
이번 단계를 시작하기 전에 가장 깊게 고민하고 시간을 쏟았던 부분은 불필요한 리렌더링 방어와 Context API를 활용한 상태 관리 최적화입니다. 해당 부분을 개선하기 위해 어떻게 해결했는지에 대해 작성한 글입니다. (https://j-brothers.tistory.com/255)
처음 결제 폼을 구현했을 때는 카드 번호를 한 글자 입력할 때마다 값과 상관없는 다른 입력창(유효 기간, cvc)이 다시 그려지는 문제가 있었습니다. React Developer Tools의 Profiler와 하이라이트 기능을 통해 검사해 본 결과, 렌더링 범위가 너무 넓어 앱의 성능이 저하되고 있다는 것을 눈으로 확인할 수 있었습니다. 또한,
CardInfo컴포넌트에 모든 input 관련 로직을 다 처리하고 있었고,PaymentWidget컴포넌트에 3개의 input 상태 관리를 맡고 있어서 책임이 너무 무겁다고 생각했습니다. 이것을 통해 현재 상태 관리 설계에 문제가 있음을 인식했습니다.이러한 렌더링 Waterfall을 개선하기 위해 상태 관리 구조를 개편했습니다. 가장 먼저
PaymentWidget이 가지는CardNumber,ExpireDate,CvcNumber상태를 나누어줄 필요를 느꼈습니다. 우선,CardPreview컴포넌트에 input 상태 값이 업데이트되냐, 안되냐에 따라 나누었습니다.CardPreview와 관련된CardNumber와ExpireDate를CardTopSection컴포넌트에 함께 분리하였습니다. 그와 관련 없는Cvc컴포넌트는 따로 분리하였습니다. 그 다음 선택한 방법은React.memo를 사용하는 것이었습니다. 해당 컴포넌트의props가 업데이트될 때만 리렌더링이 발생하게 변경하였습니다.CardTopSection컴포넌트에서CardNumber와ExpireDate가 각각 해당하는 props가 업데이트될 때만 렌더링되도록 수정하였습니다. 그 결과,CardNumber의input값이 변경될 때,cvc컴포넌트와ExpireDate컴포넌트가 렌더링되는 것을 막을 수 있었습니다.ExpireDate또한 'cvc'와 'CardNumber' 컴포넌트와 독립적인 상태를 가질 수 있었습니다.이러한 개선을 통해 처음 평균 렌더링 시간이 24.47ms에서 7.58ms로 감소했습니다. 각각의 책임을 분산시키고, 불필요한 렌더링을 막으면서 눈에 띄는 성능 개선을 할 수 있었습니다.
하지만, 현재 구조에도 문제가 있었습니다. 카드 정보를 다 입력한 후, db에 입력 정보를 보내려면 어쨌든 최상위 부모(
PaymentWidget)에서 전체 입력 값 상태를 관리해야한다는 사실이었습니다. 그러면CardNumber,ExpireDate컴포넌트에서 사용한React.memo를CvcNumber컴포넌트에서도 사용하여 해당 props가 변경될 때에만 렌더링되도록 설정해야했습니다. 또한, 입력 컴포넌트가 증가할수록React.memo를 통해 관리해야한다는 불편함이 있었습니다. 모든 상태를 최상위 부모에서 자식으로 props로 보내기 때문에 상태 전달에 있어 불안정함이 더 커질 것으로 생각되었습니다. 이런 이유를 해결하기 위해 전역 상태 관리 api인 Context API를 선택하였습니다. 도메인별(카드 번호, 유효기간, CVC, 카드 브랜드, 비밀번호 등)로 Context Provider를 분리하여 설계하였습니다. 특정 입력값이 변경될 때 해당 데이터를 사용하는 컴포넌트만 독립적으로 리렌더링되게 하였습니다. 이제 정말 해당 input 상태 값이 필요한 컴포넌트만 렌더링되도록 만들었습니다.React.memo를 사용했을 때의 평균 렌더링 소요 시간은 7.58ms이었습니다. Context API를 도입하고 평균 렌더링 소요 시간이 5.77ms로 감소하였습니다. 중간 부모의 렌더링을 생략하고 정말 필요한 컴포넌트에서만 이루어지는 렌더링을 통해 성능이 개선되지 않았을까하는 생각입니다.
이 해결 과정을 통해 단순히 동작하는 폼을 만드는 것을 넘어서, 리액트 렌더링 동작 원리를 이해할 수 있었고, 성능을 고려한 컴포넌트 설계와 상태 관리가 프론트엔드 개발에서 왜 중요한지를 배울 수 있었습니다.
최종 구조도입니다.
App.tsx에서 각 입력 상태의Context API Provider가 감싸져있습니다. 각각의Provider는 각 입력 커스텀 훅(파란색 박스)의 반환값을 받고 있습니다.Provider에 의해 제공되는value는Context(초록색 박스)를 통해 해당value가 필요한 컴포넌트에 전달되고 있습니다.2) 이번 리뷰를 통해 논의하고 싶은 부분
1. Context API 외의 상태 관리 설계 능력 향상
Context API를 활용하여 렌더링 최적화를 진행했지만, 한편으로는 처음부터 상태의 위치(Colocation)나 컴포넌트 합성(Composition)을 더 잘 설계했다면 Context 없이도 충분히 좋은 성능을 내지 않았을까? 하는 아쉬움이 남습니다. 멘토님만의 상태 관리 설계 노하우나, 프론트엔드 설계 능력을 기르기 위해 평소에 어떤 훈련을 하시는지 조언을 구하고 싶습니다.
2. React Developer Tools를 활용한 성능 리팩토링 방향성
이번 과제에서는 React Developer Tools의 Profiler를 적극 활용해 렌더링 병목을 시각적으로 확인하고 리팩토링하는 경험을 해보았습니다. 도구의 수치에 기반한 이러한 성능 최적화 접근 방식이 실무에서도 자주 쓰이는 올바른 방향인지, 혹은 멘토님이 생각하시는 더 중요하거나 좋은 리팩토링 접근법이 있는지 궁금합니다.
3. 커스텀 훅의 분리 기준 (관심사 분리)
현재 커스텀 훅(
useCardNumberInput등) 내부에 입력 상태, 에러 상태,onChange/onBlur핸들러, 그리고 내부validation로직까지 너무 많은 책임이 섞여 있어 단일 책임 원칙에 위배되는 것 같아 고민입니다. 이런 경우 훅을 더 잘게 쪼갠다면 어떤 기준(예: 상태 관리용 훅 vs 검증 로직용 훅 등)으로 나누는 것이 유지보수성에 좋을지 의견을 여쭙고 싶습니다.4. 파생된 상태를 활용한 에러 관리
현재 입력 에러를 별도의 상태(
useState)로 관리하고 있는데, 상태 동기화 문제를 피하기 위해 입력 상태 값에 따라 계산되는 파생 상태 형태로 에러 로직을 작성하는 것이 더 권장된다는 것을 알게 되었습니다. 현재의 훅 구조에서 에러를 파생 상태로 변경하는 것에 대해 멘토님은 어떻게 생각하시는지 궁금합니다.5. 브라우저 네이티브 속성 vs JS 상태를 통한 입력 제어
Validation시 숫자가 아닌 문자는return을 통해 빠른 리턴을 하였고 (#520 (comment)),<input>의maxLength로 입력 길이 제한을 하는 등 HTML 속성에 의존했습니다. 그런데 브라우저 기본 기능에 의존하기보다 JS(React)의 상태 제어를 통해 입력 제한을 관리하는 것이 더 유연하고 좋다는 피드백을 접했습니다. 실무에서는 브라우저 제어와 JS 제어 중 어느 쪽에 더 비중을 두는지, 그리고 JS 제어를 더 선호한다면 그 구체적인 이유가 무엇인지 궁금합니다.6. 여러 전역 상태의 효율적인 초기화 방법과 key 활용에 대한 의문
Enrollment컴포넌트에서 완료 버튼 클릭 시, 5개로 나뉜Provider의 입력값을 모두 초기화하려니 호출해야 할 함수가 너무 많아졌습니다. 이를 효율적으로 처리할 방법을 찾던 중, 아래 코드처럼 최상위Provider에key prop을 부여하고 이 값을 변경하여 하위 트리를 강제로 언마운트/리마운트(초기화) 시키는 방법을 접하게 되었습니다.다만 제가 알기로
key는 본래 컴포넌트를 식별하기 위한 용도인데, 이렇게 전체 초기화를 유도하는 방법으로 사용해도 괜찮은 패턴인지 의문이 듭니다. 멘토님이 보시기에 이 방식이 괜찮은지, 아니면 각 훅의Action들을 모아 하나의resetAll훅으로 처리하는 등의 설계가 더 괜찮은지 궁금합니다.7. 가독성을 높이는 변수/함수 네이밍 컨벤션과 일관성 유지
이번 결제 폼을 구현하며 다양한 입력 상태와 이벤트 핸들러, 검증(Validation) 유틸 함수들을 다루다 보니 네이밍에 대한 고민이 깊어졌습니다. 예를 들어
handleCardNumberChange처럼 도메인과 액션이 합쳐져 이름이 너무 길어지기도 하고,Boolean값을 반환하는 검증 함수(isNumeric등)나 에러 상태 변수의 이름을 지을 때 팀이나 프로젝트 차원에서 어떤 컨벤션을 따르는 것이 가장 직관적일지 판단하기 어려웠습니다.멘토님께서 가장 중요하게 생각하시는 네이밍 원칙이 무엇인지, 혹은 피하고 싶은 패턴이 있으시다면 조언을 부탁드리고 싶습니다.
✅ 리뷰어 체크 포인트
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*를 사용했는가?