-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] 스토리지 변경 & 중복 호출 방지 제거 #60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughToken storage is migrated from localStorage to sessionStorage across auth, API calls, and login flow. League page is refactored to fetch real data with tier selection and infinite scroll, supported by new API modules, endpoints, and UI components. Tailwind tooling is added and configured. Legacy mypage API is replaced. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant LeaguePage
participant TierSelector
participant UserList
participant API as API Modules
participant Session as sessionStorage
User->>LeaguePage: Open page
LeaguePage->>API: useQuery getMainInfo()
API-->>LeaguePage: {nickname, league, level, xp}
LeaguePage->>Session: read accessToken
LeaguePage->>TierSelector: set selectedTierId (from league)
User->>TierSelector: Click tier
TierSelector-->>LeaguePage: onSelectTier(tierId)
LeaguePage->>API: getUserLeagues() or getTierLeagues()
API-->>LeaguePage: users page N
LeaguePage->>UserList: render users (append)
User->>UserList: Scroll
UserList-->>LeaguePage: request next page (if hasMore)
LeaguePage->>API: fetch next page (guarded by fetchingRef)
API-->>LeaguePage: users page N+1
LeaguePage->>UserList: render (append or stop if < pageSize)
sequenceDiagram
autonumber
participant Header
participant Session as sessionStorage
participant Window as window(storage)
Header->>Session: read accessToken
Header->>Header: set isLoggedIn
Note over Header: Poll every 200ms and listen to storage events
Window-->>Header: storage event (accessToken changed)
Header->>Session: re-read accessToken
Header->>Header: update isLoggedIn
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/api/fetchChapters.ts (1)
6-14: Avoid sending "Bearer null"; guard token and consolidate headersDon’t include Authorization if no token. Also safe-guard sessionStorage for SSR.
-export default async function fetchChapters(): Promise<Chapter[]> { - const accessToken = sessionStorage.getItem('accessToken'); +export default async function fetchChapters(): Promise<Chapter[]> { + const accessToken = typeof window !== 'undefined' ? sessionStorage.getItem('accessToken') : null; try { - const response = await fetch(`https://grav-it.inuappcenter.kr/api/v1/learning/chapters`, { + const headers: Record<string, string> = { 'Content-Type': 'application/json' }; + if (accessToken) headers.Authorization = `Bearer ${accessToken}`; + const response = await fetch(`https://grav-it.inuappcenter.kr/api/v1/learning/chapters`, { method: 'GET', - headers: { - 'Content-Type': 'application/json', - Authorization: `Bearer ${accessToken}`, - }, + headers, });src/api/fetchMainInfo.ts (1)
11-14: Conditionally set Authorization headerSkip header when token is absent to prevent "Bearer null".
- headers: { - 'Content-Type': 'application/json', - Authorization: `Bearer ${accessToken}`, - }, + headers: { + 'Content-Type': 'application/json', + ...(accessToken ? { Authorization: `Bearer ${accessToken}` } : {}), + },src/constants/api.ts (1)
1-32: Remove remaining axios usages and useRef hooks
- src/api/patchOnBoarding.tsx & package.json: still import and use axios – replace with the new API client or fetch
- src/api/PostOAuth.tsx & src/pages/LeaguePage.tsx: still use useRef – refactor to eliminate these hooks
- No stray localStorage found; token reads are correctly using sessionStorage.getItem
🧹 Nitpick comments (38)
src/api/PostOAuth.tsx (4)
39-39: Token now in sessionStorage: check cross-tab login sync.sessionStorage is per-tab. If header or other tabs relied on storage events for login state sync, they won’t fire across tabs anymore. Consider BroadcastChannel or an in-app auth context for sync.
22-23: Use Vite’s env API.Replace process.env.NODE_ENV with import.meta.env.MODE to avoid prod misdetection in Vite builds.
- const dest = process.env.NODE_ENV === 'production' ? 'prod' : 'local'; + const dest = import.meta.env.MODE === 'production' ? 'prod' : 'local';
41-45: Honor returnTo after OAuth.You store returnTo during login but don’t consume it here. Redirect to it when available, then clear it.
- if (data.isOnboarded) { - navigate('/main', { replace: true }); - } else { - navigate('/set-info', { replace: true }); - } + const returnTo = sessionStorage.getItem('returnTo'); + if (data.isOnboarded) { + navigate(returnTo || '/main', { replace: true }); + } else { + navigate('/set-info', { replace: true }); + } + if (returnTo) sessionStorage.removeItem('returnTo');
10-19: Objective mismatch: duplicate-call guard still present.PR says “중복 호출 방지 제거,” but isCalledRef remains. If you still want to remove it, guard with AbortController to prevent double POSTs in StrictMode without ref state.
-import { useEffect, useRef } from 'react'; +import { useEffect } from 'react'; ... - const isCalledRef = useRef(false); ... - if (!code || isCalledRef.current) return; - isCalledRef.current = true; + if (!code) return; ... - const postCode = async () => { + const postCode = async () => { + const ac = new AbortController(); + const { signal } = ac; try { ... - const res = await fetch( + const res = await fetch( `${API_ENDPOINTS.oauth.base}/${provider}?dest=${dest}`, - { + { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ code }), + body: JSON.stringify({ code }), + signal, } ); ... - }; + return () => ac.abort(); + };src/pages/LoginPage.tsx (3)
23-23: Store full path (incl. query/hash) for precise return.pathname alone drops ?query and #hash.
-sessionStorage.setItem('returnTo', window.location.pathname); +sessionStorage.setItem('returnTo', window.location.pathname + window.location.search + window.location.hash);
12-16: Use import.meta.env.MODE under Vite.Avoid process.env in browser code with Vite.
-const dest = process.env.NODE_ENV === 'production' ? 'prod' : 'local'; +const dest = import.meta.env.MODE === 'production' ? 'prod' : 'local';
19-21: Defensive check on data.loginUrl.Optionally validate it’s a safe same-origin redirect or an expected SSO domain to prevent open-redirect misuse.
src/api/patchOnBoarding.tsx (1)
16-20: Centralize auth headers.Multiple modules build the same Authorization header. Consider an axios instance with an interceptor that reads sessionStorage to reduce duplication and drift.
tailwind.config.ts (1)
1-14: Tailwind v4: content key is no longer used.With tailwindcss ^4.x, scanning is automatic; the content array can be removed.
-export default { - content: [ - './index.html', - './src/**/*.{js,ts,jsx,tsx}', - ], +export default { theme: { extend: { fontFamily: { mbc: ['MBC1961GulimM', 'sans-serif'], }, }, }, plugins: [], }package.json (1)
13-23: Move @tailwindcss/vite to devDependencies and align versions.The Vite plugin is build-time only; keep it out of runtime deps and bump to match tailwindcss 4.1.12.
"dependencies": { - "@tailwindcss/vite": "^4.1.11", + "@tanstack/react-query": "^5.85.3", "axios": "^1.11.0", @@ "devDependencies": { + "@tailwindcss/vite": "^4.1.12",Also applies to: 24-43
src/components/@common/header/Header.tsx (1)
31-31: Tailwind class may be invalid
z-100isn’t a default Tailwind utility. Usez-[100](arbitrary value) or a supported token likez-50.- <header className="fixed w-full z-100 mx-auto my-0 px-8 py-4 flex items-center justify-center box-border bg-white h-[var(--header-height)]"> + <header className="fixed w-full z-[100] mx-auto my-0 px-8 py-4 flex items-center justify-center box-border bg-white h-[var(--header-height)]">src/pages/UserPage.tsx (1)
38-47: Verify response field names; prevent undefined mapping
res.followervsres.followerslooks inconsistent and can render 0/undefined. Also consider typing the response.- setUserinfo({ - nickname: res.nickname, - profileImgNumber: res.profileImgNumber, - handle: res.handle, - followers: res.follower, - following: res.following, - badge: dummyUserInfo.badge, - version: dummyUserInfo.version, - }); + setUserinfo({ + nickname: res.nickname, + profileImgNumber: res.profileImgNumber ?? 1, + handle: res.handle, + followers: res.followers ?? 0, + following: res.following ?? 0, + badge: dummyUserInfo.badge, + version: dummyUserInfo.version, + });Optionally define a response type in
src/types/api/mypage.tsand typegetMypage(): Promise<MyPageResponse>.src/api/fetchChapters.ts (2)
34-34: Remove debug logDrop console logs in production paths.
- console.log('서버 응답:', data); // 디버깅용 + // console.debug('fetchChapters response:', data);
8-9: Use API_ENDPOINTS instead of hard-coded URLCentralize base URLs to avoid drift and ease environment switches.
If available, replace with
API_ENDPOINTS.learning.chapters(or similar) and keep all endpoints insrc/constants/api.ts.src/api/fetchMainInfo.ts (1)
35-35: Remove debug logAvoid noisy logs in production.
- console.log('서버 응답:', data); // 디버깅용 + // console.debug('fetchMainInfo response:', data);src/constants/tiers.ts (1)
1-8: Add explicit typing and immutability for tiersStrengthen types for
iconand prevent accidental mutation.-import TierCircle from '@/assets/icons/tier-circle.svg?react'; - -export const tiers = [ - { id: 1, name: '브론즈', icon: TierCircle, exp: 10 }, - { id: 2, name: '실버', icon: TierCircle, exp: 20 }, - { id: 3, name: '골드', icon: TierCircle, exp: 30 }, - { id: 4, name: '마스터', icon: TierCircle, exp: 40 }, -]; +import TierCircle from '@/assets/icons/tier-circle.svg?react'; +import type { SVGProps, ComponentType } from 'react'; + +type Tier = { + id: number; + name: string; + icon: ComponentType<SVGProps<SVGSVGElement>>; + exp: number; +}; + +export const tiers: readonly Tier[] = [ + { id: 1, name: '브론즈', icon: TierCircle, exp: 10 }, + { id: 2, name: '실버', icon: TierCircle, exp: 20 }, + { id: 3, name: '골드', icon: TierCircle, exp: 30 }, + { id: 4, name: '마스터', icon: TierCircle, exp: 40 }, +] as const;src/constants/api.ts (1)
27-31: Preferpageovernum; confirm page indexing with BERename the parameter for clarity and verify whether the API expects 0- or 1-based pages.
- user: (num: number) => `${API_BASE_URL}/rank/user-leagues/page/${num}`, - tier: (id: number, num: number) => `${API_BASE_URL}/rank/leagues/${id}/page/${num}`, + user: (page: number) => `${API_BASE_URL}/rank/user-leagues/page/${page}`, + tier: (id: number, page: number) => `${API_BASE_URL}/rank/leagues/${id}/page/${page}`,src/api/getMypage.ts (2)
16-20: Harden error parsing for non-JSON responsesPrevents secondary exceptions on empty or text error bodies.
- if (!response.ok) { - const errorData = await response.json(); - console.error('Error response:', response.status, errorData); - throw new Error(`HTTP error! status: ${response.status}`); - } + if (!response.ok) { + let errorBody: unknown; + try { + errorBody = await response.json(); + } catch { + errorBody = await response.text(); + } + console.error('Error response:', response.status, errorBody); + throw new Error(`HTTP ${response.status}: ${response.statusText}`); + }
1-3: Type the return valueAdd an explicit return type (adjust to the actual API type).
-import { API_ENDPOINTS } from '../constants/api'; +import { API_ENDPOINTS } from '../constants/api'; +import type { MainPageResponse } from '../types/api/main'; -export const getMypage = async () => { +export const getMypage = async (): Promise<MainPageResponse> => {src/api/getUserLeagues.ts (3)
16-20: Robust error body handlingSame pattern as suggested elsewhere.
- if (!response.ok) { - const errorData = await response.json(); - console.error('Error response:', response.status, errorData); - throw new Error(`HTTP error! status: ${response.status}`); - } + if (!response.ok) { + let errorBody: unknown; + try { + errorBody = await response.json(); + } catch { + errorBody = await response.text(); + } + console.error('Error response:', response.status, errorBody); + throw new Error(`HTTP ${response.status}: ${response.statusText}`); + }
22-24: Remove debug logAvoid leaking payloads in prod logs.
- const data = await response.json(); - console.log(data); - return data; + const data = await response.json(); + return data;
3-8: Usepagenaming for clarity (propagate to callers)Improves readability; update call sites accordingly.
-export const getUserLeagues = async (num: number) => { +export const getUserLeagues = async (page: number) => { @@ - const response = await fetch(API_ENDPOINTS.leagues.user(num), { + const response = await fetch(API_ENDPOINTS.leagues.user(page), {src/api/getTierLeagues.ts (3)
16-20: Robust error body handlingAlign with shared error parsing.
- if (!response.ok) { - const errorData = await response.json(); - console.error('Error response:', response.status, errorData); - throw new Error(`HTTP error! status: ${response.status}`); - } + if (!response.ok) { + let errorBody: unknown; + try { + errorBody = await response.json(); + } catch { + errorBody = await response.text(); + } + console.error('Error response:', response.status, errorBody); + throw new Error(`HTTP ${response.status}: ${response.statusText}`); + }
22-24: Remove debug logDrop
console.login production paths.- const data = await response.json(); - console.log(data); - return data; + const data = await response.json(); + return data;
3-8: Renamenum→page(propagate)For consistency with endpoints.
-export const getTierLeagues = async (id: number, num: number) => { +export const getTierLeagues = async (id: number, page: number) => { @@ - const response = await fetch(API_ENDPOINTS.leagues.tier(id, num), { + const response = await fetch(API_ENDPOINTS.leagues.tier(id, page), {src/components/league-page/UserList.tsx (4)
35-35: Fix stray Tailwind class
h-is a typo and does nothing.- <div className="flex w-14 h-14 h- items-center justify-center"> + <div className="flex w-14 h-14 items-center justify-center">
5-17: Avoid shadowing the globalUsertypeUse a distinct name to prevent confusion with
src/types/@common/user.ts.-interface User { +interface LeagueUser { userId: number; nickname: string; level: number; lp: number; rank: number; profileImgNumber: number; } interface UserListProps { - users: User[]; + users: LeagueUser[]; loading: boolean; }
23-25: Provide a safe fallback colorPrevents undefined color when
profileImgNumberis out of range.- const profileBgColor = - PROFILE_COLORS[user.profileImgNumber as keyof typeof PROFILE_COLORS]; + const profileBgColor = + PROFILE_COLORS[user.profileImgNumber as keyof typeof PROFILE_COLORS] ?? '#C4C4C4';
55-55: Minor a11y: announce loading stateAdd ARIA to inform screen readers.
- {loading && <div className="text-center py-2">Loading...</div>} + {loading && <div className="text-center py-2" role="status" aria-live="polite">Loading...</div>}src/components/@common/level-info/LevelProgressCircle.tsx (4)
32-38: Base track is invisible (missing stroke).The background circle has no stroke color, so the track won’t render.
<circle cx={size / 2} cy={size / 2} r={radius} strokeWidth={strokeWidth} + stroke="#E5E7EB" fill="none" />
16-17: Clamp progress and guard invalid maxLevel.Prevents negative levels and maxLevel <= 0 from producing odd visuals.
- const progress = Math.min(level / maxLevel, 1); + const progress = Math.max(0, Math.min(maxLevel > 0 ? level / maxLevel : 0, 1));
43-56: Avoid gradient ID collisions across multiple instances.Use a unique id per component to prevent defs/id clashes.
- stroke="url(#levelGradient)" + stroke={`url(#${gradientId})`} ... - <defs> - <linearGradient id="levelGradient" x1="0%" y1="0%" x2="100%" y2="0%"> + <defs> + <linearGradient id={gradientId} x1="0%" y1="0%" x2="100%" y2="0%">Add this import and hook (outside the shown range):
import { useId } from 'react'; const gradientId = useId();
25-31: Add basic accessibility.If decorative, hide from AT; if meaningful, add an aria-label. Here’s the decorative option.
- <svg + <svg width={size} height={size} className="absolute top-0 left-0 z-10" style={{ transform: "rotate(90deg) scale(-1, 1)" }} + aria-hidden="true" >src/components/league-page/TierSelector.tsx (3)
1-1: Align with PR goal (“remove useRef”): drop itemRefs and use event.currentTarget.You can center-scroll without refs.
-import { useRef } from 'react'; +// no refs needed
17-26: Ref-free scrollIntoView via event target.Removes the refs array and simplifies the handler.
- const itemRefs = useRef<(HTMLDivElement | null)[]>([]); - - const handleClick = (tierId: number, index: number) => { - onSelectTier(tierId); - itemRefs.current[index]?.scrollIntoView({ + const handleClick = (tierId: number, e: React.MouseEvent<HTMLDivElement>) => { + onSelectTier(tierId); + e.currentTarget.scrollIntoView({ behavior: 'smooth', inline: 'center', block: 'nearest', }); }; ... - <div + <div key={tier.id} - ref={(el) => { itemRefs.current[index] = el; }} - onClick={() => handleClick(tier.id, index)} + onClick={(e) => handleClick(tier.id, e)} className={`flex flex-col items-center cursor-pointer transition-all duration-300 justify-center ${ isSelected ? 'scale-110 px-4' : '' } min-h-[140px]`} >Also applies to: 36-43
36-43: Make tiles keyboard-accessible (button semantics).Current divs are not focusable/clickable via keyboard.
- <div + <button key={tier.id} onClick={(e) => handleClick(tier.id, e)} - className={`flex flex-col items-center cursor-pointer transition-all duration-300 justify-center ${ + type="button" + className={`flex flex-col items-center cursor-pointer transition-all duration-300 justify-center ${ isSelected ? 'scale-110 px-4' : '' } min-h-[140px]`} > ... - </div> + </button>Note: If applied, update the handler’s event type to
React.MouseEvent<HTMLButtonElement>.src/pages/LeaguePage.tsx (2)
104-106: Use passive scroll listener to avoid main-thread jank.Small perf win for high-frequency events.
- container.addEventListener('scroll', handleScroll); - return () => container.removeEventListener('scroll', handleScroll); + container.addEventListener('scroll', handleScroll, { passive: true }); + return () => container.removeEventListener('scroll', handleScroll);
74-75: Hard-coded page size assumption.
newUsers.length < 10assumes a page size of 10. If the backend changes, pagination breaks.Expose pageSize from the API layer and compare against it (or use a
hasNextflag).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
package.json(2 hunks)src/api/PostOAuth.tsx(1 hunks)src/api/fetchChapters.ts(1 hunks)src/api/fetchMainInfo.ts(1 hunks)src/api/getMypage.ts(1 hunks)src/api/getMypage.tsx(0 hunks)src/api/getTierLeagues.ts(1 hunks)src/api/getUserLeagues.ts(1 hunks)src/api/patchOnBoarding.tsx(1 hunks)src/components/@common/header/Header.tsx(1 hunks)src/components/@common/level-info/LevelProgressCircle.tsx(1 hunks)src/components/league-page/TierSelector.tsx(1 hunks)src/components/league-page/UserList.tsx(1 hunks)src/constants/api.ts(1 hunks)src/constants/tiers.ts(1 hunks)src/pages/LeaguePage.tsx(1 hunks)src/pages/LoginPage.tsx(1 hunks)src/pages/MainPage.tsx(0 hunks)src/pages/UserPage.tsx(2 hunks)tailwind.config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- src/api/getMypage.tsx
- src/pages/MainPage.tsx
🧰 Additional context used
🧬 Code graph analysis (14)
src/api/PostOAuth.tsx (1)
src/api/getMypage.tsx (1)
accessToken(5-30)
src/api/getUserLeagues.ts (2)
src/api/getMypage.tsx (1)
accessToken(5-30)src/api/fetchUnits.ts (1)
fetchUnits(5-42)
src/api/getTierLeagues.ts (2)
src/api/getMypage.tsx (1)
accessToken(5-30)src/api/fetchUnits.ts (1)
fetchUnits(5-42)
src/api/fetchChapters.ts (2)
src/api/getMypage.tsx (1)
accessToken(5-30)src/pages/ChapterListPage.tsx (1)
ChapterListPage(7-37)
src/api/fetchMainInfo.ts (1)
src/api/getMypage.tsx (1)
accessToken(5-30)
src/api/patchOnBoarding.tsx (2)
src/api/getMypage.tsx (1)
accessToken(5-30)src/pages/SetInfoPage.tsx (2)
nickname(47-66)SetInfoPage(11-133)
src/pages/UserPage.tsx (1)
src/api/getMypage.tsx (1)
accessToken(5-30)
src/api/getMypage.ts (1)
src/api/getMypage.tsx (1)
accessToken(5-30)
src/constants/api.ts (1)
src/types/api/main.ts (1)
MainPageResponse(10-16)
src/constants/tiers.ts (3)
src/components/@common/level-info/UserStats.tsx (1)
PlayerStatsProps(12-21)src/types/@common/user.ts (1)
User(3-9)src/types/api/main.ts (1)
MainPageResponse(10-16)
src/components/@common/level-info/LevelProgressCircle.tsx (2)
src/components/@common/level-info/LevelProgressBar.tsx (1)
LevelProgressBarProps(6-19)src/components/chapter-page/CircularSegmentIndicator.tsx (3)
children(3-113)_(85-106)_(69-82)
src/components/@common/header/Header.tsx (3)
src/components/@common/header/HeaderNav.tsx (1)
HeaderNav(19-37)src/components/@common/header/LoginButton.tsx (1)
LoginButton(3-9)src/components/@common/header/StartButton.tsx (1)
StartButton(3-9)
src/components/league-page/UserList.tsx (1)
src/types/@common/user.ts (1)
User(3-9)
src/pages/LeaguePage.tsx (2)
src/pages/MainPage.tsx (1)
MainPage(11-92)src/components/@common/level-info/UserStats.tsx (1)
PlayerStatsProps(12-21)
🔇 Additional comments (7)
tailwind.config.ts (1)
7-11: LGTM on custom font alias.The mbc family extension looks fine; ensure the font is actually loaded via @font-face or similar.
package.json (1)
31-38: Tailwind v4 toolchain sanity check.With tailwindcss ^4.x, autoprefixer/postcss configs differ from v3. Verify your CSS entry uses @import "tailwindcss" and that no legacy content config is relied upon.
src/api/getMypage.ts (1)
13-14: Confirm cookie requirement with BearerUsing both
credentials: 'include'and Bearer is fine if the API needs cookies; otherwise drop credentials to simplify CORS.src/components/league-page/UserList.tsx (1)
37-38: Verify SVG usescurrentColorIf the SVG doesn’t inherit
currentColor,style={{ color }}won’t affect fill. Confirm or switch tofill.src/components/@common/level-info/LevelProgressCircle.tsx (1)
1-5: Type namespace safety for React types.Depending on tsconfig,
React.ReactNodemay error. Prefer importing the type.-interface LevelProgressCircleProps { +import type { ReactNode } from 'react'; +interface LevelProgressCircleProps { level: number; maxLevel?: number; - children?: React.ReactNode; + children?: ReactNode; }src/components/league-page/TierSelector.tsx (1)
28-64: Verify: Use of useRef contradicts PR description.PR says “중복 호출 방지 제거 / remove useRef”, but this component used
useRef(and LeaguePage uses it too). Confirm intent.src/pages/LeaguePage.tsx (1)
27-28: Verify PR intent: “중복 호출 방지 제거”.This page still uses
useRefforfetchingRefto prevent overlap. If removal was intended repo-wide, confirm whether this guard should be dropped or kept.Also applies to: 55-56, 98-101
|
|
||
| export default async function fetchMainInfo(): Promise<MainPageResponse> { | ||
| const accessToken = localStorage.getItem('accessToken'); | ||
| const accessToken = sessionStorage.getItem('accessToken'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Guard sessionStorage for SSR
Avoid accessing Web APIs during SSR.
- const accessToken = sessionStorage.getItem('accessToken');
+ const accessToken = typeof window !== 'undefined' ? sessionStorage.getItem('accessToken') : null;📝 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 accessToken = sessionStorage.getItem('accessToken'); | |
| const accessToken = typeof window !== 'undefined' | |
| ? sessionStorage.getItem('accessToken') | |
| : null; |
🤖 Prompt for AI Agents
In src/api/fetchMainInfo.ts around line 6, the direct call to
sessionStorage.getItem('accessToken') runs during SSR and can crash; guard
access by checking that window (or sessionStorage) is defined before calling
getItem (e.g., if (typeof window !== 'undefined' && window.sessionStorage) { ...
}) and provide a safe fallback (null or an empty string) when unavailable so the
function behaves correctly both in browser and server environments.
| import { API_ENDPOINTS } from '../constants/api'; | ||
|
|
||
| export const getMypage = async () => { | ||
| const accessToken = sessionStorage.getItem('accessToken'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Guard missing token; tighten headers
Avoid sending Authorization: Bearer null and prefer Accept for GET.
const accessToken = sessionStorage.getItem('accessToken');
+ if (!accessToken) {
+ throw new Error('Missing access token');
+ }
try {
const response = await fetch(`${API_ENDPOINTS.users.base}/my-page`, {
method: 'GET',
headers: {
- 'Authorization': `Bearer ${accessToken}`,
- 'Content-Type': 'application/json',
+ Authorization: `Bearer ${accessToken}`,
+ Accept: 'application/json',
},
credentials: 'include',
});Also applies to: 9-14
🤖 Prompt for AI Agents
In src/api/getMypage.ts around line 4 (and also lines 9-14), avoid sending
Authorization: Bearer null and use Accept for GET requests: first read the token
and guard it (if no token, handle unauthenticated flow or return early) and only
add the Authorization header when token is a non-empty string; for the GET fetch
call replace Content-Type with an Accept: application/json header and ensure the
headers object is built conditionally so Authorization is omitted when token is
missing.
| import { API_ENDPOINTS } from '../constants/api'; | ||
|
|
||
| export const getTierLeagues = async (id: number, num: number) => { | ||
| const accessToken = sessionStorage.getItem('accessToken'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Token guard + lean GET headers
Same fix as other APIs.
const accessToken = sessionStorage.getItem('accessToken');
+ if (!accessToken) {
+ throw new Error('Missing access token');
+ }
@@
headers: {
- 'Authorization': `Bearer ${accessToken}`,
- 'Content-Type': 'application/json',
+ Authorization: `Bearer ${accessToken}`,
+ Accept: 'application/json',
},
credentials: 'include', Also applies to: 9-14
🤖 Prompt for AI Agents
In src/api/getTierLeagues.ts around lines 4 and 9-14, add a guard for missing
accessToken and simplify the GET request headers: check
sessionStorage.getItem('accessToken') and if falsy return or throw a clear error
(e.g., throw new Error('No access token')), then call fetch with method: 'GET'
and a minimal headers object including Authorization: `Bearer ${accessToken}`
and Accept: 'application/json' (remove any unnecessary headers/body), ensuring
early exit on missing token to prevent unauthenticated requests.
| import { API_ENDPOINTS } from '../constants/api'; | ||
|
|
||
| export const getUserLeagues = async (num: number) => { | ||
| const accessToken = sessionStorage.getItem('accessToken'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Token guard + lean GET headers
Avoid Bearer null and prefer Accept header.
const accessToken = sessionStorage.getItem('accessToken');
+ if (!accessToken) {
+ throw new Error('Missing access token');
+ }
try {
const response = await fetch(API_ENDPOINTS.leagues.user(num), {
method: 'GET',
headers: {
- 'Authorization': `Bearer ${accessToken}`,
- 'Content-Type': 'application/json',
+ Authorization: `Bearer ${accessToken}`,
+ Accept: 'application/json',
},
credentials: 'include',
});Also applies to: 9-14
🤖 Prompt for AI Agents
In src/api/getUserLeagues.ts around lines 4 and also lines 9-14, the code reads
the accessToken from sessionStorage and may send an Authorization header
containing "Bearer null"; update the token guard so you check that accessToken
is truthy and not the string "null" before using it (return or throw / handle
unauthenticated case early), and when constructing the GET request headers
always include Accept: application/json and add Authorization: `Bearer
${accessToken}` only when the token passes the guard; ensure no Authorization
header is sent if token is missing.
|
|
||
| export const PatchOnBoarding = async (nickname: string, profilePhotoNumber: number) => { | ||
| const accessToken = localStorage.getItem('accessToken'); | ||
| const accessToken = sessionStorage.getItem('accessToken'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid sending “Bearer null” when token is missing.
Guard early and surface an actionable error (or redirect) instead of hitting the API with an invalid header.
-const accessToken = sessionStorage.getItem('accessToken');
+const accessToken = sessionStorage.getItem('accessToken');
+if (!accessToken) {
+ throw new Error('Unauthenticated: missing access token. Please log in again.');
+}📝 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 accessToken = sessionStorage.getItem('accessToken'); | |
| const accessToken = sessionStorage.getItem('accessToken'); | |
| if (!accessToken) { | |
| throw new Error('Unauthenticated: missing access token. Please log in again.'); | |
| } |
🤖 Prompt for AI Agents
In src/api/patchOnBoarding.tsx around line 6, the code reads the accessToken
from sessionStorage but may send an Authorization header like "Bearer null" when
missing; change the flow to guard immediately after getting the token—if
accessToken is null/undefined/empty then either throw a clear error or redirect
to the login page and do not call the API; when building request headers only
include Authorization when a valid token exists (e.g., add header conditionally
or return early), and ensure any error paths surface an actionable message for
the caller.
| const [isLoggedIn, setIsLoggedIn] = useState<boolean>( | ||
| Boolean(sessionStorage.getItem('accessToken')) | ||
| ); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid SSR crash: guard sessionStorage in initial state
Initialize with a lazy getter and window check to prevent ReferenceError during SSR.
- const [isLoggedIn, setIsLoggedIn] = useState<boolean>(
- Boolean(sessionStorage.getItem('accessToken'))
- );
+ const [isLoggedIn, setIsLoggedIn] = useState<boolean>(() => {
+ return typeof window !== 'undefined' && !!sessionStorage.getItem('accessToken');
+ });📝 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 [isLoggedIn, setIsLoggedIn] = useState<boolean>( | |
| Boolean(sessionStorage.getItem('accessToken')) | |
| ); | |
| const [isLoggedIn, setIsLoggedIn] = useState<boolean>(() => { | |
| return typeof window !== 'undefined' && !!sessionStorage.getItem('accessToken'); | |
| }); |
🤖 Prompt for AI Agents
In src/components/@common/header/Header.tsx around lines 8 to 11, the component
reads sessionStorage directly when initializing state which can throw a
ReferenceError during SSR; change useState to use a lazy initializer function
that first checks typeof window !== 'undefined' (or checks for sessionStorage)
and returns Boolean(sessionStorage.getItem('accessToken')) only when safe,
otherwise return false, so the initial render on the server doesn't access
browser globals.
| useEffect(() => { | ||
| const handleStorageChange = () => { | ||
| setIsLoggedIn(Boolean(sessionStorage.getItem('accessToken'))); | ||
| }; | ||
|
|
||
| window.addEventListener('storage', handleStorageChange); | ||
|
|
||
| const interval = setInterval(() => { | ||
| const current = Boolean(sessionStorage.getItem('accessToken')); | ||
| setIsLoggedIn((prev) => (prev !== current ? current : prev)); | ||
| }, 200); | ||
|
|
||
| return () => { | ||
| window.removeEventListener('storage', handleStorageChange); | ||
| clearInterval(interval); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove 200ms polling; storage event is ineffective for sessionStorage
sessionStorage is per-tab; storage events won’t help across tabs and won’t fire in the same tab that sets the item. A 200ms poll is wasteful. Re-check token on focus/visibility or wire a custom "auth:changed" event from login/logout.
- useEffect(() => {
- const handleStorageChange = () => {
- setIsLoggedIn(Boolean(sessionStorage.getItem('accessToken')));
- };
-
- window.addEventListener('storage', handleStorageChange);
-
- const interval = setInterval(() => {
- const current = Boolean(sessionStorage.getItem('accessToken'));
- setIsLoggedIn((prev) => (prev !== current ? current : prev));
- }, 200);
-
- return () => {
- window.removeEventListener('storage', handleStorageChange);
- clearInterval(interval);
- };
- }, []);
+ useEffect(() => {
+ const sync = () => setIsLoggedIn(!!sessionStorage.getItem('accessToken'));
+ // Update when tab gains focus or becomes visible (covers most real flows)
+ window.addEventListener('focus', sync);
+ document.addEventListener('visibilitychange', sync);
+ // Optionally listen for a custom event emitted by auth flows
+ window.addEventListener('auth:changed', sync as EventListener);
+ return () => {
+ window.removeEventListener('focus', sync);
+ document.removeEventListener('visibilitychange', sync);
+ window.removeEventListener('auth:changed', sync as EventListener);
+ };
+ }, []);📝 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.
| useEffect(() => { | |
| const handleStorageChange = () => { | |
| setIsLoggedIn(Boolean(sessionStorage.getItem('accessToken'))); | |
| }; | |
| window.addEventListener('storage', handleStorageChange); | |
| const interval = setInterval(() => { | |
| const current = Boolean(sessionStorage.getItem('accessToken')); | |
| setIsLoggedIn((prev) => (prev !== current ? current : prev)); | |
| }, 200); | |
| return () => { | |
| window.removeEventListener('storage', handleStorageChange); | |
| clearInterval(interval); | |
| }; | |
| }, []); | |
| useEffect(() => { | |
| const sync = () => setIsLoggedIn(!!sessionStorage.getItem('accessToken')); | |
| // Update when tab gains focus or becomes visible (covers most real flows) | |
| window.addEventListener('focus', sync); | |
| document.addEventListener('visibilitychange', sync); | |
| // Optionally listen for a custom event emitted by auth flows | |
| window.addEventListener('auth:changed', sync as EventListener); | |
| return () => { | |
| window.removeEventListener('focus', sync); | |
| document.removeEventListener('visibilitychange', sync); | |
| window.removeEventListener('auth:changed', sync as EventListener); | |
| }; | |
| }, []); |
| <div className="flex flex-row gap-3 items-center"> | ||
| {isLoggedIn && <LoginButton />} | ||
| {isLoggedIn && <StartButton />} | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Login UI shows when already logged in (inverted condition)
"로그인" and "그래빗 시작하기" should appear when NOT logged in.
- {isLoggedIn && <LoginButton />}
- {isLoggedIn && <StartButton />}
+ {!isLoggedIn && <LoginButton />}
+ {!isLoggedIn && <StartButton />}📝 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.
| <div className="flex flex-row gap-3 items-center"> | |
| {isLoggedIn && <LoginButton />} | |
| {isLoggedIn && <StartButton />} | |
| </div> | |
| <div className="flex flex-row gap-3 items-center"> | |
| - {isLoggedIn && <LoginButton />} | |
| {!isLoggedIn && <LoginButton />} | |
| {!isLoggedIn && <StartButton />} | |
| </div> |
🤖 Prompt for AI Agents
In src/components/@common/header/Header.tsx around lines 38 to 41, the
LoginButton and StartButton are rendered when isLoggedIn is true (inverted
condition); change the render condition to !isLoggedIn so both buttons are shown
only when the user is NOT logged in (replace {isLoggedIn && <LoginButton />} and
{isLoggedIn && <StartButton />} with checks against !isLoggedIn).
| const fetchUsers = useCallback( | ||
| async (targetPage: number, targetHasMore: boolean) => { | ||
| if (!selectedTierId || fetchingRef.current || !targetHasMore) return; | ||
|
|
||
| fetchingRef.current = true; | ||
| try { | ||
| let newUsers: User[] = []; | ||
| if (selectedTierId === getTierIdFromLeague(mainInfo?.league)) { | ||
| newUsers = await getUserLeagues(targetPage); | ||
| } else { | ||
| newUsers = await getTierLeagues(selectedTierId, targetPage); | ||
| } | ||
|
|
||
| if (newUsers.length > 0) { | ||
| setUsers((prev) => { | ||
| const existingIds = new Set(prev.map((u) => u.userId)); | ||
| return [...prev, ...newUsers.filter((u) => !existingIds.has(u.userId))]; | ||
| }); | ||
| setPage(targetPage + 1); | ||
| } | ||
|
|
||
| if (newUsers.length < 10) setHasMore(false); | ||
| } catch (error) { | ||
| console.error('유저 불러오기 실패:', error); | ||
| } finally { | ||
| fetchingRef.current = false; | ||
| } | ||
| }, | ||
| [selectedTierId, mainInfo] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale response can corrupt list when tier changes mid-fetch.
An in-flight request for the old tier can append into the new tier’s list.
Minimal fix: gate state updates to the latest request.
const fetchingRef = useRef(false);
+ const requestIdRef = useRef(0);
...
const fetchUsers = useCallback(
async (targetPage: number, targetHasMore: boolean) => {
if (!selectedTierId || fetchingRef.current || !targetHasMore) return;
fetchingRef.current = true;
+ const requestId = ++requestIdRef.current;
try {
let newUsers: User[] = [];
if (selectedTierId === getTierIdFromLeague(mainInfo?.league)) {
newUsers = await getUserLeagues(targetPage);
} else {
newUsers = await getTierLeagues(selectedTierId, targetPage);
}
+ // Abandon if a newer request started (e.g., tier changed)
+ if (requestId !== requestIdRef.current) return;
+
if (newUsers.length > 0) {
setUsers((prev) => {
const existingIds = new Set(prev.map((u) => u.userId));
return [...prev, ...newUsers.filter((u) => !existingIds.has(u.userId))];
});
setPage(targetPage + 1);
}
if (newUsers.length < 10) setHasMore(false);
} catch (error) {
console.error('유저 불러오기 실패:', error);
} finally {
fetchingRef.current = false;
}
},
[selectedTierId, mainInfo]
);Alternative: pass AbortController to API and abort on tier change.
Also applies to: 84-91
🤖 Prompt for AI Agents
In src/pages/LeaguePage.tsx around lines 53 to 81 (and similarly 84-91), an
in-flight fetch for a previous tier can append results into the currently
selected tier's list; capture the current selectedTierId (or generate an
incrementing requestId stored in a ref) at the start of fetchUsers and only
apply setUsers/setPage/setHasMore when the stored latest
requestId/selectedTierId still matches (or use an AbortController passed to the
API and abort previous requests on tier change); update the code to check this
gate before every state update and ensure fetchingRef.current is cleared in all
paths.
| sessionStorage.setItem('returnTo', window.location.pathname); | ||
|
|
||
|
|
||
| window.location.href = data.loginUrl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify returnTo consumption after OAuth.
Ensure some code reads sessionStorage.getItem('returnTo') and navigates accordingly (PostOAuth suggestion provided).
🏁 Script executed:
#!/bin/bash
# Find all reads/writes of returnTo and ensure sessionStorage is used consistently.
rg -nP "returnTo" -C2Length of output: 256
Implement returnTo consumption after OAuth login.
No code reads sessionStorage.getItem('returnTo') post-login—add logic in your OAuth callback (e.g. the component handling data.loginUrl) to:
const returnTo = sessionStorage.getItem('returnTo') || '/';
sessionStorage.removeItem('returnTo');
window.location.href = returnTo;🤖 Prompt for AI Agents
In src/pages/LoginPage.tsx around lines 23 to 26, you store a returnTo value in
sessionStorage before redirecting to the OAuth provider but never consume it
after the OAuth callback; update the OAuth callback handler (the component/page
that receives the redirect or handles data.loginUrl resolution) to read
sessionStorage.getItem('returnTo') (defaulting to '/'), remove the item, and
then navigate to that path instead of a fixed URL so the user is returned to
their original location after login.
.github/pull_request_template.md
🔍 작업 유형
📄 작업 내용
localStorage 에서 sesstionStorage로 변경
useRef 제거
🔗 관련 이슈
#59
✅ 체크리스트
🖼️ 🖥 구현 결과 (선택사항)
✔ 리뷰 요구사항
📋 참고 문서
Summary by CodeRabbit