refactor: 마지막 아이템 스크롤 전용 로직을 공용 쿼리 액션 훅으로 통합 - #381
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough전용 Changes토너먼트 마지막 항목 스크롤
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Discord 스레드 연동용 메타데이터입니다. discord-pr-bot 워크플로가 자동 생성하며, 수정·삭제하면 PR 과 Discord 알림 연동이 끊깁니다. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![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
🤖 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
`@apps/web/src/app/tournament/`[id]/create/_components/TournamentCreateClient.tsx:
- Around line 33-35: Update the TournamentCreateClient flow using useQueryAction
so the scroll-to-last action remains observable by the Carousel after the query
parameter is removed. Either explicitly reset the action with setIsActive(false)
at the appropriate time or preserve scrollToLast as query-driven state until the
new basket is added, ensuring query-only navigation scrolls to the last basket.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4fff47d1-784a-462d-828b-f7321b6c6fcb
📒 Files selected for processing (7)
apps/web/src/app/tournament/[id]/create/_components/TournamentCreateClient.tsxapps/web/src/app/tournament/[id]/create/_components/tournament-item-basket/TournamentItemBasketCarousel.tsxapps/web/src/app/tournament/[id]/create/_hooks/useBasketCarousel.tsapps/web/src/app/tournament/[id]/create/_hooks/useScrollToLast.tsapps/web/src/app/tournament/[id]/create/by-wish/_hooks/usePostTournamentItemsByWish.tsapps/web/src/consts/queryAction.tsapps/web/src/hooks/useNotificationSSE.ts
💤 Files with no reviewable changes (2)
- apps/web/src/app/tournament/[id]/create/_hooks/useScrollToLast.ts
- apps/web/src/app/tournament/[id]/create/_hooks/useBasketCarousel.ts
| const { isActive: scrollToLast } = useQueryAction({ | ||
| action: QUERY_ACTION.VALUE.SCROLL_TO_LAST, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'TournamentCreateClient\.tsx|useQueryAction\.ts|TournamentItemBasketCarousel\.(tsx|ts)$' . | sed 's#^\./##'
echo "== outline candidate files =="
for f in $(fd 'TournamentCreateClient\.tsx|useQueryAction\.ts|TournamentItemBasketCarousel\.(tsx|ts)$' .); do
echo "--- $f ---"
wc -l "$f"
ast-grep outline "$f" || true
done
echo "== relevant snippets =="
for f in $(fd 'TournamentCreateClient\.tsx|useQueryAction\.ts|TournamentItemBasketCarousel\.(tsx|ts)$' .); do
echo "--- $f lines 1-240 ---"
sed -n '1,240p' "$f" | nl -ba
done
echo "== search query action usages =="
rg -n "useQueryAction|QUERY_ACTION\.VALUE\.SCROLL_TO_LAST|scrollToLast|scroll-to-last|prevItemCountRef|useQuery" apps/web/src -SRepository: TeamPiKi/client
Length of output: 1697
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=(
"apps/web/src/app/tournament/[id]/create/_components/TournamentCreateClient.tsx"
"apps/web/src/app/tournament/[id]/create/_components/tournament-item-basket/TournamentItemBasketCarousel.tsx"
"apps/web/src/hooks/useQueryAction.ts"
)
for f in "${files[@]}"; do
echo "--- $f line count ---"
wc -l "$f"
echo "--- $f content ---"
awk '{printf "%6d\t%s\n", NR, $0}' "$f"
done
echo "== targeted searches =="
rg -n "useQueryAction|QUERY_ACTION\.VALUE\.SCROLL_TO_LAST|scrollToLast|scroll-to-last|prevItemCountRef|useQuery" apps/web/src -S || true
echo "== package/runtime availability =="
node --version 2>/dev/null || true
python3 --version 2>/dev/null || trueRepository: TeamPiKi/client
Length of output: 29130
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('apps/web/src/hooks/useQueryAction.ts')
src = p.read_text()
print('useQueryAction uses useState initializer:', 'useState(() => searchParams.get(paramKey) === action)' in src)
print('useQueryAction effect replaces URL immediately:', '.router.replace(clearPath ?? pathname, { scroll: false })' in src)
print('useQueryAction effect does not call setIsActive:', 'setIsActive(' in src and 'onAction?.();\n router.replace' in src)
t = Path('apps/web/src/app/tournament/[id]/create/_components/tournament-item-basket/TournamentItemBasketCarousel.tsx').read_text()
print('carousel reads scrollToLast only into prevItemCountRef initializer:', t.count('scrollToLast') == 1 and 'prevItemCountRef.current = scrollToLast' in t)
print('carousel scroll effect compares items.length with prev count:', 'items.length > prevItemCountRef.current' in t)
client = Path('apps/web/src/app/tournament/[id]/create/_components/TournamentCreateClient.tsx').read_text()
print('client forwards isActive to carousel:', 'scrollToLast={scrollToLast}' in client)
p = Path('apps/web/src/hooks/useNotificationSSE.ts').read_text()
print('notification hook contains SCROLL_TO_LAST_QUERY constant:', 'SCROLL_TO_LAST_QUERY' in p)
lines = p.splitlines()
for i,l in enumerate(lines, start=1):
if 'SCROLL_TO_LAST_QUERY' in l or 'TOURNAMENT_CREATE(tournamentId)' in l:
start=max(1,i-6); end=min(len(lines),i+10)
print(f'-- useNotificationSSE.ts lines {start}-{end} --')
for j in range(start,end+1):
print(f'{j:5d}\t{lines[j-1]}')
PY
python3 - <<'PY'
from pathlib import Path
# Model Next.js route reuse behavior relevant to the concern:
# - app/[id]/create/page.tsx is a route, not an app-level layout; same route with same app dir
# causes the client component's state initialized from searchParams to persist; URL changes update searchParams hooks.
# - useQueryAction's first-effect removes the query while isActive stays initialized true.
# - Carousel receives true once; ref stores 0 as first prevItemCountRef.current, then scroll effect consumes items.length > 0 once.
# - A later same-route query-only addition sets searchParams to include action, but the hook's state is already true and the
# removal effect immediately clears it before the user has a chance to observe scrolling.
print('concern_model:')
print('1. first URL without action: isActive false, prevItemCountRef initial = items.length, no scroll when item appended.')
print('2. user appends item: isActive false, scroll condition: appended 1 > previous N? false; no scroll.')
print('3. notification URL with action while component mounted: inactive state remains true already; query-removing effect triggers immediately; active indicator disappears before carousel can observe second append.')
PYRepository: TeamPiKi/client
Length of output: 3026
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,90p' apps/web/src/app/tournament/\[id\]/create/by-wish/_hooks/usePostTournamentItemsByWish.ts
sed -n '1,95p' apps/web/src/hooks/usePostWishLink.ts
python3 - <<'PY'
from pathlib import Path
for p in [
Path('apps/web/src/app/tournament/[id]/create/by-wish/_hooks/usePostTournamentItemsByWish.ts'),
Path('apps/web/src/hooks/usePostWishLink.ts'),
]:
print(f'--- {p} ---')
text = p.read_text()
for needle in ['scroll-to-last', 'SCROLL_TO_LAST', 'TOURNAMENT_CREATE', 'router.push', 'queryClient']:
print(f'{needle}:', text.find(needle))
PYRepository: TeamPiKi/client
Length of output: 3156
동일한 생성 라우트에서 딥링크 스크롤을 반영하도록 해 주세요.
useQueryAction이 ?action=scroll-to-last 쿼리를 감지한 뒤 즉시 URL에서 제거하더라도 기존 생성 페이지에는 prevItemCountRef가 이미 최신 아이템 수로 초기화되어 있어 새 바구니가 추가되더라도 마지막 바구니로 스크롤되지 않습니다. 생성 화면도 같은 라우트에서 query-only navigation을 지원하므로 setIsActive(false)를 호출하거나, action 감지 시 scrollToLast를 query 상태로 관리해 Carousel에서 실제 스크롤을 처리하도록 바꿔주세요.
🤖 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
`@apps/web/src/app/tournament/`[id]/create/_components/TournamentCreateClient.tsx
around lines 33 - 35, Update the TournamentCreateClient flow using
useQueryAction so the scroll-to-last action remains observable by the Carousel
after the query parameter is removed. Either explicitly reset the action with
setIsActive(false) at the appropriate time or preserve scrollToLast as
query-driven state until the new basket is added, ensuring query-only navigation
scrolls to the last basket.
There was a problem hiding this comment.
확인해봤는데 useQueryAction이 useState lazy initializer로 마운트 시점에 쿼리를 스냅샷한 뒤 제거하기 때문에, 쿼리가 URL에서 사라져도 scrollToLast는 true로 유지되어 Carousel에서 계속 관찰 가능해요. Carousel도 이 값을 prevItemCountRef 초기값(마운트 1회)에서만 소비하고 이후엔 아이템 카운트 증가로만 스크롤하므로 리셋이 없어도 부작용이 없고, 리팩토링 전 useScrollToLast와 동작이 동일합니다
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
hook=$(fd -a '^useQueryAction\.ts$' apps/web/src/hooks | head -n 1)
carousel=$(fd -a '^TournamentItemBasketCarousel\.tsx$' apps/web/src/app/tournament | head -n 1)
printf '%s\n' "== useQueryAction outline =="
ast-grep outline "$hook" --view expanded
printf '%s\n' "== useQueryAction =="
nl -ba "$hook"
printf '%s\n' "== carousel outline =="
ast-grep outline "$carousel" --view expanded
printf '%s\n' "== carousel relevant state/effects =="
rg -n -C 8 'scrollToLast|prevItemCountRef|useEffect|scrollTo' "$carousel"
printf '%s\n' "== all useQueryAction call sites and implementations =="
rg -n -C 3 '\buseQueryAction\s*\(' apps/web/src --glob '*.{ts,tsx}'Length of output: 1246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
hook=$(fd -a '^useQueryAction\.ts$' apps/web/src/hooks | head -n 1)
carousel=$(fd -a '^TournamentItemBasketCarousel\.tsx$' apps/web/src/app/tournament | head -n 1)
echo '== useQueryAction source =='
awk '{printf "%6d\t%s\n", NR, $0}' "$hook"
echo '== carousel source: relevant lifecycle =='
awk '
/scrollToLast|prevItemCountRef|useEffect|scrollTo/ { start=NR-10; if (start<1) start=1; end=NR+16 }
NR>=start && NR<=end { lines[NR]=$0 }
END { for (i=1; i<=NR; i++) if (i in lines) printf "%6d\t%s\n", i, lines[i] }
' "$carousel"
echo '== query-action callers (context) =='
rg -n -C 4 '\buseQueryAction\s*\(' apps/web/src --glob '*.{ts,tsx}' || trueLength of output: 11131
@kanghaeun, 확인 결과 최초 마운트 경로에 대한 설명은 맞습니다. 다만 동일하게 마운트된 생성 화면에서의 query-only navigation 문제는 남아 있습니다.
useQueryAction.tsLine 38의 lazy initializer는 첫 렌더에서만 실행됩니다.- 이후
searchParams에action=scroll-to-last가 추가되면 Line 40-45의 effect는 URL만 제거하며setIsActive(true)를 호출하지 않습니다. - 따라서 기존
isActive가false인 상태에서는scrollToLast가 계속false입니다. - 캐러셀도 Line 45에서
scrollToLast를 ref 초기값으로 한 번만 사용하고, Line 50-63의 effect는scrollToLast변경을 관찰하지 않습니다.
즉, 새로 진입하여 마운트되는 흐름에서는 리팩터링 전과 동일하지만, 코멘트에서 언급한 것처럼 같은 생성 라우트가 유지된 채 action 쿼리만 추가될 수 있다면 마지막 바구니 스크롤은 트리거되지 않습니다. 해당 navigation이 실제로 발생하지 않는다는 보장이 있다면 알려주세요.
There was a problem hiding this comment.
맞아요 SSE 알림 토스트의 바로가기 로 이미 머물러 있는 create 화면에 진입하면 말씀하신 query-only navigation이 발생할 수 있습니다 (보장 없음). 다만 그 경로는 리팩터링 전에도 스크롤이 트리거되지 않던 경로예요 query-only 진입에서도 스크롤이 되게 만드는 건 기존에 없던 동작의 신규 개선이라 별도 이슈로 다루겠습니다.
There was a problem hiding this comment.
@kanghaeun, understood. Since query-only navigation did not trigger this behavior before the refactor, I agree it is out of scope for this PR and should not block it.
Would you like me to create a follow-up GitHub issue for supporting action=scroll-to-last on same-route query-only navigation from SSE notification deep links?
2 participants
작업 요약
?scrollToLast=true) + 전용 훅(useScrollToLast)을 프로젝트에 이미 있는 쿼리 기반 1회성 액션 공용 훅useQueryAction(?action=<value>)으로 통합작업 상세 내용
마지막 아이템 스크롤 기능만을 위한 전용 쿼리 파라미터와 훅이 별도로 존재했으나, 프로젝트에 이미 동일한 목적(쿼리 기반 1회성 액션 처리)의 공용 훅
useQueryAction이 있어 중복 구조였습니다. 이를 해결하기 위해 전용 로직을 공용 훅으로 통합합니다.공용 액션으로 통합
consts/queryAction.ts에SCROLL_TO_LAST액션 값 추가?action=scroll-to-last로 변경TournamentCreateClient에서useScrollToLast→useQueryAction으로 교체scrollToLast만 첫 렌더 seed로 사용하므로onScrolled정리 로직 제거정리
useScrollToLast.ts삭제useBasketCarousel.ts함께 삭제연관 이슈
scrollToLast쿼리를useQueryAction으로 대체 #372Summary by CodeRabbit