Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughforwardRef, React.memo, useCallback, useMemo 의존을 대규모로 제거했습니다. 다수 컴포넌트는 명시적 Changes
Sequence Diagram(s)(생략) 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)
📝 Coding Plan
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 React Compiler와 React 19의 도입에 맞춰 기존 코드베이스의 수동 최적화 패턴을 제거하는 것을 목표로 합니다. 컴파일러가 자동으로 처리하는 메모이제이션 및 참조 전달 로직을 제거하여 코드의 복잡성을 줄이고, 프레임워크의 최신 기능을 최대한 활용하도록 리팩토링했습니다. 이를 통해 개발 효율성을 높이고 애플리케이션의 잠재적인 성능 이점을 확보할 수 있습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
| export function AlertIcon({ | ||
| ref, | ||
| size = "xlarge", | ||
| className, | ||
| ...props | ||
| }: AlertIconProps & { ref?: React.Ref<HTMLSpanElement> }) { |
There was a problem hiding this comment.
타입 정의를 간소화할 수 있습니다.
IconProps에 ref가 이미 포함되어 있고, AlertIconProps는 IconProps에서 svg만 제외한 타입이므로 ref를 이미 가지고 있습니다. 따라서 & { ref?: React.Ref<HTMLSpanElement> } 부분은 중복됩니다.
아래와 같이 AlertIconProps만 사용하도록 수정하면 코드가 더 간결해집니다. 이 패턴은 다른 모든 아이콘 컴포넌트에도 동일하게 적용될 수 있습니다.
| export function AlertIcon({ | |
| ref, | |
| size = "xlarge", | |
| className, | |
| ...props | |
| }: AlertIconProps & { ref?: React.Ref<HTMLSpanElement> }) { | |
| export function AlertIcon({ | |
| ref, | |
| size = "xlarge", | |
| className, | |
| ...props | |
| }: AlertIconProps) { |
References
- 단순성: 복잡한 해결책보다 단순한 해결책을 선호합니다. (GAK 프로젝트 코드 스타일 가이드, 라인 12) (link)
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/features/session/components/SessionCreateForm.tsx (1)
69-78:⚠️ Potential issue | 🟠 Major렌더 단계에서
URL.createObjectURL호출 제거 필요.문제:
selectedImage가 존재할 때 매 렌더마다 새 blob URL이 생성되고,useEffect의존성에imagePreviewUrl이 포함되어 매번 cleanup 재실행됨.영향:
- 렌더마다 새 blob URL 생성으로 메모리 누수 발생
- 불필요한 effect 재실행으로 성능 저하
권장 수정:
OnboardingProfile.tsx패턴 적용
useRef로 현재 URL 추적URL.createObjectURL을 렌더 단계 대신 이미지 선택 핸들러에서만 호출useEffect의존성 배열을 빈 배열[]로 변경 (cleanup 함수만 유지)참고: OnboardingProfile.tsx의 개선된 패턴
const objectUrlRef = useRef<string | null>(null); const handleFileChange = (e: React.ChangeEvent<HTMLInputElement>) => { // ... const nextObjectUrl = URL.createObjectURL(file); if (objectUrlRef.current) { URL.revokeObjectURL(objectUrlRef.current); } objectUrlRef.current = nextObjectUrl; setPreviewUrl(nextObjectUrl); }; useEffect(() => { return () => { if (objectUrlRef.current) { URL.revokeObjectURL(objectUrlRef.current); } }; }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/session/components/SessionCreateForm.tsx` around lines 69 - 78, The current SessionCreateForm creates a blob URL during render via imagePreviewUrl = URL.createObjectURL(selectedImage), causing new URLs every render and repeated effect cleanup; fix by moving createObjectURL into the image selection handler (e.g., handleFileChange / onImageChange), track the current URL with a useRef (objectUrlRef), call URL.revokeObjectURL on the previous ref before assigning a new URL and set state with the new preview URL, and change the cleanup effect to useEffect(() => () => { if (objectUrlRef.current) URL.revokeObjectURL(objectUrlRef.current); }, []) so the render no longer calls createObjectURL and cleanup runs only on unmount.src/features/session/components/Banner/Banner.tsx (1)
28-53: 🧹 Nitpick | 🔵 TrivialReact Compiler가 활성화되어 있어 현재 코드는 안전합니다.
분석:
next.config.ts에서reactCompiler: true로 설정되어 있습니다. React Compiler 1.0.0+는 컴포넌트 로컬 함수의 identity를 자동으로 메모이제이션하므로,startAutoRoll과clearAutoRoll은 렌더마다 재생성되지 않습니다. 따라서 useEffect가 불필요하게 재실행되는 문제는 발생하지 않습니다.개선 제안: 다음 구조로 리팩터링하면 코드가 더 명확해집니다. (필수 아님)
- interval 생성/정리 로직을 useEffect 안으로 옮기기
- dependency 배열을
[isHovered]로 단순화💡 코드 개선 예시
- const clearAutoRoll = () => { - if (intervalRef.current) { - clearInterval(intervalRef.current); - intervalRef.current = null; - } - }; - - const startAutoRoll = () => { - clearAutoRoll(); - intervalRef.current = setInterval(() => { - setActiveIndex((prev) => { - const next = (prev + 1) % 2; - setDirection(next === 1 ? "forward" : "backward"); - return next; - }); - }, AUTO_ROLL_INTERVAL); - }; - useEffect(() => { - if (isHovered) { - clearAutoRoll(); - } else { - startAutoRoll(); - } - return clearAutoRoll; - }, [isHovered, startAutoRoll, clearAutoRoll]); + if (intervalRef.current) { + clearInterval(intervalRef.current); + intervalRef.current = null; + } + + if (isHovered) return; + + intervalRef.current = setInterval(() => { + setActiveIndex((prev) => { + const next = (prev + 1) % 2; + setDirection(next === 1 ? "forward" : "backward"); + return next; + }); + }, AUTO_ROLL_INTERVAL); + + return () => { + if (intervalRef.current) { + clearInterval(intervalRef.current); + intervalRef.current = null; + } + }; + }, [isHovered]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/session/components/Banner/Banner.tsx` around lines 28 - 53, The useEffect currently depends on startAutoRoll and clearAutoRoll leading to unnecessary dependencies; refactor by moving the interval creation/cleanup inside the useEffect that watches isHovered, using intervalRef and AUTO_ROLL_INTERVAL directly: on mount/update if !isHovered set intervalRef.current = setInterval(...) that calls setActiveIndex and setDirection as in startAutoRoll, and always return a cleanup that clears intervalRef via clearInterval and nulls it (previous clearAutoRoll logic); remove startAutoRoll and clearAutoRoll from the effect dependency array and keep the effect dependency array as [isHovered].src/components/DatePicker/useDatePicker.ts (1)
59-140: 🧹 Nitpick | 🔵 TrivialReact Compiler는 IIFE 표현식을 자동 메모이제이션하지 않음
calendarDaysIIFE가 매 렌더마다 실행됩니다. React Compiler는 IIFE를 포함한 독립적인 JavaScript 표현식을 메모이제이션하지 않으므로 불필요한 재계산이 발생합니다. 계산 비용이 가볍긴 하나(O(42)), 최적화를 위해useMemo로 감싸기를 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DatePicker/useDatePicker.ts` around lines 59 - 140, calendarDays is computed inside an IIFE and recalculates on every render; wrap that computation in React's useMemo to memoize it. Import useMemo and replace the IIFE with useMemo(() => { ...compute days using currentMonth, selectedRange, isSameDay, isDateInRange... }, [currentMonth, selectedRange.startDate, selectedRange.endDate]) so the calendar only recomputes when currentMonth or the selectedRange boundaries change; keep the existing logic for today/twoWeeksLater inside the memo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/DatePicker/DatePicker.tsx`:
- Around line 138-142: The hour/minute local state in DatePicker is only
initialized from the value prop and won't update when value changes; add a
useEffect inside the DatePicker component that listens to value (and possibly
value.getHours/getMinutes) and calls setHour(value ? value.getHours() : new
Date().getHours()) and setMinute(value ? Math.floor(value.getMinutes() / 5) * 5
: Math.floor(new Date().getMinutes() / 5) * 5) to keep the hour and minute state
synchronized with external prop updates; ensure you reference the existing hour,
minute, setHour, setMinute, and value symbols so the time picker UI updates in
controlled mode.
In `@src/components/Dropdown/useDropdown.ts`:
- Around line 33-40: The selectOption function currently changes state and calls
onChange even when the dropdown is disabled; add an early-return guard at the
start of selectOption to check the disabled flag (e.g., if (disabled) return;)
so no further calls to setInternalValue, onChange, setIsOpen, or setFocusedIndex
occur when disabled, leaving the function otherwise unchanged (retain
isControlled, setInternalValue, onChange, setIsOpen, setFocusedIndex behavior).
In `@src/components/Icon/ChevronRightIcon.tsx`:
- Around line 17-22: The ChevronRightIcon component currently uses the
UPPER_SNAKE_CASE constant CHEVRON_RIGHT_SVG while other icons use PascalCase
names; to unify naming, rename CHEVRON_RIGHT_SVG to a PascalCase identifier
(e.g., ChevronRightSvg) and update all references (including the export/import
sites and the Icon usage inside ChevronRightIcon) so the component
ChevronRightIcon and matching icon constants follow the same PascalCase
convention across the codebase.
In `@src/components/Icon/HeartIcon.tsx`:
- Around line 18-23: HeartIconProps currently duplicates the ref definition;
remove the redundant ref in the function signature and centralize it in the
type: update HeartIconProps to explicitly include ref?:
React.Ref<HTMLSpanElement> (based on IconProps minus "svg") and change the
HeartIcon parameter typing from HeartIconProps & { ref?:
React.Ref<HTMLSpanElement> } to just HeartIconProps (and update the destructured
parameter to avoid shadowing the reserved ref identifier if needed); apply the
same pattern consistently across other icon components to keep signatures
uniform (reference: HeartIconProps, IconProps, HeartIcon).
In `@src/components/Icon/ThumbDownIcon.tsx`:
- Around line 20-25: 현재 ThumbDownIcon 함수가 인라인으로 ref 타입을 intersection({ ref?:
React.Ref<HTMLSpanElement> })해 사용하고 있으니, Icon.tsx의 IconProps에 ref?:
React.Ref<HTMLSpanElement>를 추가하고 ThumbDownIcon의 파라미터 타입을 ThumbDownIconProps만
사용하도록 변경하세요; 즉 IconProps에 ref를 선언한 뒤 ThumbDownIcon({ ref, ...props }:
ThumbDownIconProps)에서 Icon 컴포넌트로 ref를 전달하도록 수정하고 관련 타입 임포트(IconProps 또는
ThumbDownIconProps)가 올바른지 확인하세요 (참조: ThumbDownIcon, IconProps, Icon,
ThumbDownSvg).
In `@src/components/ImageUploader/ImageUploader.tsx`:
- Around line 167-173: The current ref assignment in the ImageUploader component
sets ref.current = node but React.Ref<T> may be readonly, causing a TypeScript
error; update the handler that sets external refs (the block that assigns
internalInputRef.current and then handles the incoming ref variable) to safely
support both callback refs and object refs by checking typeof ref === "function"
for callbacks and otherwise casting ref to
React.MutableRefObject<HTMLInputElement | null> (or appropriate element type)
before assigning .current, ensuring internalInputRef and the external ref are
set without readonly violations.
In `@src/components/Input/Textarea.tsx`:
- Line 179: Textarea 컴포넌트의 주석(현재 "// Priority: errorMessage > helperText")을 한국어로
바꾸고 '우선순위가 왜 이런지'를 설명하는 이유 중심의 코멘트를 작성하세요; 예를 들어 errorMessage가 유효성 실패로 사용자에게
즉각적·명확한 피드백을 주기 때문에 helperText보다 우선한다는 근거를 포함하여 Textarea 컴포넌트의 내부 로직(예:
errorMessage 처리 흐름과 helperText 대체 동작)을 참조해 설명하세요.
In `@src/components/ProgressBar/ProgressBar.tsx`:
- Around line 25-39: The ProgressBar component currently passes NaN through to
the DOM because clampedProgress uses Math.min/Math.max which don't normalize
NaN; update the normalization of the incoming progress value (used to compute
clampedProgress, aria-valuenow, and the inline width) by first validating
progress with Number.isFinite(progress) and falling back to 0 before applying
Math.min/Math.max; specifically, change how clampedProgress is computed in
ProgressBar.tsx so that if Number.isFinite(progress) is false it uses 0,
otherwise it clamps the numeric value to [0,100], ensuring aria-valuenow and the
width style never receive NaN or "NaN%".
In `@src/components/StepperSlide/StepperSlide.tsx`:
- Around line 102-120: The slider div in StepperSlide (the element with
role="slider" and handlers handleMouseDown/handleKeyDown) lacks an accessible
name; fix by adding either aria-label (e.g., aria-label="stepper value") or
aria-labelledby pointing to an external label element so screen readers can
identify the control, and ensure the chosen label reflects the slider’s purpose
and current value (aria-valuenow remains) while keeping existing
aria-valuemin/aria-valuemax/aria-disabled intact.
- Around line 45-48: The comments in StepperSlide.tsx are restating the UI
structure instead of explaining intent; remove or rewrite the redundant "what"
comments (e.g., the bubble area comment near the JSX block containing
myFocusPercentage and the other noted comment blocks at the same component:
around the sections that render focus bubbles and controls) and replace them
only where necessary with brief "why" comments that explain intent, constraints,
or trade-offs for future maintainers (for example why myFocusPercentage is
conditionally rendered, any accessibility/format choices, or layout quirks).
Locate the comments around the JSX fragments that render the bubble ("말풍선 영역"),
the "내 집중도 말풍선" conditional using myFocusPercentage, and the other listed
comment regions (lines referenced in the review) and remove purely descriptive
comments or convert them to intent-focused notes tied to the component/handlers
(e.g., StepperSlide component behavior, conditional rendering, and styling
decisions).
In `@src/components/StepperSlide/useStepperSlide.ts`:
- Around line 32-38: getValueFromPosition is recreated on every render causing
the useEffect that registers mousemove/mouseup to re-run and churn listeners;
extract the coordinate-to-value logic into a pure helper function outside the
component (e.g., computeValueFromPosition(clientX, trackRef, min, max) or
similar) and update the effect to depend only on stable values (isDragging, min,
max, onChange) and use the external helper instead of the inline
getValueFromPosition; ensure the effect references trackRef.current inside the
handler without capturing a new function each render so listeners are not
re-registered during drag.
In `@src/components/Toast/Toast.tsx`:
- Line 1: This file declares a React component (uses useState and useEffect in
src/components/Toast/Toast.tsx) but is missing the Next.js client directive; add
the directive "use client"; as the very first line of the file so the component
(Toast using useState and useEffect) runs on the client and avoids server/client
boundary build/runtime errors.
- Around line 31-44: The auto-close effect uses duration but captures id and
onClose stalely; update Toast by separating the animation/state timer from the
actual close callback: keep handleClose (which calls setIsClosing(true) then
calls onClose(id) after 200ms) but move the auto-close logic into a dedicated
useEffect that depends on [duration, id, onClose] (or capture latest id/onClose
via refs) so the timeout invokes the current onClose for the correct id, and
ensure the cleanup clears the timer; also keep the short 200ms animation delay
inside handleClose so the animation and actual removal remain decoupled.
In `@src/features/lobby/hooks/useCountdown.ts`:
- Around line 14-16: The calculateRemaining function is declared outside the
useEffect and triggers react-hooks/exhaustive-deps warnings because it is
referenced inside the effect; to fix, either move calculateRemaining into the
useEffect so it doesn't create a stale closure or memoize it with useCallback
including targetTime in its dependency array; update references to
calculateRemaining accordingly (look for calculateRemaining and the useEffect
that calls it) so ESLint no longer complains and the effect only depends on
targetTime.
In `@src/features/session/components/LeaveConfirmDialog.tsx`:
- Around line 29-34: Move the showModal() side effect out of the callback ref
setDialogRef and control dialog open state inside a useEffect that watches
dialogRef.current and any relevant open-state prop; specifically, remove the
call to node.showModal() from setDialogRef (which should only assign
dialogRef.current = node) and add a useEffect that calls
dialogRef.current.showModal() when the dialog element becomes non-null and you
intend it opened, and calls dialogRef.current.close() when it should be closed,
referencing dialogRef and the component's open-related state/props to drive the
effect.
In `@src/features/session/components/SessionList/ParticipantsFilter.tsx`:
- Around line 37-41: Extract the repeated popover close-and-focus logic into a
custom hook (e.g., useSessionFilterPopover) to replace the containerRef +
triggerRef + closeFilter + useClickOutside pattern: move creation of
containerRef and triggerRef, the closeFilter handler (which calls
onOpenChange(false) and triggers triggerRef.current?.focus()), and the
useClickOutside subscription into the hook, and have it accept/return the open
state and onOpenChange callback (or provide open toggles) so callers can simply
use const {containerRef, triggerRef, close} = useSessionFilterPopover(isOpen,
onOpenChange) and remove duplicated logic from components using those symbols.
In `@src/features/session/hooks/useSessionListFilters.ts`:
- Around line 112-119: The setPage function currently stringifies the incoming
page directly which allows invalid values like 0, negative numbers, NaN, or
Infinity to be written to the query; update setPage (which calls updateFilters)
to first coerce and validate the page: convert to a finite integer (e.g.,
Number(page) then Math.floor or parseInt), check Number.isFinite and that the
integer is > 1, and only then pass String(validPage) to updateFilters, otherwise
pass null for page; keep the updateFilters call and its { resetPage: false }
option intact.
In `@src/hooks/useDragAndDrop.ts`:
- Around line 101-121: The handleDragOver handler currently updates state even
when the dropzone is disabled; after the existing e.preventDefault() and
e.stopPropagation() calls in handleDragOver add an early guard that returns
immediately if disabled is true to prevent updates to mousePosition and
dragFileName; specifically, inside handleDragOver (the function that uses
containerRef, setMousePosition, setDragFileName and calls extractFileName) check
the disabled flag and return before computing rect/setMousePosition or calling
extractFileName so the component does not rerender or leave stale drag metadata
when disabled.
---
Outside diff comments:
In `@src/components/DatePicker/useDatePicker.ts`:
- Around line 59-140: calendarDays is computed inside an IIFE and recalculates
on every render; wrap that computation in React's useMemo to memoize it. Import
useMemo and replace the IIFE with useMemo(() => { ...compute days using
currentMonth, selectedRange, isSameDay, isDateInRange... }, [currentMonth,
selectedRange.startDate, selectedRange.endDate]) so the calendar only recomputes
when currentMonth or the selectedRange boundaries change; keep the existing
logic for today/twoWeeksLater inside the memo.
In `@src/features/session/components/Banner/Banner.tsx`:
- Around line 28-53: The useEffect currently depends on startAutoRoll and
clearAutoRoll leading to unnecessary dependencies; refactor by moving the
interval creation/cleanup inside the useEffect that watches isHovered, using
intervalRef and AUTO_ROLL_INTERVAL directly: on mount/update if !isHovered set
intervalRef.current = setInterval(...) that calls setActiveIndex and
setDirection as in startAutoRoll, and always return a cleanup that clears
intervalRef via clearInterval and nulls it (previous clearAutoRoll logic);
remove startAutoRoll and clearAutoRoll from the effect dependency array and keep
the effect dependency array as [isHovered].
In `@src/features/session/components/SessionCreateForm.tsx`:
- Around line 69-78: The current SessionCreateForm creates a blob URL during
render via imagePreviewUrl = URL.createObjectURL(selectedImage), causing new
URLs every render and repeated effect cleanup; fix by moving createObjectURL
into the image selection handler (e.g., handleFileChange / onImageChange), track
the current URL with a useRef (objectUrlRef), call URL.revokeObjectURL on the
previous ref before assigning a new URL and set state with the new preview URL,
and change the cleanup effect to useEffect(() => () => { if
(objectUrlRef.current) URL.revokeObjectURL(objectUrlRef.current); }, []) so the
render no longer calls createObjectURL and cleanup runs only on unmount.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 198a3214-9a3b-4ed5-9f92-b31bec11ea97
📒 Files selected for processing (92)
src/components/Badge/Badge.tsxsrc/components/Button/Button.tsxsrc/components/ButtonGroup/ButtonGroup.tsxsrc/components/CategoryFilterButton/CategoryFilterButton.tsxsrc/components/ChipGroup/ChipGroup.tsxsrc/components/DatePicker/DatePicker.tsxsrc/components/DatePicker/DatePicker.types.tssrc/components/DatePicker/TimePickerPanel.tsxsrc/components/DatePicker/useDatePicker.tssrc/components/Dropdown/useDropdown.tssrc/components/Filter/Filter.tsxsrc/components/Icon/AlertIcon.tsxsrc/components/Icon/ArrowLeftIcon.tsxsrc/components/Icon/ArrowRightIcon.tsxsrc/components/Icon/ArrowRotateRightIcon.tsxsrc/components/Icon/BarGraphIcon.tsxsrc/components/Icon/CalendarIcon.tsxsrc/components/Icon/ChatIcon.tsxsrc/components/Icon/CheckIcon.tsxsrc/components/Icon/ChevronDownIcon.tsxsrc/components/Icon/ChevronLeftIcon.tsxsrc/components/Icon/ChevronRightIcon.tsxsrc/components/Icon/ClearIcon.tsxsrc/components/Icon/ClockIcon.tsxsrc/components/Icon/CloseIcon.tsxsrc/components/Icon/CloudUploadIcon.tsxsrc/components/Icon/DefaultProfileIcon.tsxsrc/components/Icon/EditContainedIcon.tsxsrc/components/Icon/EditIcon.tsxsrc/components/Icon/EditProfileIcon.tsxsrc/components/Icon/FileIcon.tsxsrc/components/Icon/FrameIcon.tsxsrc/components/Icon/GoogleIcon.tsxsrc/components/Icon/HeartFillIcon.tsxsrc/components/Icon/HeartIcon.tsxsrc/components/Icon/Icon.tsxsrc/components/Icon/InfoIcon.tsxsrc/components/Icon/KakaoIcon.tsxsrc/components/Icon/LogoutIcon.tsxsrc/components/Icon/MinusIcon.tsxsrc/components/Icon/NoteIcon.tsxsrc/components/Icon/PencilIcon.tsxsrc/components/Icon/PlusIcon.tsxsrc/components/Icon/ProfileCircleIcon.tsxsrc/components/Icon/ProfileIcon.tsxsrc/components/Icon/SearchIcon.tsxsrc/components/Icon/ShareIcon.tsxsrc/components/Icon/StarIcon.tsxsrc/components/Icon/ThumbDownIcon.tsxsrc/components/Icon/ThumbUpIcon.tsxsrc/components/Icon/ThumbsDownIcon.tsxsrc/components/Icon/ThumbsUpIcon.tsxsrc/components/Icon/UsersIcon.tsxsrc/components/Icon/XIcon.tsxsrc/components/ImageUploader/ImageUploader.tsxsrc/components/Input/TextInput.tsxsrc/components/Input/Textarea.tsxsrc/components/ProgressBar/ProgressBar.tsxsrc/components/ProgressRing/ProgressRing.tsxsrc/components/SearchInput/SearchInput.tsxsrc/components/StepperSlide/StepperSlide.tsxsrc/components/StepperSlide/useStepperSlide.tssrc/components/Thumbnail/Thumbnail.tsxsrc/components/Toast/Toast.tsxsrc/features/lobby/components/KickConfirmDialog.tsxsrc/features/lobby/components/KickedDialog.tsxsrc/features/lobby/components/SessionJoinModal.tsxsrc/features/lobby/hooks/useCountdown.tssrc/features/member/components/Profile/AccountProfileCard.tsxsrc/features/member/components/Profile/DeleteAccountWarnings.tsxsrc/features/member/components/Profile/LogoutModal.tsxsrc/features/member/hooks/useDeleteAccountFlow.tssrc/features/session/components/Banner/Banner.tsxsrc/features/session/components/LeaveConfirmDialog.tsxsrc/features/session/components/MyTimer/MyTimer.tsxsrc/features/session/components/RecommendedSection/RecommendedSectionContent.tsxsrc/features/session/components/SessionCreateForm.tsxsrc/features/session/components/SessionList/DateRangeFilter.tsxsrc/features/session/components/SessionList/DurationFilter.tsxsrc/features/session/components/SessionList/ParticipantsFilter.tsxsrc/features/session/components/SessionList/SortFilter.tsxsrc/features/session/components/SessionList/StartTimeFilter.tsxsrc/features/session/components/SessionPageContent.tsxsrc/features/session/components/SessionTimerSection.tsxsrc/features/session/hooks/useCountUp.tssrc/features/session/hooks/useRecommendedCarousel.tssrc/features/session/hooks/useSessionListFilters.tssrc/features/session/hooks/useSessionTimer.tssrc/features/session/hooks/useShareSession.tssrc/hooks/useDialog.tssrc/hooks/useDragAndDrop.tssrc/lib/sse/useSSE.ts
| const selectOption = (optionValue: string) => { | ||
| if (!isControlled) { | ||
| setInternalValue(optionValue); | ||
| } | ||
| onChange?.(optionValue); | ||
| setIsOpen(false); | ||
| setFocusedIndex(-1); | ||
| }; |
There was a problem hiding this comment.
selectOption에 disabled 가드가 필요합니다.
문제: Line 33-40의 selectOption는 disabled 상태에서도 onChange와 상태 변경을 수행합니다.
영향: 비활성화된 Dropdown에서 값 변경이 발생해 UX/상태 일관성이 깨질 수 있습니다.
권장 수정: 함수 시작부에 Early Return(if (disabled) return;)을 추가하세요.
권장 수정 diff
const selectOption = (optionValue: string) => {
+ if (disabled) return;
if (!isControlled) {
setInternalValue(optionValue);
}
onChange?.(optionValue);
setIsOpen(false);
setFocusedIndex(-1);
};As per coding guidelines "Use Early Return pattern to minimize else statements".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Dropdown/useDropdown.ts` around lines 33 - 40, The
selectOption function currently changes state and calls onChange even when the
dropdown is disabled; add an early-return guard at the start of selectOption to
check the disabled flag (e.g., if (disabled) return;) so no further calls to
setInternalValue, onChange, setIsOpen, or setFocusedIndex occur when disabled,
leaving the function otherwise unchanged (retain isControlled, setInternalValue,
onChange, setIsOpen, setFocusedIndex behavior).
There was a problem hiding this comment.
@Rootjang92 알겠습니다, 해당 제안은 무시하겠습니다.
| export function ChevronRightIcon({ | ||
| ref, | ||
| ...props | ||
| }: ChevronRightIconProps & { ref?: React.Ref<HTMLSpanElement> }) { | ||
| return <Icon ref={ref} svg={CHEVRON_RIGHT_SVG} {...props} />; | ||
| }); | ||
|
|
||
| ChevronRightIcon.displayName = "ChevronRightIcon"; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM!
React 19 ref 패턴이 올바르게 적용되었습니다.
참고: CHEVRON_RIGHT_SVG 상수명이 UPPER_SNAKE_CASE를 따르는 점은 코딩 가이드라인에 부합합니다. 다른 아이콘 파일들은 PascalCase(PlusSvg, ThumbUpSvg 등)를 사용 중이므로, 일관성을 위해 전체 아이콘 컴포넌트에서 네이밍 컨벤션 통일을 고려해 볼 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Icon/ChevronRightIcon.tsx` around lines 17 - 22, The
ChevronRightIcon component currently uses the UPPER_SNAKE_CASE constant
CHEVRON_RIGHT_SVG while other icons use PascalCase names; to unify naming,
rename CHEVRON_RIGHT_SVG to a PascalCase identifier (e.g., ChevronRightSvg) and
update all references (including the export/import sites and the Icon usage
inside ChevronRightIcon) so the component ChevronRightIcon and matching icon
constants follow the same PascalCase convention across the codebase.
| export type HeartIconProps = Omit<IconProps, "svg">; | ||
|
|
||
| export const HeartIcon = forwardRef<HTMLSpanElement, HeartIconProps>((props, ref) => { | ||
| export function HeartIcon({ | ||
| ref, | ||
| ...props | ||
| }: HeartIconProps & { ref?: React.Ref<HTMLSpanElement> }) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'export type (HeartIconProps|CheckIconProps|FrameIconProps|NoteIconProps|DefaultProfileIconProps|ArrowRightIconProps)\b' src/components/Icon
rg -n '\b(HeartIconProps|CheckIconProps|FrameIconProps|NoteIconProps|DefaultProfileIconProps|ArrowRightIconProps)\b' src \
-g '!**/HeartIcon.tsx' \
-g '!**/CheckIcon.tsx' \
-g '!**/FrameIcon.tsx' \
-g '!**/NoteIcon.tsx' \
-g '!**/DefaultProfileIcon.tsx' \
-g '!**/ArrowRightIcon.tsx'Repository: dnd-side-project/dnd-14th-9-frontend
Length of output: 642
🏁 Script executed:
#!/bin/bash
# Check the complete HeartIcon.tsx file
cat -n src/components/Icon/HeartIcon.tsxRepository: dnd-side-project/dnd-14th-9-frontend
Length of output: 1513
🏁 Script executed:
#!/bin/bash
# Check IconProps base type definition
fd "Icon\.tsx|IconProps" src/components/Icon --type f | head -5
rg -n "export.*IconProps|interface IconProps" src/components/Icon -A 3Repository: dnd-side-project/dnd-14th-9-frontend
Length of output: 12674
🏁 Script executed:
#!/bin/bash
# Verify if these props types are actually used anywhere with imports
rg -n "import.*Props from" src \
-g '**/*.{ts,tsx}' \
| grep -E "(HeartIconProps|CheckIconProps|FrameIconProps|NoteIconProps|DefaultProfileIconProps|ArrowRightIconProps)"Repository: dnd-side-project/dnd-14th-9-frontend
Length of output: 62
🏁 Script executed:
#!/bin/bash
# Get the complete IconProps interface definition
cat -n src/components/Icon/Icon.tsx | head -35Repository: dnd-side-project/dnd-14th-9-frontend
Length of output: 1141
HeartIconProps의 함수 시그니처가 중복 타입 정의를 가집니다.
문제: IconProps는 ref?: React.Ref<HTMLSpanElement>를 명시하고, HeartIconProps = Omit<IconProps, "svg">는 svg만 제거하므로 ref는 이미 포함됩니다. 그런데 함수 매개변수 타입에서 HeartIconProps & { ref?: React.Ref<HTMLSpanElement> }로 중복하게 정의되었습니다.
권장 수정: HeartIconProps 타입 정의에 ref를 명시하고 함수 시그니처에서 중복을 제거하세요. 이 패턴이 다른 Icon 컴포넌트에도 반복되므로 일관성을 위해 통일해야 합니다.
수정 예시
-export type HeartIconProps = Omit<IconProps, "svg">;
+export type HeartIconProps = Omit<IconProps, "svg"> & {
+ ref?: React.Ref<HTMLSpanElement>;
+};
export function HeartIcon({
ref,
...props
-}: HeartIconProps & { ref?: React.Ref<HTMLSpanElement> }) {
+}: HeartIconProps) {
return <Icon ref={ref} svg={HeartSvg} {...props} />;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Icon/HeartIcon.tsx` around lines 18 - 23, HeartIconProps
currently duplicates the ref definition; remove the redundant ref in the
function signature and centralize it in the type: update HeartIconProps to
explicitly include ref?: React.Ref<HTMLSpanElement> (based on IconProps minus
"svg") and change the HeartIcon parameter typing from HeartIconProps & { ref?:
React.Ref<HTMLSpanElement> } to just HeartIconProps (and update the destructured
parameter to avoid shadowing the reserved ref identifier if needed); apply the
same pattern consistently across other icon components to keep signatures
uniform (reference: HeartIconProps, IconProps, HeartIcon).
| export function ThumbDownIcon({ | ||
| ref, | ||
| ...props | ||
| }: ThumbDownIconProps & { ref?: React.Ref<HTMLSpanElement> }) { | ||
| return <Icon ref={ref} svg={ThumbDownSvg} {...props} />; | ||
| }); | ||
|
|
||
| ThumbDownIcon.displayName = "ThumbDownIcon"; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
ref 타입 정의 방식 개선 권장.
현재 ThumbDownIconProps & { ref?: React.Ref<HTMLSpanElement> } 형태로 인라인 intersection 사용 중. 동작은 정상이나, IconProps에 ref를 포함시키면 모든 아이콘 컴포넌트에서 일관된 타입 정의 가능.
♻️ 제안: IconProps에 ref 포함
Icon.tsx의 IconProps에 ref 타입을 추가하면 각 아이콘에서 intersection 불필요:
// Icon.tsx에서
export interface IconProps {
// ... existing props
ref?: React.Ref<HTMLSpanElement>;
}
// ThumbDownIcon.tsx에서
export function ThumbDownIcon({ ref, ...props }: ThumbDownIconProps) {
return <Icon ref={ref} svg={ThumbDownSvg} {...props} />;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Icon/ThumbDownIcon.tsx` around lines 20 - 25, 현재 ThumbDownIcon
함수가 인라인으로 ref 타입을 intersection({ ref?: React.Ref<HTMLSpanElement> })해 사용하고 있으니,
Icon.tsx의 IconProps에 ref?: React.Ref<HTMLSpanElement>를 추가하고 ThumbDownIcon의 파라미터
타입을 ThumbDownIconProps만 사용하도록 변경하세요; 즉 IconProps에 ref를 선언한 뒤 ThumbDownIcon({
ref, ...props }: ThumbDownIconProps)에서 Icon 컴포넌트로 ref를 전달하도록 수정하고 관련 타입
임포트(IconProps 또는 ThumbDownIconProps)가 올바른지 확인하세요 (참조: ThumbDownIcon, IconProps,
Icon, ThumbDownSvg).
| const setDialogRef = (node: HTMLDialogElement | null) => { | ||
| if (node && !node.open) { | ||
| node.showModal(); | ||
| } | ||
| dialogRef.current = node; | ||
| }, []); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In React 19, when a callback ref function identity changes between renders, does React call the previous ref with null and the new ref with the element again? What is the recommended pattern for imperative side effects like dialog.showModal()?
💡 Result:
In React 19, when a callback ref function's identity changes between renders (e.g., due to not being memoized with useCallback), React treats it as a new ref. It calls the previous ref callback with null (cleanup) and then calls the new ref callback with the DOM element (attach). This behavior is consistent with pre-React 19 callback ref handling and is unchanged in React 19, which primarily adds support for ref callbacks to return cleanup functions (called on unmount or ref change without passing null). To avoid unnecessary calls when the ref function changes unintentionally, memoize it with useCallback (ideally with empty deps [] or extracted outside the component for mount-only effects). For imperative side effects like dialog.showModal, the recommended pattern uses useRef + useEffect: const dialogRef = useRef(null); useEffect( => { if (isOpen && dialogRef.current) { dialogRef.current.showModal; } return => { if (dialogRef.current) { dialogRef.current.close; } }; }, [isOpen]); This safely handles the DOM imperative API after commit. Callback refs can work for simple mount effects but are less suitable for state-driven imperative calls like showModal due to timing (called during render/commit, before effects). useEffect is preferred for side effects needing state/props or post-DOM guarantees.
Citations:
- 1: https://tkdodo.eu/blog/ref-callbacks-react-19-and-the-compiler
- 2: https://www.codingtag.com/ref-callbacks-and-cleanup-in-react-19
- 3: Bug: React 18 vs 19 callback refs facebook/react#34071
- 4: https://dzone.com/articles/react-callback-refs-guide
- 5: https://clhenrick.io/blog/react-a11y-modal-dialog/
- 6: use of React 19 ref callbacks for IntersectionObserver tracking thebuilder/react-intersection-observer#718
- 7: https://react.dev/reference/react/useImperativeHandle
- 8: https://react.dev/reference/react/useRef
- 9: https://beta.reactjs.org/learn/referencing-values-with-refs
- 10: https://beta.reactjs.org/learn/manipulating-the-dom-with-refs
🏁 Script executed:
cat -n src/features/session/components/LeaveConfirmDialog.tsxRepository: dnd-side-project/dnd-14th-9-frontend
Length of output: 3071
callback ref에서 showModal() side effect를 제거하고 useEffect로 제어하세요
문제: 라인 29-34의 setDialogRef는 매 렌더마다 새 함수가 되므로, React가 이전 callback을 null로, 새 callback을 element로 호출합니다. 이 과정에서 showModal() side effect가 반복 실행됩니다.
영향: 렌더링 순서에 따라 모달이 불필요하게 재오픈되거나 여러 번 실행될 수 있습니다.
권장 수정:
수정 diff
-import { useRef } from "react";
+import { useEffect, useRef } from "react";
@@
- const setDialogRef = (node: HTMLDialogElement | null) => {
- if (node && !node.open) {
- node.showModal();
- }
- dialogRef.current = node;
- };
+ useEffect(() => {
+ const node = dialogRef.current;
+ if (node && !node.open) {
+ node.showModal();
+ }
+ return () => {
+ if (node?.open) {
+ node.close();
+ }
+ };
+ }, []);
@@
- ref={setDialogRef}
+ ref={dialogRef}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/session/components/LeaveConfirmDialog.tsx` around lines 29 - 34,
Move the showModal() side effect out of the callback ref setDialogRef and
control dialog open state inside a useEffect that watches dialogRef.current and
any relevant open-state prop; specifically, remove the call to node.showModal()
from setDialogRef (which should only assign dialogRef.current = node) and add a
useEffect that calls dialogRef.current.showModal() when the dialog element
becomes non-null and you intend it opened, and calls dialogRef.current.close()
when it should be closed, referencing dialogRef and the component's open-related
state/props to drive the effect.
There was a problem hiding this comment.
react complier가 메모이제이션 하므로 수정 불필요
There was a problem hiding this comment.
@Rootjang92, 확인했습니다. React Compiler가 setDialogRef를 자동으로 메모이즈하여 함수 identity를 안정적으로 유지한다면, callback ref가 mount/unmount 시점에만 호출되므로 showModal() 반복 실행 문제는 발생하지 않습니다. 설명 감사합니다.
🧠 Learnings used
Learnt from: Rootjang92
Repo: dnd-side-project/dnd-14th-9-frontend PR: 231
File: src/features/lobby/hooks/useLeaveOnUnmount.ts:5-10
Timestamp: 2026-03-18T06:44:40.003Z
Learning: In the dnd-14th-9-frontend project (Next.js/React/TypeScript), when typing a ref parameter in a hook interface, use `React.MutableRefObject<T>` (not `React.RefObject<T>`) when the ref is created via `useRef(initialValue)` with a non-null initial value (e.g., `useRef(false)`). `RefObject<T>` implies `readonly current: T | null`, while `MutableRefObject<T>` correctly reflects mutability for flag refs.
| const closeFilter = () => { | ||
| onOpenChange(false); | ||
| triggerRef.current?.focus(); | ||
| }, [onOpenChange]); | ||
| }; | ||
| useClickOutside(containerRef, closeFilter, isOpen); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
필터 닫기/포커스 복귀 로직은 공통 훅으로 추출을 권장합니다.
문제: 동일한 containerRef + triggerRef + closeFilter + useClickOutside 패턴이 여러 필터 컴포넌트에 반복됩니다.
영향: 동작 변경 시 파일별 누락/불일치 위험이 커집니다.
권장 수정: 공통 훅(예: useSessionFilterPopover)으로 추출해 일관성을 강제하세요.
As per coding guidelines, **/*.{tsx,jsx}: Extract complex logic into custom hooks to keep components clean.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/session/components/SessionList/ParticipantsFilter.tsx` around
lines 37 - 41, Extract the repeated popover close-and-focus logic into a custom
hook (e.g., useSessionFilterPopover) to replace the containerRef + triggerRef +
closeFilter + useClickOutside pattern: move creation of containerRef and
triggerRef, the closeFilter handler (which calls onOpenChange(false) and
triggers triggerRef.current?.focus()), and the useClickOutside subscription into
the hook, and have it accept/return the open state and onOpenChange callback (or
provide open toggles) so callers can simply use const {containerRef, triggerRef,
close} = useSessionFilterPopover(isOpen, onOpenChange) and remove duplicated
logic from components using those symbols.
| const handleDragOver = (e: React.DragEvent<HTMLElement>) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
|
|
||
| if (containerRef?.current) { | ||
| const rect = containerRef.current.getBoundingClientRect(); | ||
| setMousePosition({ | ||
| x: e.clientX - rect.left, | ||
| y: e.clientY - rect.top, | ||
| }); | ||
| } | ||
|
|
||
| // 파일명이 아직 설정되지 않은 경우 추출 시도 | ||
| if (!dragFileName || dragFileName === "파일") { | ||
| const fileName = extractFileName(e); | ||
| if (fileName && fileName !== "파일") { | ||
| setDragFileName(fileName); | ||
| } else if (!dragFileName && fileName) { | ||
| setDragFileName(fileName); | ||
| } | ||
| }, | ||
| [disabled, onFileDrop, resetDragState] | ||
| ); | ||
| } |
There was a problem hiding this comment.
disabled 상태에서도 onDragOver가 state를 갱신합니다.
문제: handleDragOver에는 disabled 가드가 없어 mousePosition과 dragFileName이 계속 바뀝니다.
영향: 비활성화된 드롭존에서도 불필요한 리렌더가 발생하고, onDragLeave는 조기 반환하므로 드래그 메타데이터가 남을 수 있습니다.
권장 수정: e.preventDefault()와 e.stopPropagation() 직후 if (disabled) return;로 빠져 state 변경을 막으세요.
✏️ 제안 수정
const handleDragOver = (e: React.DragEvent<HTMLElement>) => {
e.preventDefault();
e.stopPropagation();
+ if (disabled) return;
if (containerRef?.current) {
const rect = containerRef.current.getBoundingClientRect();
setMousePosition({📝 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.
| const handleDragOver = (e: React.DragEvent<HTMLElement>) => { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| if (containerRef?.current) { | |
| const rect = containerRef.current.getBoundingClientRect(); | |
| setMousePosition({ | |
| x: e.clientX - rect.left, | |
| y: e.clientY - rect.top, | |
| }); | |
| } | |
| // 파일명이 아직 설정되지 않은 경우 추출 시도 | |
| if (!dragFileName || dragFileName === "파일") { | |
| const fileName = extractFileName(e); | |
| if (fileName && fileName !== "파일") { | |
| setDragFileName(fileName); | |
| } else if (!dragFileName && fileName) { | |
| setDragFileName(fileName); | |
| } | |
| }, | |
| [disabled, onFileDrop, resetDragState] | |
| ); | |
| } | |
| const handleDragOver = (e: React.DragEvent<HTMLElement>) => { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| if (disabled) return; | |
| if (containerRef?.current) { | |
| const rect = containerRef.current.getBoundingClientRect(); | |
| setMousePosition({ | |
| x: e.clientX - rect.left, | |
| y: e.clientY - rect.top, | |
| }); | |
| } | |
| // 파일명이 아직 설정되지 않은 경우 추출 시도 | |
| if (!dragFileName || dragFileName === "파일") { | |
| const fileName = extractFileName(e); | |
| if (fileName && fileName !== "파일") { | |
| setDragFileName(fileName); | |
| } else if (!dragFileName && fileName) { | |
| setDragFileName(fileName); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useDragAndDrop.ts` around lines 101 - 121, The handleDragOver
handler currently updates state even when the dropzone is disabled; after the
existing e.preventDefault() and e.stopPropagation() calls in handleDragOver add
an early guard that returns immediately if disabled is true to prevent updates
to mousePosition and dragFileName; specifically, inside handleDragOver (the
function that uses containerRef, setMousePosition, setDragFileName and calls
extractFileName) check the disabled flag and return before computing
rect/setMousePosition or calling extractFileName so the component does not
rerender or leave stale drag metadata when disabled.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stories/components/DatePicker/DatePicker.stories.tsx (1)
12-12: 🛠️ Refactor suggestion | 🟠 Major
Meta/StoryObj타입 축을 통일하세요.문제: Line 162에서
StoryObj<DatePickerSingleProps>를 사용해, 파일의Meta<DatePickerRangeProps>/StoryObj<DatePickerRangeProps>축과 분리되었습니다.영향: 스토리별 args 타입 검증과 Docs/Controls 추론이 일관되지 않아 잘못된 prop 조합이 누락될 수 있습니다.
권장 수정: Storybook 공식 가이드에 따라
meta를Meta<typeof DatePicker>로 변경하고, 모든 스토리를type Story = StoryObj<typeof meta>로 통일하세요. 다른 props 타입이 필요하면 별도 stories 파일을 사용하세요.권장 변경 예시
-const meta: Meta<DatePickerRangeProps> = { +const meta: Meta<typeof DatePicker> = { title: "Components/DatePicker", component: DatePicker, tags: ["autodocs"], }; export default meta; -type Story = StoryObj<DatePickerRangeProps>; +type Story = StoryObj<typeof meta>; export const Default: Story = { ... }; -export const ControlledTimePicker: StoryObj<DatePickerSingleProps> = { +export const ControlledTimePicker: Story = { render: function ControlledTimePickerStory() { ... }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stories/components/DatePicker/DatePicker.stories.tsx` at line 12, 현재 파일은 meta: Meta<DatePickerRangeProps>로 선언되어 있으나 일부 스토리에서 StoryObj<DatePickerSingleProps>를 사용해 타입 축이 일치하지 않습니다; meta를 Meta<typeof DatePicker>로 변경하고 모든 스토리 타입을 type Story = StoryObj<typeof meta>로 통일하여 meta와 각 스토리의 args/controls 추론을 일관되게 만드세요 (다른 prop 형태가 필요하면 DatePickerSingle 전용 별도 stories 파일을 만드세요).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/DatePicker/DatePicker.tsx`:
- Around line 136-139: The time source and update path are wrong in the
showTimePicker flow: replace the hour/minute computation to derive from
selectedDate ?? value ?? defaultValue instead of value alone, and stop calling
onChange directly in the showTimePicker handlers—always route date/time updates
through useDatePickerSingle's internal updater baseHandleDateClick so the
component's internal state (selectedDate) stays in sync; update references in
DatePicker.tsx (hour/minute computation and the handlers in the showTimePicker
block) to use the combined source and call baseHandleDateClick(...) rather than
onChange(...) directly.
- Line 48: Replace the English section-label comments like {/* Header */} in
DatePicker.tsx with either a Korean, why-focused explanation or remove them if
the code is self-explanatory; update all instances noted (lines near the header
and the other spots referenced) so comments follow the project rule "Write
comments in Korean" and explain intent/rationale (why) rather than restating
what the JSX/logic already shows — look for the comment occurrences around the
DatePicker component render and adjacent helper functions to apply this change.
- Around line 49-122: The DatePicker JSX duplicates header, weekday and grid
markup across modes; extract three presentational components (CalendarHeader,
WeekdayHeader, CalendarGrid) or render functions and pass mode-specific props:
for CalendarHeader provide displayYearMonth, displayText, goToPrevMonth,
goToNextMonth; for WeekdayHeader provide WEEKDAYS; for CalendarGrid provide
calendarDays, handleDateClick and any class/selection flags. Move complex
selection logic from DatePicker into a small hook if needed, replace the
duplicated blocks with these components inside DatePicker, and ensure keys and
disabled/aria/onclick behavior still derive from calendarDays, day.isDisabled,
day.date, day.isSelected, day.isInRange, day.isToday, day.isRangeStart and
day.isRangeEnd.
In `@src/stories/components/DatePicker/DatePicker.stories.tsx`:
- Around line 166-180: Consolidate the duplicated Date cloning and setting logic
into a small internal helper like applyTime(hours, minutes) that creates a new
Date from date (or new Date()), calls setHours(hours, minutes, 0, 0) and then
calls setDate(newDate); replace setToMorning and setToEvening to call
applyTime(9, 0) and applyTime(18, 30) respectively (leave setToNow as
setDate(new Date()) or call applyTime with current hours/minutes if you prefer).
---
Outside diff comments:
In `@src/stories/components/DatePicker/DatePicker.stories.tsx`:
- Line 12: 현재 파일은 meta: Meta<DatePickerRangeProps>로 선언되어 있으나 일부 스토리에서
StoryObj<DatePickerSingleProps>를 사용해 타입 축이 일치하지 않습니다; meta를 Meta<typeof
DatePicker>로 변경하고 모든 스토리 타입을 type Story = StoryObj<typeof meta>로 통일하여 meta와 각
스토리의 args/controls 추론을 일관되게 만드세요 (다른 prop 형태가 필요하면 DatePickerSingle 전용 별도
stories 파일을 만드세요).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e298a652-dc44-4152-8b5a-05ba88d5a05a
📒 Files selected for processing (2)
src/components/DatePicker/DatePicker.tsxsrc/stories/components/DatePicker/DatePicker.stories.tsx
| className | ||
| )} | ||
| > | ||
| {/* Header */} |
There was a problem hiding this comment.
주석 규칙(언어/의도) 위반입니다.
문제: 주석이 영어이고 섹션 라벨처럼 what만 설명합니다.
영향: 팀 컨벤션 불일치로 코드 리뷰/협업 일관성이 떨어집니다.
권장 수정: 한국어로 why 중심 주석으로 바꾸거나, 자명한 섹션 라벨 주석은 제거하세요.
As per coding guidelines "Write comments in Korean" and "In comments, explain the 'why' rather than the 'what' - code should explain the what".
Also applies to: 78-78, 87-87, 188-188, 190-190, 220-220, 232-232, 267-267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DatePicker/DatePicker.tsx` at line 48, Replace the English
section-label comments like {/* Header */} in DatePicker.tsx with either a
Korean, why-focused explanation or remove them if the code is self-explanatory;
update all instances noted (lines near the header and the other spots
referenced) so comments follow the project rule "Write comments in Korean" and
explain intent/rationale (why) rather than restating what the JSX/logic already
shows — look for the comment occurrences around the DatePicker component render
and adjacent helper functions to apply this change.
| <div className="mb-4 flex items-center justify-between"> | ||
| <div className="flex items-center gap-3"> | ||
| <span className="text-text-secondary text-lg font-semibold">{displayYearMonth}</span> | ||
| <div className="text-text-disabled flex items-center gap-1 text-[13px]"> | ||
| <CalendarIcon size="xsmall" /> | ||
| <span>{displayText}</span> | ||
| </div> | ||
|
|
||
| <div className="flex items-center gap-1"> | ||
| <button | ||
| type="button" | ||
| onClick={goToPrevMonth} | ||
| className="text-text-disabled hover:text-text-secondary transition-colors" | ||
| aria-label="이전 달" | ||
| > | ||
| <ChevronDownIcon size="small" className="rotate-90" /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| onClick={goToNextMonth} | ||
| className="text-text-disabled hover:text-text-secondary transition-colors" | ||
| aria-label="다음 달" | ||
| > | ||
| <ChevronDownIcon size="small" className="-rotate-90" /> | ||
| </button> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Weekday Headers */} | ||
| <div className="mb-2 grid grid-cols-7"> | ||
| {WEEKDAYS.map((day) => ( | ||
| <div | ||
| key={day} | ||
| className="text-text-secondary py-2 text-center text-[15px] font-semibold" | ||
| > | ||
| {day} | ||
| </div> | ||
| ))} | ||
| </div> | ||
|
|
||
| {/* Calendar Grid */} | ||
| <div className="grid flex-1 grid-cols-7"> | ||
| {calendarDays.map((day, index) => ( | ||
| <button | ||
| key={day.date ? day.date.toISOString() : `empty-${index}`} | ||
| type="button" | ||
| disabled={day.isDisabled || !day.date} | ||
| onClick={() => day.date && handleDateClick(day.date)} | ||
| className={cn( | ||
| "flex h-11 w-full items-center justify-center text-[15px] transition-colors", | ||
| !day.date && "cursor-default", | ||
| day.date && !day.isDisabled && "hover:bg-alpha-white-8 cursor-pointer", | ||
| day.isDisabled && "text-text-disabled cursor-not-allowed", | ||
| !day.isDisabled && !day.isSelected && "text-text-secondary", | ||
| day.isToday && !day.isSelected && !day.isInRange && "bg-alpha-white-16 rounded-sm", | ||
| day.isInRange && "bg-alpha-white-8", | ||
| day.isRangeStart && "rounded-l-sm", | ||
| day.isRangeEnd && "rounded-r-sm" | ||
| )} | ||
| > | ||
| {day.date ? ( | ||
| <span | ||
| className={cn( | ||
| "flex h-9 w-9 items-center justify-center", | ||
| day.isSelected && | ||
| "text-text-brand-default rounded-sm bg-green-500/10 font-semibold" | ||
| )} | ||
| > | ||
| {day.dayOfMonth} | ||
| </span> | ||
| ) : ( | ||
| "" | ||
| )} | ||
| </button> | ||
| ))} | ||
| <div className="flex items-center gap-1"> | ||
| <button | ||
| type="button" | ||
| onClick={goToPrevMonth} | ||
| className="text-text-disabled hover:text-text-secondary transition-colors" | ||
| aria-label="이전 달" | ||
| > | ||
| <ChevronDownIcon size="small" className="rotate-90" /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| onClick={goToNextMonth} | ||
| className="text-text-disabled hover:text-text-secondary transition-colors" | ||
| aria-label="다음 달" | ||
| > | ||
| <ChevronDownIcon size="small" className="-rotate-90" /> | ||
| </button> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
| ); | ||
|
|
||
| DatePickerRange.displayName = "DatePickerRange"; | ||
| {/* Weekday Headers */} | ||
| <div className="mb-2 grid grid-cols-7"> | ||
| {WEEKDAYS.map((day) => ( | ||
| <div key={day} className="text-text-secondary py-2 text-center text-[15px] font-semibold"> | ||
| {day} | ||
| </div> | ||
| ))} | ||
| </div> | ||
|
|
||
| const DatePickerSingle = forwardRef<HTMLDivElement, DatePickerSingleProps>( | ||
| ({ value, defaultValue, onChange, disabled, className, showTimePicker }, ref) => { | ||
| const [hour, setHour] = useState(() => (value ? value.getHours() : new Date().getHours())); | ||
| const [minute, setMinute] = useState(() => { | ||
| const mins = value ? value.getMinutes() : new Date().getMinutes(); | ||
| return Math.floor(mins / 5) * 5; | ||
| }); | ||
| {/* Calendar Grid */} | ||
| <div className="grid flex-1 grid-cols-7"> | ||
| {calendarDays.map((day, index) => ( | ||
| <button | ||
| key={day.date ? day.date.toISOString() : `empty-${index}`} | ||
| type="button" | ||
| disabled={day.isDisabled || !day.date} | ||
| onClick={() => day.date && handleDateClick(day.date)} | ||
| className={cn( | ||
| "flex h-11 w-full items-center justify-center text-[15px] transition-colors", | ||
| !day.date && "cursor-default", | ||
| day.date && !day.isDisabled && "hover:bg-alpha-white-8 cursor-pointer", | ||
| day.isDisabled && "text-text-disabled cursor-not-allowed", | ||
| !day.isDisabled && !day.isSelected && "text-text-secondary", | ||
| day.isToday && !day.isSelected && !day.isInRange && "bg-alpha-white-16 rounded-sm", | ||
| day.isInRange && "bg-alpha-white-8", | ||
| day.isRangeStart && "rounded-l-sm", | ||
| day.isRangeEnd && "rounded-r-sm" | ||
| )} | ||
| > | ||
| {day.date ? ( | ||
| <span | ||
| className={cn( | ||
| "flex h-9 w-9 items-center justify-center", | ||
| day.isSelected && | ||
| "text-text-brand-default rounded-sm bg-green-500/10 font-semibold" | ||
| )} | ||
| > | ||
| {day.dayOfMonth} | ||
| </span> | ||
| ) : ( | ||
| "" | ||
| )} | ||
| </button> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
캘린더 렌더링 중복과 과도한 중첩을 분리하세요.
문제: range/single 모드에서 헤더·요일·그리드 JSX가 거의 동일하게 중복되어 있고 중첩 깊이도 큽니다.
영향: 한쪽만 수정되는 회귀가 쉽게 발생하고, 유지보수 비용이 계속 증가합니다.
권장 수정: CalendarHeader, WeekdayHeader, CalendarGrid를 공통 컴포넌트(또는 렌더 함수)로 추출하고, 모드별 차이(클래스/클릭 핸들러/선택 상태)만 props로 주입하세요.
As per coding guidelines "Keep nesting depth to a maximum of 2-3 levels" and "Extract complex logic into custom hooks to keep components clean".
Also applies to: 191-264
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DatePicker/DatePicker.tsx` around lines 49 - 122, The
DatePicker JSX duplicates header, weekday and grid markup across modes; extract
three presentational components (CalendarHeader, WeekdayHeader, CalendarGrid) or
render functions and pass mode-specific props: for CalendarHeader provide
displayYearMonth, displayText, goToPrevMonth, goToNextMonth; for WeekdayHeader
provide WEEKDAYS; for CalendarGrid provide calendarDays, handleDateClick and any
class/selection flags. Move complex selection logic from DatePicker into a small
hook if needed, replace the duplicated blocks with these components inside
DatePicker, and ensure keys and disabled/aria/onclick behavior still derive from
calendarDays, day.isDisabled, day.date, day.isSelected, day.isInRange,
day.isToday, day.isRangeStart and day.isRangeEnd.
| const hour = value ? value.getHours() : new Date().getHours(); | ||
| const minute = value | ||
| ? Math.floor(value.getMinutes() / 5) * 5 | ||
| : Math.floor(new Date().getMinutes() / 5) * 5; |
There was a problem hiding this comment.
showTimePicker 경로에서 uncontrolled 동작이 깨집니다.
문제: Line 136-139에서 시간 소스를 value만으로 계산하고, Line 151-175에서 변경 반영을 onChange?.(...)로만 처리해 useDatePickerSingle의 내부 상태 경로(baseHandleDateClick)를 우회합니다.
영향: defaultValue 기반 사용 시 날짜/시간 선택이 UI에 반영되지 않거나, 현재 시간으로 덮여 잘못 저장될 수 있습니다.
권장 수정: 시간 소스를 selectedDate ?? value ?? defaultValue 기준으로 계산하고, 날짜/시간 변경은 모두 baseHandleDateClick 경유로 일원화하세요.
🔧 제안 수정
function DatePickerSingle({
value,
defaultValue,
onChange,
@@
const {
displayYearMonth,
displayText,
calendarDays,
selectedDate,
@@
} = useDatePickerSingle({ value, defaultValue, onChange });
- const hour = value ? value.getHours() : new Date().getHours();
- const minute = value
- ? Math.floor(value.getMinutes() / 5) * 5
- : Math.floor(new Date().getMinutes() / 5) * 5;
+ const timeSource = selectedDate ?? value ?? defaultValue ?? new Date();
+ const hour = timeSource.getHours();
+ const minute = Math.floor(timeSource.getMinutes() / 5) * 5;
+
+ const commitDateChange = (nextDate: Date) => {
+ baseHandleDateClick(nextDate);
+ };
const handleDateClick = (date: Date) => {
- if (showTimePicker) {
- const newDate = new Date(date);
- newDate.setHours(hour, minute, 0, 0);
- onChange?.(newDate);
- } else {
+ if (!showTimePicker) {
baseHandleDateClick(date);
+ return;
}
+ const newDate = new Date(date);
+ newDate.setHours(hour, minute, 0, 0);
+ commitDateChange(newDate);
};
const handleHourChange = (newHour: number) => {
if (selectedDate) {
const newDate = new Date(selectedDate);
newDate.setHours(newHour, minute, 0, 0);
- onChange?.(newDate);
+ commitDateChange(newDate);
}
};
const handleMinuteChange = (newMinute: number) => {
if (selectedDate) {
const newDate = new Date(selectedDate);
newDate.setHours(hour, newMinute, 0, 0);
- onChange?.(newDate);
+ commitDateChange(newDate);
}
};Also applies to: 151-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DatePicker/DatePicker.tsx` around lines 136 - 139, The time
source and update path are wrong in the showTimePicker flow: replace the
hour/minute computation to derive from selectedDate ?? value ?? defaultValue
instead of value alone, and stop calling onChange directly in the showTimePicker
handlers—always route date/time updates through useDatePickerSingle's internal
updater baseHandleDateClick so the component's internal state (selectedDate)
stays in sync; update references in DatePicker.tsx (hour/minute computation and
the handlers in the showTimePicker block) to use the combined source and call
baseHandleDateClick(...) rather than onChange(...) directly.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/DatePicker/DatePicker.tsx`:
- Around line 146-175: The time panel ignores user changes when selectedDate is
null because hour/minute are derived only from selectedDate ?? defaultValue and
handleHourChange/handleMinuteChange no-op if selectedDate is falsy; change to
hold draft time state (e.g., draftHour/draftMinute initialized from timeSource)
and update those in handleHourChange/handleMinuteChange so the UI reflects user
picks even when selectedDate is null, then in handleDateClick merge
draftHour/draftMinute into the new Date before calling baseHandleDateClick;
alternatively, if you choose not to support draft time, disable the time picker
UI when selectedDate is null (controlled by showTimePicker and selectedDate) so
users cannot change time before selecting a date.
- Around line 40-46: The root wrapper only applies visual disabled styles
(pointer-events-none, opacity-50) but does not set native disabled on
interactive controls, so keyboard users can still operate the picker; update all
interactive <button> elements (month navigation buttons, the day/date cell
buttons rendered in the calendar grid, weekday header buttons if any, and action
buttons such as "Today"/"Clear") to include disabled={disabled ||
<existingCondition>} so they are actually non-interactive when the component
prop disabled is true; keep the root visual styles but ensure every button in
the DatePicker component checks the disabled prop (e.g., the prev/next month
buttons, the map that renders date buttons, and any footer action buttons) and
short-circuit handlers if necessary to rely on the native disabled behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1956f9c5-ad4d-48b0-b4fe-b71c661094a3
📒 Files selected for processing (1)
src/components/DatePicker/DatePicker.tsx
| <div | ||
| ref={ref} | ||
| className={cn( | ||
| "border-border-subtle flex min-h-[345px] w-[356px] flex-col rounded-md border p-4", | ||
| disabled && "pointer-events-none opacity-50", | ||
| className | ||
| )} |
There was a problem hiding this comment.
disabled가 시각적으로만 적용돼 실제 조작은 계속 가능합니다.
문제: Line 40-46과 Line 179-186은 루트에 pointer-events-none만 주고 있고, 월 이동/날짜 버튼은 disabled를 받지 않습니다.
영향: Tab/Enter/Space로 월 이동과 날짜 선택이 계속 가능해서 disabled 상태가 깨집니다. 키보드 사용자 기준으로는 비활성 컴포넌트가 여전히 동작합니다.
권장 수정: 각 button에 disabled={disabled || ...}를 직접 연결하세요. 루트의 시각적 처리(opacity-50)는 그대로 두되, 실제 비활성화는 네이티브 속성으로 보장하는 편이 안전합니다.
수정 예시
<button
type="button"
onClick={goToPrevMonth}
+ disabled={disabled}
className="text-text-disabled hover:text-text-secondary transition-colors"
aria-label="이전 달"
>
- disabled={day.isDisabled || !day.date}
+ disabled={disabled || day.isDisabled || !day.date}As per coding guidelines "src/components/**: 접근성(a11y) 속성이 적절히 사용되었는지 확인".
Also applies to: 59-74, 90-120, 179-186, 202-217, 236-264
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DatePicker/DatePicker.tsx` around lines 40 - 46, The root
wrapper only applies visual disabled styles (pointer-events-none, opacity-50)
but does not set native disabled on interactive controls, so keyboard users can
still operate the picker; update all interactive <button> elements (month
navigation buttons, the day/date cell buttons rendered in the calendar grid,
weekday header buttons if any, and action buttons such as "Today"/"Clear") to
include disabled={disabled || <existingCondition>} so they are actually
non-interactive when the component prop disabled is true; keep the root visual
styles but ensure every button in the DatePicker component checks the disabled
prop (e.g., the prev/next month buttons, the map that renders date buttons, and
any footer action buttons) and short-circuit handlers if necessary to rely on
the native disabled behavior.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/DatePicker/DatePicker.tsx (1)
39-45:⚠️ Potential issue | 🟠 Major
disabled가 아직 네이티브 비활성화로 연결되지 않습니다.문제: 루트에는
pointer-events-none만 적용되고, 이 파일에서 직접 렌더하는 월 이동 버튼은disabled가 없으며 날짜 버튼도 부모disabled를 반영하지 않습니다.영향: 마우스는 막혀 보여도 Tab/Enter/Space로 월 이동과 날짜 선택이 계속 가능해
disabled상태가 실제 동작과 어긋납니다.권장 수정: 월 이동 버튼에는
disabled={disabled}를, 날짜 버튼에는disabled={disabled || day.isDisabled || !day.date}를 직접 연결하세요. 시각 처리용opacity-50는 그대로 두고, 비활성화 자체는 네이티브 속성으로 보장하는 편이 안전합니다.As per coding guidelines "
src/components/**: 접근성(a11y) 속성이 적절히 사용되었는지 확인".Also applies to: 59-74, 87-95, 178-185, 201-240
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DatePicker/DatePicker.tsx` around lines 39 - 45, The root div only applies pointer-events-none visually but doesn't set native disabled state—update the month navigation buttons (the prev/next month button elements rendered in DatePicker) to include disabled={disabled}, and update each date button where days are rendered (the date cell/button—referencing the day variable and its properties) to set disabled={disabled || day.isDisabled || !day.date}; keep the existing opacity-50 class for visual dimming but rely on the native disabled attribute to block keyboard activation as well. Ensure you apply these changes where month nav buttons and the date buttons are created (the DatePicker component's month navigation and day rendering code paths).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/DatePicker/DatePicker.tsx`:
- Around line 39-45: The root div only applies pointer-events-none visually but
doesn't set native disabled state—update the month navigation buttons (the
prev/next month button elements rendered in DatePicker) to include
disabled={disabled}, and update each date button where days are rendered (the
date cell/button—referencing the day variable and its properties) to set
disabled={disabled || day.isDisabled || !day.date}; keep the existing opacity-50
class for visual dimming but rely on the native disabled attribute to block
keyboard activation as well. Ensure you apply these changes where month nav
buttons and the date buttons are created (the DatePicker component's month
navigation and day rendering code paths).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4a12d0a3-b052-4d3c-86f2-9c98cd058134
📒 Files selected for processing (1)
src/components/DatePicker/DatePicker.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings.json:
- Around line 3-12: This change wrongly mixes IDE/tooling config with the
functional PR — revert the edits to the "mcpServers" block in
.claude/settings.json from this PR and open a separate PR that adds/changes the
"mcpServers" settings (keys: "mcpServers", "playwright", "context7"); in that
separate PR remove the automatic accept flag by dropping "-y" from the "args"
array for both "playwright" and "context7" commands and pin explicit versions
instead of "@latest" (e.g. use "@playwright/mcp@0.0.68" and a specific
"@upstash/context7-mcp@<version>") so the tool versions are fixed and user
consent is required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0dd934b1-58bf-4952-a1ef-68410d5d35a7
📒 Files selected for processing (2)
.claude/settings.json.gitignore
| "playwright@claude-plugins-official": true, | ||
| "context7@claude-plugins-official": true | ||
| }, | ||
| "mcpServers": { | ||
| "playwright": { | ||
| "command": "npx", | ||
| "args": ["-y", "@playwright/mcp@latest"] | ||
| }, | ||
| "context7": { "command": "npx", "args": ["-y", "@upstash/context7-mcp@latest"] } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify npm packages exist and check their latest versions
echo "=== Checking `@playwright/mcp` ==="
npm view `@playwright/mcp` version 2>/dev/null || echo "Package not found"
echo "=== Checking `@upstash/context7-mcp` ==="
npm view `@upstash/context7-mcp` version 2>/dev/null || echo "Package not found"Repository: dnd-side-project/dnd-14th-9-frontend
Length of output: 164
IDE 설정을 별도 PR로 분리하세요
.claude/settings.json의 MCP 서버 설정은 React Compiler 기반 memoization 제거 목표와 무관합니다. 도구 설정 변경은 별도 PR로 분리하면 코드 이력 관리와 검토가 명확합니다.
추가 개선사항:
npx -y플래그는 패키지 설치를 자동 승인하므로 보안 위험 증가 → 사용자 확인 프롬프트 추가 필요@latest버전 고정 없이 사용하면 팀원 간 도구 버전 불일치 가능 → 특정 버전 명시 권장 (예:@playwright/mcp@0.0.68)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings.json around lines 3 - 12, This change wrongly mixes
IDE/tooling config with the functional PR — revert the edits to the "mcpServers"
block in .claude/settings.json from this PR and open a separate PR that
adds/changes the "mcpServers" settings (keys: "mcpServers", "playwright",
"context7"); in that separate PR remove the automatic accept flag by dropping
"-y" from the "args" array for both "playwright" and "context7" commands and pin
explicit versions instead of "@latest" (e.g. use "@playwright/mcp@0.0.68" and a
specific "@upstash/context7-mcp@<version>") so the tool versions are fixed and
user consent is required.
tnemnorivnelee
left a comment
There was a problem hiding this comment.
확인이 늦었습니다 죄송합니다ㅠ
작업 내용
Summary by CodeRabbit
Refactor
Documentation
참고 사항
연관 이슈
CodeRabbit 운영 메모
@coderabbitai review@coderabbitai full review비교 시 큰 차이 없음
메모이제이션 제거 (로비 테스트)

메모이제이션 있을 경우 (로비 테스트)
