Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. Walkthrough
Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/result/[id]/page.tsx (2)
131-131: 의도적으로 미사용 파라미터는_접두사 컨벤션 사용 권장
hot파라미터를 사용하지 않지만 시그니처 유지를 위해 남겨두었다면,_hot으로 명명하는 것이 TypeScript/ESLint의no-unused-vars규칙 위반을 방지하는 관용적인 방식입니다.♻️ 수정 제안
- hot: boolean | undefined, // hot 파라미터를 안 쓰게 되더라도 시그니처 유지를 위해 둠 + _hot: boolean | undefined, // 시그니처 유지용 — 의도적 미사용🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/result/`[id]/page.tsx at line 131, Rename the intentionally unused parameter hot to _hot in the function/component signature where it's declared (look for the parameter list containing "hot: boolean | undefined" in the page component in app/result/[id]/page.tsx) so it follows the unused-variable convention and avoids TypeScript/ESLint no-unused-vars warnings; update any internal references (if any) accordingly and run lint/type checks to confirm no further unused-var errors remain.
224-227:category변수를map외부로 호이스팅 권장
meetingData는 이터레이션 간에 변하지 않으므로category를 매 순회마다 재계산할 필요가 없습니다.map()호출 전에 한 번만 추출하는 것이 명확하고 효율적입니다.♻️ 수정 제안
+ const category = + meetingData?.data?.purposes?.[meetingData.data.purposes.length - 1]; {locationResults.map((result) => { - const category = - meetingData?.data?.purposes?.[meetingData.data.purposes.length - 1]; const categoryText = getCategoryText(category, result.hot, result.id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/result/`[id]/page.tsx around lines 224 - 227, Hoist the computation of category out of the locationResults.map callback since meetingData does not change across iterations: compute const category = meetingData?.data?.purposes?.[meetingData.data.purposes.length - 1] once before calling locationResults.map and then pass that precomputed category into the map where getCategoryText(category, result.hot, result.id) is invoked; update any references inside the map to use the hoisted category to avoid repeated recalculation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/result/`[id]/page.tsx:
- Around line 27-41: Replace the useEffect + setLocalCategory pattern by
initializing localCategory via useState's lazy initializer that reads
localStorage only on the client: remove the useEffect block that references
useEffect, setLocalCategory and id, and change the useState for localCategory to
a lazy initializer that checks typeof window !== 'undefined' and reads
localStorage.getItem(`meeting_${id}_category`) (falling back to ''), mirroring
the myNickname pattern to avoid setState-in-effect ESLint errors.
---
Nitpick comments:
In `@app/result/`[id]/page.tsx:
- Line 131: Rename the intentionally unused parameter hot to _hot in the
function/component signature where it's declared (look for the parameter list
containing "hot: boolean | undefined" in the page component in
app/result/[id]/page.tsx) so it follows the unused-variable convention and
avoids TypeScript/ESLint no-unused-vars warnings; update any internal references
(if any) accordingly and run lint/type checks to confirm no further unused-var
errors remain.
- Around line 224-227: Hoist the computation of category out of the
locationResults.map callback since meetingData does not change across
iterations: compute const category =
meetingData?.data?.purposes?.[meetingData.data.purposes.length - 1] once before
calling locationResults.map and then pass that precomputed category into the map
where getCategoryText(category, result.hot, result.id) is invoked; update any
references inside the map to use the hoisted category to avoid repeated
recalculation.
| // 로컬스토리지에서 가져온 카테고리 값을 저장할 State | ||
| const [localCategory, setLocalCategory] = useState<string>(''); | ||
|
|
||
| const { data: midpointData, isLoading, isError } = useMidpoint(id); | ||
| const { data: meetingData } = useCheckMeeting(id); | ||
|
|
||
| // 컴포넌트 마운트 시 로컬스토리지 값 가져오기 (Hydration 에러 방지) | ||
| useEffect(() => { | ||
| if (typeof window !== 'undefined' && id) { | ||
| const storedCategory = localStorage.getItem(`meeting_${id}_category`); | ||
| if (storedCategory) { | ||
| setLocalCategory(storedCategory); | ||
| } | ||
| } | ||
| }, [id]); |
There was a problem hiding this comment.
CI 파이프라인 실패 원인 수정 필요 — useEffect 내부의 setState 직접 호출
react-hooks/set-state-in-effect ESLint 오류로 인해 CI가 실패하고 있습니다. 같은 파일의 myNickname (Line 22–25)이 이미 useState lazy initializer + typeof window 가드 패턴으로 동일한 목적을 달성하고 있으므로, localCategory도 동일하게 리팩토링하고 useEffect를 제거하면 됩니다.
🐛 수정 제안
- // 로컬스토리지에서 가져온 카테고리 값을 저장할 State
- const [localCategory, setLocalCategory] = useState<string>('');
-
...
-
- // 컴포넌트 마운트 시 로컬스토리지 값 가져오기 (Hydration 에러 방지)
- useEffect(() => {
- if (typeof window !== 'undefined' && id) {
- const storedCategory = localStorage.getItem(`meeting_${id}_category`);
- if (storedCategory) {
- setLocalCategory(storedCategory);
- }
- }
- }, [id]);
+ // 로컬스토리지에서 가져온 카테고리 값을 저장할 State (myNickname과 동일한 lazy initializer 패턴)
+ const [localCategory] = useState<string>(() => {
+ if (typeof window === 'undefined') return '';
+ return localStorage.getItem(`meeting_${id}_category`) || '';
+ });🧰 Tools
🪛 ESLint
[error] 38-38: Error: Calling setState synchronously within an effect can trigger cascading renders
Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following:
- Update external systems with the latest state from React.
- Subscribe for updates from some external system, calling setState in a callback function when external state changes.
Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect).
/home/jailuser/git/app/result/[id]/page.tsx:38:9
36 | const storedCategory = localStorage.getItem(meeting_${id}_category);
37 | if (storedCategory) {
38 | setLocalCategory(storedCategory);
| ^^^^^^^^^^^^^^^^ Avoid calling setState() directly within an effect
39 | }
40 | }
41 | }, [id]);
(react-hooks/set-state-in-effect)
🪛 GitHub Actions: CI
[error] 38-38: React: Calling setState synchronously within an effect. Avoid calling setState() directly within an effect (react-hooks/set-state-in-effect).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/result/`[id]/page.tsx around lines 27 - 41, Replace the useEffect +
setLocalCategory pattern by initializing localCategory via useState's lazy
initializer that reads localStorage only on the client: remove the useEffect
block that references useEffect, setLocalCategory and id, and change the
useState for localCategory to a lazy initializer that checks typeof window !==
'undefined' and reads localStorage.getItem(`meeting_${id}_category`) (falling
back to ''), mirroring the myNickname pattern to avoid setState-in-effect ESLint
errors.
1 participant
🚀 fix: 모임 목적 하드코딩 제거 이후 로컬스토리지에서 카테고리 추출
📝 변경사항
✅ 체크리스트
📸 스크린샷
💬 리뷰어 전달사항
Summary by CodeRabbit
릴리스 노트
버그 수정
개선 사항