Skip to content

refactor: 응원톡 UX 개선 및 코드 정리#563

Open
seongminn wants to merge 6 commits into
wip-v2from
cheer-talk-ux
Open

refactor: 응원톡 UX 개선 및 코드 정리#563
seongminn wants to merge 6 commits into
wip-v2from
cheer-talk-ux

Conversation

@seongminn
Copy link
Copy Markdown
Member

✅ 작업 내용

  • 페이지 진입 시 스크롤 하단으로 고정
  • 새로운 응원톡 표시
  • nextjs best practices skills 추가

♾️ 기타

  • 추가로 필요한 작업 내용

@seongminn seongminn requested a review from sungwonnoh June 7, 2026 11:47
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 7, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
spectator Ready Ready Preview, Comment Jun 7, 2026 11:49am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
manager Skipped Skipped Jun 7, 2026 11:49am

@vercel vercel Bot temporarily deployed to Preview – manager June 7, 2026 11:48 Inactive
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces Next.js best practices documentation and refactors the spectator app's cheer-talk features by modularizing chat scroll, socket connection, and notification logic into dedicated hooks and components. It also replaces the next/font loading of the Pretendard font with a global CSS @import. The code review feedback highlights a critical violation of the Rules of Hooks in useGameCheerTalkSocket.ts due to an early return before a hook call. Additionally, using @import in globals.css for font loading is identified as a render-blocking anti-pattern that contradicts the newly added font optimization guidelines. Finally, replacing useLayoutEffect with useEffect in useChatScroll.ts is recommended to avoid Next.js SSR warnings.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +16 to +19
if (!socketUrl) {
console.warn('Socket URL is not defined. Cheer Talk socket will not connect.');
return items;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

React Hook 규칙(Rules of Hooks)에 따르면, 모든 Hook은 조건문이나 조기 반환(early return) 뒤에 호출되어서는 안 됩니다. 현재 코드에서는 socketUrl이 없을 경우 18번 라인에서 조기 반환을 하고 있어, 그 아래에 있는 useSocket Hook 호출이 생략될 수 있습니다. 이는 런타임 에러를 유발할 수 있으므로 조기 반환을 제거하고 useSocket을 항상 호출하도록 수정해야 합니다.

  if (!socketUrl) {
    console.warn('Socket URL is not defined. Cheer Talk socket will not connect.');
  }

@@ -1,3 +1,5 @@
@import url('https://cdn.jsdelivr.net/gh/orioncactus/pretendard@v1.3.9/dist/web/variable/pretendardvariable-dynamic-subset.min.css');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

CSS에서 @import를 사용하여 외부 폰트를 로드하는 것은 렌더링을 차단(render-blocking)하고 네트워크 워터폴을 생성하여 성능 저하를 유발하는 안티패턴입니다.

이 PR에서 추가하신 .agents/skills/next-best-practices/font.md 문서의 209~214번 라인에서도 @import 사용을 지양하도록 명시하고 있습니다. 기존에 사용하던 next/font 기반의 폰트 로딩 방식(예: Pretendard 로컬 폰트 설정)을 유지하거나, next/font/local을 사용하여 폰트를 최적화하는 것이 좋습니다.

@@ -0,0 +1,88 @@
import { useCallback, useLayoutEffect, useRef } from 'react';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

useEffect를 사용하기 위해 react 패키지에서 useEffect를 가져와야 합니다.

Suggested change
import { useCallback, useLayoutEffect, useRef } from 'react';
import { useCallback, useEffect, useLayoutEffect, useRef } from 'react';


// 최초 렌더링 시 1회 하단으로 이동
useLayoutEffect(() => {
if (didInitialScrollRef.current) return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Next.js의 SSR(Server-Side Rendering) 환경에서는 서버에서 컴포넌트를 프리렌더링할 때 useLayoutEffect가 사용되면 Warning: useLayoutEffect does nothing on the server 경고가 발생합니다.

이를 방지하기 위해, 최초 렌더링 시 하단으로 스크롤하는 효과는 useEffect로 변경하는 것을 권장합니다. 이 효과는 레이아웃 동기화가 엄격하게 필요하지 않으므로 useEffect로도 충분히 동작합니다.

  useEffect(() => {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant