After finishAfter finish - 바텀시트 버그 수정, SEO, 최적화 및 SSR추가 - coderabbit 때매 재시도#65
After finishAfter finish - 바텀시트 버그 수정, SEO, 최적화 및 SSR추가 - coderabbit 때매 재시도#65Hys-Lee merged 343 commits intoprography:mainfrom
Conversation
빌드 시 html생성이 될 때도 있고 안될 때도 있어서. CI에서는 캐싱 문제일 수 있다기에 수정
상호작용이나 hooks사용하는데, use client적는 것 깜빡했었습니다..
라벨 클릭 시 스크롤 초기화 문제로 인해.
실제 오늘 날짜로 하면 크로마틱에서 계속 변화 감지해서 수정함.
날짜를 바꿔 넣어도 컴포넌트 내 비교를 위한 new Date()값은 날마다 바뀔 것이므로, 기준 날짜를 고정시켰습니다
관련 svg설정 next turbopack에 적용
GoalMenuContainer, GoalTItleArea. TodoList는 todoItem이 빈 배열이라 checkedLen필요 없을 때를 고려해 수정함.
고정 너비가 아니라 full로 수정함. 고정 크기일 필요가 없어보여서 임시로 수정. 추후 디자이너가 결정한 것으로 수정 가능.
svg들 폴더 위치 변경
모달 공통 컴포넌트와 파생 컴포넌트 제작. 공통에서 종속적인 부분이 있어 합성 컴포넌트로 제작. 모달 전역 관리를 위해 zustand와 커스텀 훅을 사용. svgr관련 세팅 대해 브랜치 꼬여서 임시 저장.
모달 관련 실행 테스트 위해
webpack에서 빌드 시 번들 정보 파일 생성 코드 제거, 콘솔 제거 루트 페이지에서 cheerRequest는 ssr로 처리하고 있어서, hydration에서 제거함.
try catch 블럭 추가 및 cookie통한 토큰 접근 추가
인터셉트한 결과를 기존 api처리 동작에 완전히 편입
에러나서 터지지 않도록 함.
데이터가 없다면 아예 fallback에 undefined라도 들어가선 안됨.
setTimeout에서 발생하기에, 서버환경에서 실행되면 에러가 발생하고, 이 에러는 핸들링 되지도 못함.
서버 토큰 발급 관련 버그 확인용
cookie에 리프레시 토큰도 저장하도록 수정 및 관련 서버 함수 제작
allSettled로 인한 병렬 요청에서 reissue 요청을 한번만 수행하도록 수정
WalkthroughThis PR implements server-side authentication and token management, integrates WebView OAuth token flow with cookie persistence, refactors the main page for server-side hydration, enhances UI components with Suspense boundaries, adds build analysis tooling, and updates environment configuration and metadata settings. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Browser
participant Server as Next.js Server
participant API as Backend API
Note over Client,Server: Server-side Token Refresh Flow
Client->>Server: Request Page (with accessToken cookie)
Server->>API: Fetch with Authorization Header
API-->>Server: 401 Unauthorized
Server->>Server: tokenHandler: get refreshToken
Server->>API: POST /refresh with refreshToken
API-->>Server: New accessToken, refreshToken
Server->>Server: Update cookie store
Server->>API: Retry original request
API-->>Server: Success (200)
Server->>Client: Render + Set new cookie
Note over Client: Browser updates cookies
sequenceDiagram
participant ReactNative as React Native App
participant WebView as WebView (Browser)
participant Handler as WebViewHandler
participant AuthStore as Auth Store
Note over ReactNative,WebView: WebView Token Injection Flow
ReactNative->>WebView: postMessage(TOKEN_INJECTION payload)
WebView->>Handler: Message received + parsed
Handler->>AuthStore: setAccessToken + setRefreshToken
Handler->>Handler: Mark user logged in
Handler->>ReactNative: postMessage(ACK)
Note over AuthStore: Tokens persisted in cookies
Note over ReactNative,WebView: WebView Token Reissue Flow
ReactNative->>WebView: postMessage(REISSUE_TOKEN {refreshToken})
WebView->>Handler: Message parsed
Handler->>AuthStore: Update tokens from payload
Handler->>ReactNative: postMessage(ACK)
sequenceDiagram
participant Client as Client Request
participant MainPage as app/page.tsx (Server)
participant Hydration as MainHydration
participant API as API Client
participant MainComponent as Main Component
Note over Client,MainComponent: Server-side Hydration Flow
Client->>MainPage: GET /
MainPage->>Hydration: Execute MainHydration()
Hydration->>API: Promise.allSettled([user/profile, points, goals])
API-->>Hydration: Results
Hydration->>Hydration: Map to fallback data structure
MainPage->>MainComponent: ServerAuthGuard → FallbackProvider → Main
MainComponent-->>Client: Render with pre-fetched data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
next.config.ts (1)
38-55: Guard against missingresourceQueryon the SVG rule
fileLoaderRule.resourceQueryisundefinedin the default Next 15 asset rule, so spreadingfileLoaderRule.resourceQuery.notthrows “Cannot read properties of undefined (reading 'not')” duringnext build. Please compute the exclusion array defensively before constructing the SVGR rule.- const fileLoaderRule = config.module.rules.find((rule: any) => - rule.test?.test?.(".svg"), - ); + const fileLoaderRule = config.module.rules.find((rule: any) => + rule.test?.test?.(".svg"), + ); + if (!fileLoaderRule) { + return config; + } + + const resourceQueryNot = Array.isArray(fileLoaderRule.resourceQuery?.not) + ? [...fileLoaderRule.resourceQuery.not, /url/] + : [/url/]; … - { - test: /\.svg$/i, - issuer: fileLoaderRule.issuer, - resourceQuery: { not: [...fileLoaderRule.resourceQuery.not, /url/] }, // exclude if *.svg?url - use: ["@svgr/webpack"], - }, + { + test: /\.svg$/i, + issuer: fileLoaderRule.issuer, + resourceQuery: { not: resourceQueryNot }, // exclude if *.svg?url + use: ["@svgr/webpack"], + },app/onboarding/page.tsx (1)
86-90: Inconsistent token persistence between redirect paths.The first redirect path (Lines 74-80) sets tokens to cookies, but this second redirect path (Lines 86-90) doesn't. This inconsistency could lead to missing tokens in certain scenarios.
Consider applying the same cookie-setting logic here, or refactor to a shared helper function:
// 로그인된 상태에서 goals가 있으면 onboarding 완료 처리 후 redirect (게스트가 아닌 경우만) if (isLoggedIn && goals && goals.length > 0 && !isGuest) { + if (accessToken) { + cookies.set("accessToken", accessToken); + } + if (refreshToken) { + cookies.set("refreshToken", refreshToken); + } setHasCompletedOnboarding(true); router.replace("/"); return;
🧹 Nitpick comments (19)
app/loading.tsx (1)
5-11: Remove unnecessary fragment wrapper.The fragment is unnecessary when rendering a single child element.
export default function MainLoading() { - return ( - <> - <Loading /> - </> - ); + return <Loading />; }.env.local (1)
5-8: Consider removing the commented API_URL line.Line 6 contains a commented-out API_URL that points to the Swagger docs rather than the API base. Since the active API_URL on line 5 is now correct, the commented line can be removed to reduce clutter.
Apply this diff:
API_URL=http://158.179.175.134:8080 -# API_URL=http://158.179.175.134:8080/v3/api-docs/Motimo%20API%20List NEXT_PUBLIC_FRONTEND_BASE_URL=http://localhost:3000Additionally, note that
FRONTEND_BASE_URLandNEXT_PUBLIC_FRONTEND_BASE_URLboth exist with the same value. This may be intentional (server vs. client usage), but if they serve the same purpose, consider consolidating them.components/main/GoalMenuContainer/GoalMenuContainer.tsx (2)
11-11: Clarify whether motion functionality removal is temporary or permanent.The motion/drag functionality has been disabled by commenting out the implementation. If this is a permanent change, the commented code and related unused variables should be removed entirely. If temporary, consider adding a comment explaining why.
If this is a permanent removal, apply this diff to clean up unused code:
-// import { motion, useMotionValue } from "motion/react"; - type GoalMenuInfo = Pick<GoalMenuProps, "goal" | "percentage"> & { goalId: string; };
35-56: Remove unused refs and state after disabling drag functionality.The
containerRef,contentRef,containerW,contentW, and the associateduseEffectare no longer used after commenting out the motion implementation. This dead code should be removed.Apply this diff to remove unused code:
const [selectedGoalIdx, setSelectedGoalIdx] = useState(0); const { updateGoalId } = useGoalStore(); const { openModal, closeModal } = useModal(); - const containerRef = useRef<HTMLDivElement>(null); - const contentRef = useRef<HTMLDivElement>(null); - const [containerW, setContainerW] = useState(0); - const [contentW, setContentW] = useState(0); const router = useRouter(); - // const x = useMotionValue(0); useEffect(() => { updateGoalId(goalMenuInfoList[selectedGoalIdx]?.goalId ?? null); }, [goalMenuInfoList[selectedGoalIdx]?.goalId, updateGoalId]); - useEffect(() => { - const updateSizes = () => { - if (containerRef.current && contentRef.current) { - setContainerW(containerRef.current.offsetWidth); - setContentW(contentRef.current.scrollWidth); - } - }; - - updateSizes(); - }, [goalMenuInfoList]); - const goalNum = goalMenuInfoList.length;api/getToken.ts (2)
6-6: Remove commented debug log.The commented console.log on line 6 should be removed to keep the code clean.
Apply this diff:
const cookieStore = await cookies(); const token = cookieStore.get("accessToken")?.value; - // console.log("accessToken on Server: ", token); return token; };
3-8: Consider adding explicit return type and error handling.The function would benefit from an explicit return type annotation and error handling for cases where the
cookies()API might fail.Apply this diff to improve type safety and error handling:
-const getToken = async () => { - const cookieStore = await cookies(); - const token = cookieStore.get("accessToken")?.value; - // console.log("accessToken on Server: ", token); - return token; +const getToken = async (): Promise<string | undefined> => { + try { + const cookieStore = await cookies(); + const token = cookieStore.get("accessToken")?.value; + return token; + } catch (error) { + console.error("Failed to retrieve access token:", error); + return undefined; + } };components/main/MainHeader/MainHeader.tsx (1)
17-22: Drop the unused cheer phrase queryWe still trigger
useCheerPhrase()even though its result is never rendered after the banner refactor. That’s an extra network fetch for every header mount with no payoff. Please remove the hook import/call (or wire the data into the new children) to avoid the wasted request.-import { useCheerPhrase, useMyProfile, usePoints } from "@/api/hooks"; +import { useMyProfile, usePoints } from "@/api/hooks"; … - const { data: cheerData } = useCheerPhrase(); const { data: pointData } = usePoints(); - const cheerPhrase = cheerData?.cheerPhrase ?? "";api/queries.ts (1)
46-54: Remove commented-out code.This commented code block appears to be an older version of the
subGoalTodosquery that has been replaced. Dead code should be removed to improve maintainability.Apply this diff:
- // // Sub Goal API - // subGoalTodos: (subGoalId: string | null, config?: SWRConfiguration) => - // useApiQuery( - // "세부목표Api", - // "getIncompleteOrTodayTodos", - // subGoalId ? [subGoalId] : null, - // undefined, - // config, - // ),app/sitemap.ts (1)
4-11: Consider expanding the sitemap.The sitemap currently includes only
/onboarding, but the robots.txt allows crawling of/. Consider adding other public routes (like the main page) to improve SEO coverage..env.dev (1)
5-7: Document the server vs. client environment variable distinction.The duplication between
API_URL/NEXT_PUBLIC_API_URLandFRONTEND_BASE_URL/NEXT_PUBLIC_FRONTEND_BASE_URLfollows Next.js conventions whereNEXT_PUBLIC_*variables are exposed to the browser. Consider adding a comment to clarify this distinction for maintainability.Apply this diff:
+# Server-only API URL (not exposed to browser) API_URL=http://158.179.175.134:8080 +# Client-accessible frontend base URL NEXT_PUBLIC_FRONTEND_BASE_URL=http://localhost:3000 +# Server-only frontend base URL FRONTEND_BASE_URL=http://localhost:3000components/mypage/MyPage.tsx (1)
14-14: Track or remove commented WebView integration code.The commented WebView logout integration appears to be placeholder code. If this is planned work, consider adding a TODO comment or creating a tracking issue. Otherwise, remove the commented code to reduce noise.
Would you like me to help create a tracking issue for the WebView logout integration?
Also applies to: 66-66
app/_components/ServerAuthGuard.tsx (1)
5-12: Consider token validation beyond presence check.The guard only checks for the presence of
accessTokenbut doesn't validate its expiration or format. This could allow access with expired tokens. Consider adding token validation logic or handling token refresh on the server side.app/robots.ts (1)
9-9: Remove redundant commented code.The commented
allow: "/onboarding"is redundant since Line 8 already allows"/", which includes all paths not explicitly disallowed.Apply this diff:
userAgent: "*", allow: "/", - // allow: "/onboarding", disallow: [api/getRefreshToken.ts (1)
6-6: Remove commented debug code.The commented
console.logshould be removed to keep the code clean.Apply this diff:
const token = cookieStore.get("refreshToken")?.value; - // console.log("refreshToken on Server: ", token); return token;app/onboarding/page.tsx (1)
75-75: Remove dead code.This commented assignment serves no purpose and should be removed.
Apply this diff:
if (!isGuest && accessToken) { - // const accessToken = accessToken; cookies.set("accessToken", accessToken);app/page.tsx (2)
115-145: Clean up commented code for maintainability.Lines 119-135 contain a large block of commented-out code within the Promise mapping logic. This reduces readability and makes the data flow harder to follow.
Consider removing the commented code if it's no longer needed, or extract it to a separate design document if it represents an alternative implementation approach.
177-188: Remove obsolete commented JSX.Lines 177-188 contain commented-out JSX that appears to be an old implementation. This commented code should be removed to improve maintainability.
app/layout.tsx (1)
18-27: Enhance metadata configuration and reduce duplication.The metadata additions align with the SEO objectives, but there are a few improvements to consider:
DRY violation: The
openGraph.urlat Line 25 duplicates the logic frommetadataBase. SincemetadataBaseis set, you can omitopenGraph.urlas Next.js will automatically use the base URL.Missing openGraph properties: Consider adding
title,description, andimagestoopenGraphfor richer social media previews.Localhost fallback: The fallback to
https://localhost:3000may cause issues in production if the environment variable is missing.Apply this diff to improve the metadata:
export const metadata: Metadata = { metadataBase: new URL( - process.env.NEXT_PUBLIC_FRONTEND_BASE_URL ?? "https://localhost:3000", + process.env.NEXT_PUBLIC_FRONTEND_BASE_URL || "https://motimo.com", ), title: "모티모 | 그룹 기반 목표 관리 서비스", description: "그룹 매칭을 통해 목표를 관리해요!", keywords: ["목표", "그룹", "투두", "업무", "모티모"], openGraph: { - url: process.env.NEXT_PUBLIC_FRONTEND_BASE_URL ?? "https://localhost:3000", + title: "모티모 | 그룹 기반 목표 관리 서비스", + description: "그룹 매칭을 통해 목표를 관리해요!", type: "website", }, };Note: Replace
https://motimo.comwith your actual production URL.api/service.ts (1)
138-250: Consider extracting server and client token handlers.The
tokenHandlerfunction contains complex branching logic with server/client splits throughout (Lines 147-154, 175-208, 230-237, 244-245). This makes the function difficult to test and maintain.Consider refactoring into separate handlers:
const handleServerTokenRefresh = async (refreshToken: string) => { // Lines 176-202 logic }; const handleClientTokenRefresh = async (refreshToken: string) => { // Lines 204-208 logic }; const tokenHandler = async <T, E>(...) => { return apiRes.catch(async (e) => { if (e.status === 401) { const refreshToken = typeof window === "undefined" ? await getServerRefreshToken() : getClientRefreshToken(); if (!refreshToken) { return handleMissingRefreshToken(); } const tokenRes = typeof window === "undefined" ? await handleServerTokenRefresh(refreshToken) : await handleClientTokenRefresh(refreshToken); return retryRequest(requestParams, tokenRes, apiRequest); } throw e; }); };This would improve testability and make the control flow clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
app/opengraph-image.pngis excluded by!**/*.pngcomponents/shared/public/reactions/Reaction_Best.svgis excluded by!**/*.svgcomponents/shared/public/reactions/Reaction_CheerUp.svgis excluded by!**/*.svgcomponents/shared/public/reactions/Reaction_Cool.svgis excluded by!**/*.svgcomponents/shared/public/reactions/Reaction_Good.svgis excluded by!**/*.svgcomponents/shared/public/reactions/Reaction_Love.svgis excluded by!**/*.svgpackage-lock.jsonis excluded by!**/package-lock.jsonpublic/images/Completion.svgis excluded by!**/*.svg
📒 Files selected for processing (32)
.env.dev(1 hunks).env.local(1 hunks)api/getRefreshToken.ts(1 hunks)api/getToken.ts(1 hunks)api/hooks.ts(1 hunks)api/queries.ts(1 hunks)api/service.ts(3 hunks)app/Main.tsx(1 hunks)app/_components/AuthGuard.tsx(2 hunks)app/_components/BottomSheetRenderer.tsx(2 hunks)app/_components/ServerAuthGuard.tsx(1 hunks)app/_components/WebViewHandler.tsx(1 hunks)app/details/edit/[goal_id]/page.tsx(0 hunks)app/layout.tsx(3 hunks)app/loading.tsx(1 hunks)app/onboarding/_components/LoginScreen.tsx(2 hunks)app/onboarding/page.tsx(4 hunks)app/page.tsx(1 hunks)app/robots.ts(1 hunks)app/sitemap.ts(1 hunks)components/details/ListCard/ListCard.tsx(1 hunks)components/group/index.ts(1 hunks)components/main/GoalCard/GoalCard.tsx(1 hunks)components/main/GoalDataContainer/GoalDataContainer.tsx(3 hunks)components/main/GoalMenuContainer/GoalMenuContainer.tsx(2 hunks)components/main/MainHeader/AsyncBanner.tsx(1 hunks)components/main/MainHeader/MainHeader.tsx(2 hunks)components/mypage/MyPage.tsx(2 hunks)hooks/useBottomSheet.ts(1 hunks)next.config.ts(2 hunks)package.json(4 hunks)webview.d.ts(1 hunks)
💤 Files with no reviewable changes (1)
- app/details/edit/[goal_id]/page.tsx
🧰 Additional context used
📓 Path-based instructions (3)
components/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/global-rule.mdc)
For every styling, use tailwind css.
Files:
components/main/GoalMenuContainer/GoalMenuContainer.tsxcomponents/group/index.tscomponents/mypage/MyPage.tsxcomponents/details/ListCard/ListCard.tsxcomponents/main/MainHeader/AsyncBanner.tsxcomponents/main/GoalDataContainer/GoalDataContainer.tsxcomponents/main/GoalCard/GoalCard.tsxcomponents/main/MainHeader/MainHeader.tsx
{components/**/*.{js,jsx,ts,tsx},**/*.css}
📄 CodeRabbit inference engine (.cursor/rules/global-rule.mdc)
For tokens, use tokens.css (mdc:app/tokens.css).
Files:
components/main/GoalMenuContainer/GoalMenuContainer.tsxcomponents/group/index.tscomponents/mypage/MyPage.tsxcomponents/details/ListCard/ListCard.tsxcomponents/main/MainHeader/AsyncBanner.tsxcomponents/main/GoalDataContainer/GoalDataContainer.tsxcomponents/main/GoalCard/GoalCard.tsxcomponents/main/MainHeader/MainHeader.tsx
components/**/*
📄 CodeRabbit inference engine (.cursor/rules/global-rule.mdc)
Component should be created in components/
Files:
components/main/GoalMenuContainer/GoalMenuContainer.tsxcomponents/group/index.tscomponents/mypage/MyPage.tsxcomponents/details/ListCard/ListCard.tsxcomponents/main/MainHeader/AsyncBanner.tsxcomponents/main/GoalDataContainer/GoalDataContainer.tsxcomponents/main/GoalCard/GoalCard.tsxcomponents/main/MainHeader/MainHeader.tsx
🧠 Learnings (1)
📚 Learning: 2025-07-21T07:45:37.405Z
Learnt from: CR
Repo: prography/10th-Motimo-FE PR: 0
File: .cursor/rules/global-rule.mdc:0-0
Timestamp: 2025-07-21T07:45:37.405Z
Learning: Applies to {components/**/*.{js,jsx,ts,tsx},**/*.css} : For tokens, use tokens.css (mdc:app/tokens.css).
Applied to files:
app/layout.tsx
🧬 Code graph analysis (8)
app/Main.tsx (2)
app/_components/AuthGuard.tsx (1)
AuthGuard(14-49)api/service.ts (1)
tokens(110-113)
app/_components/ServerAuthGuard.tsx (1)
.storybook/nextjs-mock.ts (1)
redirect(36-38)
app/page.tsx (3)
api/service.ts (2)
api(255-255)popTokens(124-128)api/queries.ts (1)
queryArgs(4-168)app/Main.tsx (1)
Main(20-56)
components/main/MainHeader/AsyncBanner.tsx (2)
api/service.ts (1)
api(255-255)utils/calcLeftDay.ts (1)
calcLeftDay(1-12)
components/main/GoalDataContainer/GoalDataContainer.tsx (2)
utils/calcLeftDay.ts (1)
calcLeftDay(1-12)api/hooks.ts (1)
useGoalDetail(175-175)
components/main/MainHeader/MainHeader.tsx (1)
api/hooks.ts (2)
useCheerPhrase(190-190)usePoints(189-189)
app/_components/AuthGuard.tsx (1)
api/service.ts (1)
tokens(110-113)
api/service.ts (4)
api/generated/motimo/Api.ts (1)
HttpClient(940-1135)api/getToken.ts (1)
getToken(10-10)api/getRefreshToken.ts (1)
getRefreshToken(10-10).storybook/nextjs-mock.ts (1)
redirect(36-38)
🪛 dotenv-linter (4.0.0)
.env.local
[warning] 5-5: [UnorderedKey] The API_URL key should go before the NEXT_PUBLIC_API_URL key
(UnorderedKey)
[warning] 7-7: [UnorderedKey] The NEXT_PUBLIC_FRONTEND_BASE_URL key should go before the SWAGGER_URL key
(UnorderedKey)
[warning] 8-8: [UnorderedKey] The FRONTEND_BASE_URL key should go before the NEXT_PUBLIC_API_URL key
(UnorderedKey)
.env.dev
[warning] 5-5: [UnorderedKey] The API_URL key should go before the NEXT_PUBLIC_API_URL key
(UnorderedKey)
[warning] 6-6: [UnorderedKey] The NEXT_PUBLIC_FRONTEND_BASE_URL key should go before the SWAGGER_URL key
(UnorderedKey)
[warning] 7-7: [UnorderedKey] The FRONTEND_BASE_URL key should go before the NEXT_PUBLIC_API_URL key
(UnorderedKey)
🔇 Additional comments (9)
components/group/index.ts (1)
9-9: The commented export does not break existing internal usage—all current imports use direct paths.ReactionModal is actively used in
components/group/groupRoom/GroupChatRoom/GroupChatRoom.tsxandcomponents/group/ReactionModal.stories.tsx, but both import directly (not via the barrel export). Commenting out the index.ts export is safe for the codebase but is a breaking change only for external consumers. If this is intentional API refactoring, consider either removing the line entirely or adding a deprecation comment explaining why.app/loading.tsx (1)
1-1: "use client" is necessary and correct. The Loading component uses Lottie-react, a client-side animation library that renders animations in the browser. Sinceapp/loading.tsxis a special Next.js App Router file that displays UI during data fetching, it must be a client component to properly render client-side animations within the Suspense boundary.components/main/GoalCard/GoalCard.tsx (1)
198-201: LGTM - Debug cleanup.Commenting out the debug log is appropriate for production code.
hooks/useBottomSheet.ts (1)
107-107: LGTM - Debug cleanup.Removing the debug log is appropriate.
components/details/ListCard/ListCard.tsx (1)
110-110: Verify that zIndex should be a string.The
zIndexis set as a string"40"rather than a number. In React inline styles, numeric properties likezIndexare typically numbers unless there's a specific requirement for a string value.If no specific reason exists for using a string, consider applying this diff:
- bottomSheetFixerStyle: { bottom: "0px", zIndex: "40" }, + bottomSheetFixerStyle: { bottom: "0px", zIndex: 40 },app/layout.tsx (1)
38-44: Verify font preload removal impact.The font preload link has been commented out. While this may reduce initial HTML size slightly, it can impact font loading performance and may cause FOUT (Flash of Unstyled Text).
Please verify:
- Was this removed intentionally as part of the optimization mentioned in the PR objectives?
- Have you tested the impact on CLS (Cumulative Layout Shift) and font rendering?
If the preload was removed for optimization, consider using
font-display: swapin your font-face declaration (which you already have withdisplay: "swap"at Line 13) as a sufficient alternative. Otherwise, consider restoring the preload for better performance.api/service.ts (3)
110-136: Well-designed token storage for RSC environment.The token storage utilities using React 19's
cache()API correctly handle server-side request-scoped storage and race condition prevention for parallel requests. TherefreshTokenPromisepattern ensures only one token refresh happens even when multiple requests fail simultaneously.Based on library documentation.
198-240: Remove colons from cookie key names in AuthGuard.tsx.The review comment identifies a real problem—tokens not persisting—but diagnoses it incorrectly. The current architecture does persist refreshed tokens to cookies via the AuthGuard component. However, there's a bug at lines 37 and 40 where cookies are set with incorrect key names:
// Current (incorrect): Cookies.set("accessToken:", reissuedTokens.accessToken); // Key has colon Cookies.set("refreshToken:", reissuedTokens.refreshToken); // Key has colon // Should be: Cookies.set("accessToken", reissuedTokens.accessToken); Cookies.set("refreshToken", reissuedTokens.refreshToken);The server's
getRefreshToken()andgetToken()functions read cookies with names"refreshToken"and"accessToken"(without colons). This mismatch means refreshed tokens are written to cookies with wrong keys and never read on subsequent requests, causing stale tokens to persist.The fix is to remove the colons from the cookie key names in AuthGuard, not to add server-side cookie persistence as suggested in the review.
Likely an incorrect or invalid review comment.
159-162: Remove false claim about line 162 being unreachable; clarify server vs. client-side behavior.The original concern contains an error: Line 162 is NOT unreachable. When
typeof window !== "undefined"(client-side), the redirect condition is false and execution reaches thethrow new Error("no refresh token")statement, which is the correct behavior for client-side error handling.The server-side redirect at line 160 is appropriate for Server Components, and the client-side error throw at line 162 properly handles the client-side case. The code structure correctly bifurcates based on execution environment—no changes needed.
| // export const useSubGoalTodos = useQuery.subGoalTodos; | ||
| export const useCompletedGoals = useQuery.completedGoals; | ||
| // export const useSubGoalTodos = useQuery.subGoalTodos; | ||
| export const useSubGoalTodos = useQuery.subGoalTodos; |
There was a problem hiding this comment.
Remove duplicate commented exports.
The commented-out export statements on lines 179 and 181 are redundant with the active export on line 182, reducing code clarity.
Apply this diff to remove the duplicate commented lines:
export const useGoalsNotInGroup = useQuery.goalsNotInGroup;
-// export const useSubGoalTodos = useQuery.subGoalTodos;
export const useCompletedGoals = useQuery.completedGoals;
-// export const useSubGoalTodos = useQuery.subGoalTodos;
export const useSubGoalTodos = useQuery.subGoalTodos;📝 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.
| // export const useSubGoalTodos = useQuery.subGoalTodos; | |
| export const useCompletedGoals = useQuery.completedGoals; | |
| // export const useSubGoalTodos = useQuery.subGoalTodos; | |
| export const useSubGoalTodos = useQuery.subGoalTodos; | |
| export const useCompletedGoals = useQuery.completedGoals; | |
| export const useSubGoalTodos = useQuery.subGoalTodos; |
🤖 Prompt for AI Agents
In api/hooks.ts around lines 179 to 182, there are duplicate commented export
lines (the commented useSubGoalTodos on lines 179 and 181) that are redundant
with the active export on line 182; remove the two commented-out export
statements so only the active export remains, keeping file formatting intact.
| baseUrl: (() => { | ||
| return process.env.API_URL || ""; | ||
| // return process.env.NEXT_PUBLIC_API_URL || process.env.API_URL; | ||
| })(), |
There was a problem hiding this comment.
Verify API_URL availability in client environment.
The baseUrl now only uses API_URL (Line 19), with NEXT_PUBLIC_API_URL commented out. In Next.js, environment variables without the NEXT_PUBLIC_ prefix are only available on the server side.
This will cause client-side API calls to fail because process.env.API_URL will be undefined in the browser.
Apply this diff to fix:
baseUrl: (() => {
- return process.env.API_URL || "";
- // return process.env.NEXT_PUBLIC_API_URL || process.env.API_URL;
+ return process.env.NEXT_PUBLIC_API_URL || process.env.API_URL || "";
})(),This ensures the client uses NEXT_PUBLIC_API_URL and the server can use API_URL (since Next.js replaces process.env references at build time based on the environment).
📝 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.
| baseUrl: (() => { | |
| return process.env.API_URL || ""; | |
| // return process.env.NEXT_PUBLIC_API_URL || process.env.API_URL; | |
| })(), | |
| baseUrl: (() => { | |
| return process.env.NEXT_PUBLIC_API_URL || process.env.API_URL || ""; | |
| })(), |
🤖 Prompt for AI Agents
In api/service.ts around lines 18 to 21, the baseUrl uses process.env.API_URL
which is not available in the browser; change the selection to prefer
process.env.NEXT_PUBLIC_API_URL for client-side builds and fall back to
process.env.API_URL for server-side usage (e.g. set baseUrl to
process.env.NEXT_PUBLIC_API_URL || process.env.API_URL) so client bundles get a
NEXT_PUBLIC_ value while servers can still use the non-public variable.
| if (timer) { | ||
| clearTimeout(timer); | ||
| rejectTimer("debouncing"); | ||
| rejectTimer(); | ||
| // rejectTimer("debouncing"); | ||
| } |
There was a problem hiding this comment.
Protect against undefined rejectTimer invocation.
At Line 75, rejectTimer() is called, but rejectTimer is only defined inside the Promise constructor (Line 79). If a request is made before the Promise is created, or if there's any race condition, this could throw an error.
Apply this diff to add a safety check:
if (timer) {
clearTimeout(timer);
- rejectTimer();
- // rejectTimer("debouncing");
+ if (rejectTimer) {
+ rejectTimer();
+ }
}Alternatively, initialize rejectTimer at the function scope:
const timeLimit = 300;
const timerDictionary: { [apiFullUrl: string]: number | undefined } = {};
- let rejectTimer: (reason?: any) => void;
+ let rejectTimer: ((reason?: any) => void) | undefined;📝 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.
| if (timer) { | |
| clearTimeout(timer); | |
| rejectTimer("debouncing"); | |
| rejectTimer(); | |
| // rejectTimer("debouncing"); | |
| } | |
| if (timer) { | |
| clearTimeout(timer); | |
| if (rejectTimer) { | |
| rejectTimer(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In api/service.ts around lines 73-77, the code calls rejectTimer() which may be
undefined and can throw if invoked before the Promise sets it; either guard the
call with a check (if (typeof rejectTimer === "function") rejectTimer(); ) or
initialize rejectTimer in the outer function scope to a no-op before the Promise
(e.g. let rejectTimer: () => void = () => {}; then assign the real reject inside
the Promise) so clearTimeout(timer) can safely attempt to cancel the pending
promise without risking an undefined-call error.
| let refreshTokenPromise = getRefreshTokenPromise(); | ||
|
|
||
| if (!refreshTokenPromise) { | ||
| refreshTokenPromise = api.authController | ||
| .reissue({ | ||
| refreshToken: refreshToken, | ||
| }) | ||
| .then((newTokens) => { | ||
| const nonNullableNewTokens = { | ||
| accessToken: "", | ||
| refreshToken: "", | ||
| ...newTokens, | ||
| }; | ||
| pushTokens(nonNullableNewTokens); | ||
| return nonNullableNewTokens; | ||
| }) | ||
| .finally(() => { | ||
| setRefreshTokenPromise(undefined); | ||
| }); | ||
| setRefreshTokenPromise(refreshTokenPromise); | ||
| } | ||
|
|
||
| tokenRes = await refreshTokenPromise; | ||
| pushTokens({ | ||
| accessToken: tokenRes.accessToken, | ||
| refreshToken: tokenRes.refreshToken, | ||
| }); |
There was a problem hiding this comment.
Add timeout to token refresh promise.
The token refresh promise (Lines 176-196) has no timeout, which could cause the request to hang indefinitely if the auth API is slow or unresponsive. This is especially problematic in server-side rendering where hung requests block server resources.
Consider wrapping the refresh promise with a timeout:
const REFRESH_TIMEOUT = 10000; // 10 seconds
const createRefreshPromise = () => {
return Promise.race([
api.authController.reissue({ refreshToken }),
new Promise((_, reject) =>
setTimeout(() => reject(new Error('Token refresh timeout')), REFRESH_TIMEOUT)
)
]).then((newTokens) => {
// ... existing logic
});
};🤖 Prompt for AI Agents
In api/service.ts around lines 176 to 202, the token refresh promise has no
timeout and can hang indefinitely; wrap the reissue call in a timed promise (use
a REFRESH_TIMEOUT constant, e.g., 10000 ms) by using Promise.race between
api.authController.reissue({ refreshToken }) and a timeout promise that rejects
with a clear Error like "Token refresh timeout"; ensure the timed promise
preserves the existing .then logic that normalizes and pushes new tokens, still
calls setRefreshTokenPromise(undefined) in finally, and that only successful
resolution results in pushTokens — propagate rejection so callers can handle
timeout errors.
| const nonNullableNewTokens = { | ||
| accessToken: "", | ||
| refreshToken: "", | ||
| ...newTokens, | ||
| }; | ||
| pushTokens(nonNullableNewTokens); | ||
| return nonNullableNewTokens; |
There was a problem hiding this comment.
Unsafe default values for token response.
Lines 184-188 create nonNullableNewTokens with default empty strings for accessToken and refreshToken. If the API returns a response without these fields, this will push invalid empty tokens instead of failing fast.
Apply this diff to validate tokens properly:
- .then((newTokens) => {
- const nonNullableNewTokens = {
- accessToken: "",
- refreshToken: "",
- ...newTokens,
- };
- pushTokens(nonNullableNewTokens);
- return nonNullableNewTokens;
- })
+ .then((newTokens) => {
+ if (!newTokens?.accessToken || !newTokens?.refreshToken) {
+ throw new Error("Invalid token response from reissue API");
+ }
+ pushTokens({
+ accessToken: newTokens.accessToken,
+ refreshToken: newTokens.refreshToken,
+ });
+ return newTokens;
+ })📝 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 nonNullableNewTokens = { | |
| accessToken: "", | |
| refreshToken: "", | |
| ...newTokens, | |
| }; | |
| pushTokens(nonNullableNewTokens); | |
| return nonNullableNewTokens; | |
| if (!newTokens?.accessToken || !newTokens?.refreshToken) { | |
| throw new Error("Invalid token response from reissue API"); | |
| } | |
| pushTokens({ | |
| accessToken: newTokens.accessToken, | |
| refreshToken: newTokens.refreshToken, | |
| }); | |
| return newTokens; |
🤖 Prompt for AI Agents
In api/service.ts around lines 184 to 190, the code currently constructs
nonNullableNewTokens with empty-string defaults and pushes them, which can hide
missing token fields; change this to validate that newTokens.accessToken and
newTokens.refreshToken exist and are non-empty (e.g., if either is missing or
falsy, throw or return a clear error instead of substituting empty strings),
only call pushTokens when both tokens are present, and return the validated
tokens; do not silently default to empty strings.
| if (!isGuest && accessToken) { | ||
| // const accessToken = accessToken; | ||
| cookies.set("accessToken", accessToken); | ||
| } | ||
| if (!isGuest && refreshToken) { | ||
| cookies.set("refreshToken", refreshToken); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Security concern: Client-side cookie storage without httpOnly flag.
Using js-cookie to store authentication tokens makes them accessible to JavaScript, creating an XSS vulnerability. Auth tokens should be set server-side with httpOnly and secure flags. Consider moving token persistence to the server or using secure cookie options.
Additionally, verify whether this cookie setting is necessary, as the server-side utilities (api/getToken.ts, api/getRefreshToken.ts) already handle server-side cookie access.
I'll help verify this security concern. Let me gather information about the codebase implementation.
🏁 Script executed:
#!/bin/bash
# Check the imports and implementation in app/onboarding/page.tsx
echo "=== Checking app/onboarding/page.tsx for cookie handling ==="
cat app/onboarding/page.tsx | head -30
echo -e "\n=== Lines 74-80 context ==="
sed -n '60,90p' app/onboarding/page.tsx
echo -e "\n=== Search for cookie library imports ==="
rg -n "import.*cookie" app/onboarding/page.tsx
echo -e "\n=== Search for all cookie-related code in file ==="
rg -n -C5 "cookies\.set" app/onboarding/page.tsxLength of output: 2593
🏁 Script executed:
#!/bin/bash
# Check server-side utilities
echo "=== Checking api/getToken.ts ==="
find . -name "getToken.ts" -o -name "getToken.js" | head -5
echo -e "\n=== Checking api/getRefreshToken.ts ==="
find . -name "getRefreshToken.ts" -o -name "getRefreshToken.js" | head -5
echo -e "\n=== Search for server-side cookie handling ==="
rg -n "cookies\.set" --type=ts --type=js -g "!node_modules" | head -20Length of output: 318
🏁 Script executed:
#!/bin/bash
# Check package.json for cookie library
echo "=== Checking dependencies for cookie libraries ==="
cat package.json | grep -A 20 -B 2 "dependencies"Length of output: 661
🏁 Script executed:
#!/bin/bash
# Check server-side utilities
echo "=== Checking api/getToken.ts ==="
cat api/getToken.ts
echo -e "\n=== Checking api/getRefreshToken.ts ==="
cat api/getRefreshToken.ts
echo -e "\n=== Check for server-side Set-Cookie usage ==="
rg -n "Set-Cookie" --type=ts --type=js -g "!node_modules"
echo -e "\n=== Check for cookies from next/headers ==="
rg -n "from.*next/headers" --type=ts --type=js -g "!node_modules" | head -10Length of output: 984
🏁 Script executed:
#!/bin/bash
# Check useAuthStore to understand token flow
echo "=== Checking useAuthStore ==="
find . -name "*useAuthStore*" -o -name "*authStore*" | grep -v node_modules
# Get the store file
store_file=$(find . -name "*useAuthStore*" -o -name "*authStore*" | grep -v node_modules | head -1)
if [ -n "$store_file" ]; then
echo -e "\n=== Content of auth store ==="
cat "$store_file"
fiLength of output: 3676
🏁 Script executed:
#!/bin/bash
# Check if js-cookie can set httpOnly
echo "=== js-cookie package info ==="
cd node_modules/js-cookie 2>/dev/null && cat README.md | head -50 || echo "Cannot access node_modules"Length of output: 57
🏁 Script executed:
#!/bin/bash
# Find where tokens are initially set/obtained
echo "=== Search for setAccessToken calls ==="
rg -n "setAccessToken" --type=ts --type=tsx -C3
echo -e "\n=== Search for login/auth endpoints ==="
find . -path "*/api/*" -name "*login*" -o -path "*/api/*" -name "*auth*" | grep -v node_modules | head -10
echo -e "\n=== Search for Set-Cookie or response headers ==="
rg -n "headers\|Set-Cookie" --type=ts -g "!node_modules" | head -20Length of output: 159
🏁 Script executed:
#!/bin/bash
# Check for authentication flow
echo "=== Check api/service.ts for token handling ==="
cat api/service.ts | head -100Length of output: 2924
Remove client-side cookie storage and ensure tokens are set server-side with httpOnly flag.
The concern is valid. Using js-cookie to store authentication tokens on the client creates an XSS vulnerability—these tokens remain accessible to JavaScript. The httpOnly flag cannot be set from client-side code (only via server-side Set-Cookie headers).
Additionally, this cookie storage is redundant: tokens are already persisted to localStorage automatically via the auth store's persist middleware. Remove the cookies.set() calls at lines 76 and 79. Instead, ensure tokens are set server-side during authentication with proper httpOnly and secure flags, then read via server-side utilities that use next/headers.
🤖 Prompt for AI Agents
In app/onboarding/page.tsx around lines 74 to 80, remove the client-side
cookies.set("accessToken", accessToken) and cookies.set("refreshToken",
refreshToken") calls (lines 76 and 79) because setting httpOnly cookies from
client-side is impossible and creates XSS risk—tokens are already persisted
client-side via the auth store persist middleware; instead ensure authentication
endpoints set tokens server-side using Set-Cookie with httpOnly, secure (and
SameSite as appropriate) flags, and read those cookies server-side using
next/headers or other server utilities when needed.
| // const cheerRequest = api.응원Api.getCheerPhrase({ | ||
| // next: { revalidate: 3600 * 12 }, | ||
| // }); | ||
| const pointRequest = api.포인트Api.getPoint(); | ||
| const goalsRequest = api.목표Api.getGoalList(); | ||
|
|
||
| const profileKey = unstable_serialize(queryArgs.myProfile().slice(0, 2)); | ||
| const cheerKey = unstable_serialize(queryArgs.cheerPhrase().slice(0, 2)); | ||
| const pointKey = unstable_serialize(queryArgs.points().slice(0, 2)); | ||
| const goalsKey = unstable_serialize(queryArgs.goals().slice(0, 2)); |
There was a problem hiding this comment.
Remove unused cheerKey or restore cheerRequest.
The cheerKey is defined at Line 103 but the corresponding cheerRequest is commented out at Lines 96-98. This creates an unused variable and will result in an incorrect fallback mapping (the indices won't align with the keyList).
Apply this diff to remove the unused key:
const userRequest = api.사용자Api.getMyProfile();
- // const cheerRequest = api.응원Api.getCheerPhrase({
- // next: { revalidate: 3600 * 12 },
- // });
const pointRequest = api.포인트Api.getPoint();
const goalsRequest = api.목표Api.getGoalList();
const profileKey = unstable_serialize(queryArgs.myProfile().slice(0, 2));
- const cheerKey = unstable_serialize(queryArgs.cheerPhrase().slice(0, 2));
const pointKey = unstable_serialize(queryArgs.points().slice(0, 2));
const goalsKey = unstable_serialize(queryArgs.goals().slice(0, 2));
- const keyList = [profileKey, cheerKey, pointKey, goalsKey];
+ const keyList = [profileKey, pointKey, goalsKey];Alternatively, if cheerPhrase data is needed, uncomment cheerRequest and add it back to the Promise.allSettled array at Line 112.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/page.tsx around lines 96 to 105, there is an unused cheerKey because the
cheerRequest call is commented out; either remove the cheerKey and its usage so
the keys align with pointRequest and goalsRequest, or restore the cheerRequest
(uncomment the api.응원Api.getCheerPhrase call) and add cheerRequest back into the
Promise.allSettled array at the corresponding position (line ~112) so the
request, keyList, and result indices remain aligned; update imports/usages
accordingly and run typecheck to ensure no unused variable warnings remain.
| const fallback: Record<string, NonNullable<typeof initData>[number]> = | ||
| initData | ||
| ? initData.reduce( | ||
| (acc, initRes, idx) => { | ||
| if (!initRes) return acc; | ||
| return { ...acc, [keyList[idx]]: initRes }; | ||
| }, | ||
| {} as Record<string, (typeof initData)[number]>, | ||
| ) | ||
| : {}; |
There was a problem hiding this comment.
Add error boundary or fallback UI for failed data fetches.
The fallback object construction filters out undefined results, but if all requests fail, the fallback will be an empty object. This could lead to client-side fetches without server-hydrated data, potentially causing loading states or errors.
Consider one of the following approaches:
- Add an error boundary around the Main component to handle hydration failures gracefully
- Provide default/empty data structures for each key when the request fails
- Add conditional rendering to show a loading or error state when critical data is missing
Example with default values:
const fallback: Record<string, NonNullable<typeof initData>[number]> =
initData
? initData.reduce(
(acc, initRes, idx) => {
return {
...acc,
[keyList[idx]]: initRes ?? getDefaultForKey(keyList[idx])
};
},
{} as Record<string, (typeof initData)[number]>,
)
: getDefaultFallbackData();| export default function sitemap(): MetadataRoute.Sitemap { | ||
| return [ | ||
| { | ||
| url: `${process.env.NEXT_PUBLIC_FRONTEND_BASE_URL}/onboarding`, |
There was a problem hiding this comment.
Add fallback for undefined environment variable.
If NEXT_PUBLIC_FRONTEND_BASE_URL is undefined, this will construct an invalid URL starting with "undefined". The robots.ts file (Line 21) includes a fallback to http://localhost:3000.
Apply this diff:
- url: `${process.env.NEXT_PUBLIC_FRONTEND_BASE_URL}/onboarding`,
+ url: `${process.env.NEXT_PUBLIC_FRONTEND_BASE_URL ?? "http://localhost:3000"}/onboarding`,📝 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.
| url: `${process.env.NEXT_PUBLIC_FRONTEND_BASE_URL}/onboarding`, | |
| url: `${process.env.NEXT_PUBLIC_FRONTEND_BASE_URL ?? "http://localhost:3000"}/onboarding`, |
🤖 Prompt for AI Agents
In app/sitemap.ts around line 6, the URL uses
process.env.NEXT_PUBLIC_FRONTEND_BASE_URL without a fallback which can produce
"undefined" in the URL; change the expression to use the same fallback as
robots.ts (e.g., use the env var if defined otherwise 'http://localhost:3000')
so the constructed URL becomes valid when the environment variable is missing.
| interface Window { | ||
| ReactNativeWebView: ReactNativeWebView; | ||
| } |
There was a problem hiding this comment.
Make the injected WebView bridge optional
Declaring window.ReactNativeWebView as always present lets code call window.ReactNativeWebView.postMessage without guards, but in non-WebView browsers that property is undefined, causing crashes at runtime. Mark it optional so TypeScript reflects reality and forces callers to null-check before sending messages.
- ReactNativeWebView: ReactNativeWebView;
+ ReactNativeWebView?: ReactNativeWebView;📝 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.
| interface Window { | |
| ReactNativeWebView: ReactNativeWebView; | |
| } | |
| interface Window { | |
| ReactNativeWebView?: ReactNativeWebView; | |
| } |
🤖 Prompt for AI Agents
In webview.d.ts around lines 6 to 8, change the Window interface declaration so
ReactNativeWebView is optional (e.g., ReactNativeWebView?: ReactNativeWebView)
to reflect that it may be undefined in non-WebView contexts; update call sites
to guard against undefined (use optional chaining or explicit null-checks)
before calling postMessage.
A/S 작업입니당
@coderabbitai ignore