Update after demo: 추가 최적화 및 기타 수정 #58
Conversation
포크한 레포에 작동 테스틀 위해 연결 스토리북 형태도 세팅함
구조 수정 및 tailwind prefix에 맞게 mapping
이전 패키지 버전에선 빌드가 안 됐음.
httpClient에서 게스트모드에서도 format을 가져서 api 콜을 할 수 있도록 코드 수정
DoneItemDetail에서 useGoalDetail훅을 사용하는데, 이는 스토리북에서 사용하려면 따로 render에서 불러와야 함. 그러나 컴포넌트 내부에서 사용되지 않아 주석처리함.
새로고침 막고, mutate처리 및 뒤로가기 정상화
todo 개수 0이 아닌 경우에 동작하도록 수정
api 경로에 따라 debounce하도록 수정 및 시간 제한 수정
기존에 수도 클래스로 적용하면 씹히는 경우가 있어서 props를 직접 사용하는 방식으로 수정
5개 넘어야 보이도록
goalId 동일한 것으로 잡도록 수정 dueDate도 계산해서 추가
게스트 모드라면 쿼리 키 다르게 해 refetch 유도 게스트 모드 기록 체크 및 기록이 있다면 온보딩 생략 중복되는 closeMSW함수 제거
WalkthroughImplements per-URL request debouncing (300ms), unified guest/no-token JSON requests, and 401-driven token refresh in the API layer; adjusts SWR keys (guest-aware and path-first), adds optimistic updates and animations to todo list UI, expands guest onboarding detection via IndexedDB, and minor MSW/mocks updates and checkbox styling refactor. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI
participant ApiClient as ApiClient (debounce+auth)
participant Server
participant Auth as Auth Store
Note over ApiClient: Per-URL debounce (300ms)
UI->>ApiClient: request(path, params)
ApiClient-->>ApiClient: debounce by URL key
ApiClient->>Server: HTTP request (JSON-only if guest/no token)
Server-->>ApiClient: 200/401 response
alt 401 Unauthorized
ApiClient->>Server: POST /auth/reissue (refreshToken)
Server-->>ApiClient: 200 new tokens / 401
alt refresh success
ApiClient->>Auth: update access/refresh tokens
ApiClient->>Server: retry original request
Server-->>ApiClient: 200 response
else refresh missing/fails
ApiClient->>Auth: logout
ApiClient-->>UI: throw 401
end
else OK
ApiClient-->>UI: data
end
sequenceDiagram
autonumber
participant Onboarding as OnboardingPage
participant Auth as Auth Store
participant IDB as IndexedDB (guest DB)
Onboarding->>Auth: read isLoggedIn, isGuest, hasCompletedOnboarding
alt LoginScreen "Browse" (guest)
LoginScreen->>IDB: list DBs
IDB-->>LoginScreen: includes DB_NAME?
alt DB exists
LoginScreen->>Auth: setHasCompletedOnboarding(true)
end
LoginScreen->>Auth: login() with isGuest=true
end
Onboarding-->>Onboarding: if hasCompletedOnboarding && isLoggedIn
Onboarding-->>Onboarding: redirect to app
sequenceDiagram
autonumber
participant List as ListCard
participant SWR as SWR Cache
participant API as API
List->>SWR: useSubGoalTodosAllInfinite()
SWR-->>List: data, isLoading, isValidating
Note over List: Checked toggle (optimistic)
List->>SWR: mutate(makeSubgoalInfiniteOptimisticData(todoId), {populateCache:false, revalidate:false, rollbackOnError:true})
List->>API: onTodoCheck(todoId)
API-->>List: success or error
alt error
SWR-->>List: rollback applied automatically
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
mocks/guestMode/handlers.ts (1)
139-156: Prevent client-supplied body from overriding server-generated id; simplify dueDate defaulting.Spreading body after id lets a malicious/buggy client override newGoal.id, creating referential mismatch with subGoals (which use goalId). Also simplify dueDate handling.
Apply this diff:
- const body = (await request.json()) as GoalCreateRq; - const goalId = genId(); - - // dueDate처리 - const dueDate = new Date(new Date().setMonth(new Date().getMonth() + 3)); - - const newGoal = { - id: goalId, - ...{ - dueDate: body.dueDate - ? undefined - : date2StringWithSpliter(dueDate, "-"), - }, - ...body, - createdAt: new Date().toISOString(), - } as any; - await dbService.add<DBGoal>("goals", newGoal); + const body = (await request.json()) as GoalCreateRq; + const goalId = genId(); + + // dueDate 기본값: 3개월 후 (yyyy-mm-dd) + const base = new Date(); + base.setMonth(base.getMonth() + 3); + const defaultDue = date2StringWithSpliter(base, "-"); + + const newGoal: DBGoal = { + ...body, + id: goalId, + dueDate: body.dueDate ?? defaultDue, + createdAt: new Date().toISOString(), + } as any; + // id를 고정하려면 put을 사용해 의도 명확화 + await dbService.put("goals", newGoal);api/useApiQuery.ts (1)
25-35: Keep SWR key as an array and makeisGuestreactive to avoid cache collisions/stale keys
${key}, guestcoerces the array key to a string; this mixes key types (array vs string) and risks collisions (e.g., objects stringify to[object Object]).- Using
useAuthStore.getState()is non-reactive; SWR key won’t update whenisGuestchanges.Apply:
- // SWR 키 생성: swrKey가 제공되면 사용, 아니면 [apiGroup, method, ...params] 형태로 생성 - const key = - swrKey || (params ? [apiGroup, method, ...params] : [apiGroup, method]); + // SWR 키 생성: 배열 형태 유지 + const baseKey = + swrKey ?? (params ? [apiGroup, method, ...params] : [apiGroup, method]); + // Reactively subscribe to isGuest + const isGuest = useAuthStore((s) => s.isGuest);- // params가 null이면 요청하지 않음 (조건부 fetching) - params === null - ? null - : useAuthStore.getState().isGuest - ? `${key}, guest` - : key, + // params가 null이고 swrKey도 없으면 요청하지 않음 (조건부 fetching) + (params === null && !swrKey) + ? null + : (isGuest ? [...baseKey, "guest"] : baseKey),app/onboarding/_components/LoginScreen.tsx (1)
53-60: Stop logging OAuth tokens and sensitive values to consoleAccess/refresh tokens and raw OAuth params are being logged. This is a production PII/security risk.
Apply one of:
- Remove these logs entirely, or
- Gate them behind a dev check and avoid printing raw token values.
Suggested minimal change (sanitize + dev-only):
- console.log("- access_token:", accessTokenFromUrl); - console.log("- refresh_token:", refreshTokenFromUrl); + if (process.env.NODE_ENV !== "production") { + console.log("- access_token: [redacted]", Boolean(accessTokenFromUrl)); + console.log("- refresh_token: [redacted]", Boolean(refreshTokenFromUrl)); + }- console.log("✅ Access Token 저장됨:", accessTokenFromUrl); + if (process.env.NODE_ENV !== "production") { + console.log("✅ Access Token 저장됨: [redacted]"); + }- console.log("✅ Refresh Token 저장됨:", refreshTokenFromUrl); + if (process.env.NODE_ENV !== "production") { + console.log("✅ Refresh Token 저장됨: [redacted]"); + }- console.log("- access_token:", authState.accessToken); - console.log("- refresh_token:", authState.refreshToken); + if (process.env.NODE_ENV !== "production") { + console.log("- access_token: [redacted]", Boolean(authState.accessToken)); + console.log("- refresh_token: [redacted]", Boolean(authState.refreshToken)); + }Also applies to: 97-107, 114-120
hooks/queries/useSubGoalTodosInfiniites.ts (1)
130-132: Avoid defaulting missing dates tonew Date().This can mislead UI/logic (an undated todo looks like “today”). Prefer
undefinedand let the view decide.Apply:
- targetDate: pageContent.date ? new Date(pageContent.date) : new Date(), + targetDate: pageContent.date ? new Date(pageContent.date) : undefined,
🧹 Nitpick comments (14)
components/shared/Checkbox/Checkbox.tsx (3)
12-16: Place props.className last and drop redundant nested templateAllow consumer overrides to win by appending props.className at the end; also remove the unnecessary nested template literal for readability.
- ${`appearance-none w-4 h-4 relative rounded overflow-hidden - hover:outline-Color-gray-40 - - - ${props.className ?? ""}`} + appearance-none w-4 h-4 relative rounded overflow-hidden + hover:outline-Color-gray-40And append props.className at the end of the class block (see next comment’s diff or add right before the closing backtick):
+ ${props.className ?? ""}
24-24: Outline utilities likely invalid; prefer supported outline/ring utilitiesoutline-[1.50px] and outline-offset-[-1.50px] are fragile: the former may set the shorthand outline without style, and negative offset is inconsistently supported. Use explicit outline width/offset or ring utilities.
- : `bg-background-alternative outline-[1.50px] outline-offset-[-1.50px] outline-Color-gray-20` + : `bg-background-alternative outline outline-1 outline-offset-1 outline-Color-gray-20`Alternatively:
+ ring-1 ring-Color-gray-20 ring-offset-1
10-26: Minor: simplify class composition with a helperUsing clsx/twMerge will dedupe conflicting classes and keep this readable as variants grow.
Example:
import { twMerge } from "tailwind-merge"; import clsx from "clsx"; <input className={twMerge( clsx( "shrink-0 appearance-none w-4 h-4 relative rounded overflow-hidden", "hover:outline-Color-gray-40", "bg-background-alternative", "checked:bg-center checked:bg-background-strong checked:bg-[url(\"data:image/svg+xml,%3Csvg%20width%3D%2214%22%20height%3D%2214%22%20viewBox%3D%220%200%2014%2014%22%20fill%3D%22none%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%3E%3Cpath%20d%3D%22M3.58325%207.27735L6.08343%209.77734L11.0833%204.77734%22%20stroke%3D%22white%22%20stroke-width%3D%221.5%22%20stroke-linecap%3D%22round%22%20stroke-linejoin%3D%22round%22%2F%3E%3C%2Fsvg%3E\")]", "checked:outline-0 checked:hover:bg-Color-primary-70", "focus-visible:outline-2 focus-visible:outline-Color-primary-70", props.className ) )} type="checkbox" {...props} />mocks/index.ts (1)
50-58: Remove commented-out closeMSW to avoid drift/confusion.This duplicates stopMsw and may confuse future maintainers. Prefer deleting or wiring it up with a single source of truth (stopMsw).
Apply this diff to remove the commented block:
-// export async function closeMSW() { -// if (typeof window === "undefined") { -// const { server } = await import("../mocks/server"); -// server.close(); -// } else { -// const { worker } = await import("../mocks/browser"); -// worker.stop(); -// } -// }components/_mocks/MSWComponent.tsx (2)
8-15: Make init idempotent against quick re-renders by setting the ref before awaiting.Setting hasInitialized after await can still race under rapid mounts. Flip it before the async import to harden idempotency.
Apply this diff:
useEffect(() => { if (hasInitialized.current) return; - - const init = async () => { - const { initMsw } = await import("../../mocks/index"); - await initMsw(); - hasInitialized.current = true; - }; + hasInitialized.current = true; + const init = async () => { + const { initMsw } = await import("../../mocks/index"); + await initMsw(); + };
25-25: Remove commented default export.Keeps mocks clean; dead/commented exports tend to linger.
-// export default MSWComponent;mocks/guestMode/handlers.ts (1)
255-263: Remove leftover debug logs in filters.console.log in hot paths adds noise. Gate behind dev flag or remove.
Apply this diff:
- const subsForGoal = subs.filter((s) => { - //test - console.log("s.goalId, goalId: ", s.goalId, goalId); - return s.goalId === String(goalId); - }); + const subsForGoal = subs.filter((s) => s.goalId === String(goalId));mocks/guestMode/db.ts (1)
1-1: Good: DB_NAME exported for guest detection; lock as literal.Minor nit: type it as a literal to avoid accidental widening.
-export const DB_NAME = "motimo-guest"; +export const DB_NAME = "motimo-guest" as const;app/onboarding/page.tsx (1)
67-69: Redirect now includes guests: OK; remove stale commented condition.Behavior matches PR goal (skip onboarding when guest has completed). Drop commented branch to avoid confusion.
- // 이미 온보딩을 완료했으면 redirect (게스트도 포함) - if (hasCompletedOnboarding && isLoggedIn) { - // if (hasCompletedOnboarding && isLoggedIn && !isGuest) { + // 이미 온보딩을 완료했으면 redirect (게스트 포함) + if (hasCompletedOnboarding && isLoggedIn) {api/useApiQuery.ts (1)
36-66: Minor: error handling is fine; consider preserving original stackCurrently rethrow wraps the original error message only. If you need original stacks during debugging, attach
cause: errorwhen constructing the new Error.Example:
- throw new Error( - `Failed to call ${String(apiGroup)}.${String(method)}: ${errorMessage}`, - ); + throw new Error( + `Failed to call ${String(apiGroup)}.${String(method)}: ${errorMessage}`, + { cause: error as unknown } + );app/onboarding/_components/LoginScreen.tsx (1)
176-188: Harden guest onboarding check; feature-detectindexedDB.databases()and handle errors
indexedDB.databases()isn’t supported in all browsers and can throw. Also, batch state updates to avoid intermediate inconsistent states.Apply:
- const handleBrowse = async () => { + const handleBrowse = async () => { // TODO: Handle browse without login reset(); - - // 게스트모드 로그인 기록 확인 - const dbs = await indexedDB.databases(); - const hasGuestDB = dbs.find((db) => db.name === DB_NAME); - if (hasGuestDB) setHasCompletedOnboarding(true); - - login(); - setIsGuest(true); + try { + // 게스트모드 로그인 기록 확인 (feature detection) + let hasGuestDB = false; + if ("databases" in indexedDB) { + const dbs = await (indexedDB as any).databases(); + hasGuestDB = !!dbs.find((db: any) => db?.name === DB_NAME); + } + if (hasGuestDB) setHasCompletedOnboarding(true); + } catch { + // no-op: treat as first-time guest + } + // Batch updates to reduce flicker + login(); + setIsGuest(true); // onNext(); };api/service.ts (1)
21-27: Nit: simplifysecurityWorkerThe
isGuest || !tokenbranch is effectively the same as “no auth header”; returning{}would also work sinceformat: "json"is set in the token branch. If you rely onformat: "json"for guest/unauth flows, keep as-is; otherwise, consider simplifying.No diff provided (optional).
components/details/ListCard/ListCard.tsx (1)
157-159: Add accessibility signal for loading state.Expose loading state to AT to complement visual opacity.
Apply:
- <main - className={`w-full flex-1 p-4 inline-flex flex-col justify-start items-center gap-5 overflow-hidden ${isLoading ? "opacity-50" : ""}`} - > + <main + className={`w-full flex-1 p-4 inline-flex flex-col justify-start items-center gap-5 overflow-hidden ${isLoading ? "opacity-50" : ""}`} + aria-busy={isLoading} + >hooks/queries/useSubGoalTodosInfiniites.ts (1)
134-134: UnifyisReachedLastdefault behavior across hooks
Inhooks/queries/useSubGoalTodosInfiniites.tsthere are two differing implementations ofisReachedLast:
Lines 67–71 (e.g. in
useSubGoalTodosIncompleteOrTodayInfinite):
const isReachedLast = data ? !data[data.length - 1]?.hasNext : true;
→ defaults totruewhendataisundefined.Lines 134–136 (e.g. in the companion infinite hook):
const isReachedLast = !!data && !data[data.length - 1]?.hasNext;
→ defaults tofalsewhendataisundefined.Pick one of these fallback behaviors and apply it consistently to both hooks so that pagination end-of-data checks don’t diverge.
📜 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 selected for processing (12)
api/service.ts(4 hunks)api/useApiQuery.ts(3 hunks)app/layout.tsx(0 hunks)app/onboarding/_components/LoginScreen.tsx(3 hunks)app/onboarding/page.tsx(1 hunks)components/_mocks/MSWComponent.tsx(2 hunks)components/details/ListCard/ListCard.tsx(5 hunks)components/shared/Checkbox/Checkbox.tsx(1 hunks)hooks/queries/useSubGoalTodosInfiniites.ts(4 hunks)mocks/guestMode/db.ts(1 hunks)mocks/guestMode/handlers.ts(3 hunks)mocks/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- app/layout.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/_mocks/MSWComponent.tsxcomponents/shared/Checkbox/Checkbox.tsxcomponents/details/ListCard/ListCard.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/_mocks/MSWComponent.tsxcomponents/shared/Checkbox/Checkbox.tsxcomponents/details/ListCard/ListCard.tsx
components/**/*
📄 CodeRabbit inference engine (.cursor/rules/global-rule.mdc)
Component should be created in components/
Files:
components/_mocks/MSWComponent.tsxcomponents/shared/Checkbox/Checkbox.tsxcomponents/details/ListCard/ListCard.tsx
🧬 Code graph analysis (2)
app/onboarding/_components/LoginScreen.tsx (1)
mocks/guestMode/db.ts (1)
DB_NAME(1-1)
hooks/queries/useSubGoalTodosInfiniites.ts (1)
lib/fetching/template/fetchTemplate.ts (1)
templateFetch(53-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: chromatic-test
🔇 Additional comments (11)
mocks/index.ts (1)
42-44: No-op whitespace after stop; behavior unchanged.No functional change. Stop path still sets isRunning = false after stopping, which is fine.
mocks/guestMode/handlers.ts (1)
34-35: ✔ date2StringWithSpliter utility verified– The function
date2StringWithSpliteris defined and exported inutils/date2String.ts:const date2StringWithSpliter = (dateValue: Date, spliter: string) => { … } export { date2StringWithSpliter };– It’s correctly spelled (one “t” in “Spliter”) and imported in
mocks/guestMode/handlers.ts(lines 34–35) as well as used throughout the application, confirming it will be bundled for the browser.No further action needed.
app/onboarding/page.tsx (1)
67-69: No action needed: guest onboarding completion logic is correctly gated by DB presenceI’ve verified that:
DB_NAMEis defined as"motimo-guest"inmocks/guestMode/db.ts.- In
LoginScreen.tsx,setHasCompletedOnboarding(true)is only called whenindexedDB.databases()contains thatDB_NAME(i.e.hasGuestDB) — so there’s no false-positive flagging for guests.- The redirect in
app/onboarding/page.tsxtriggers onhasCompletedOnboarding && isLoggedIn, which naturally only applies to guests who were explicitly marked complete via the DB check.As a result, the onboarding flag for guests cannot be set spuriously, and no further changes are required.
components/details/ListCard/ListCard.tsx (5)
17-21: Import of optimistic helper looks good.Import path and usage align with the new helper’s API.
66-67: Surfacing isValidating is a good UX touch.Consuming
isValidatingenables clear incremental state feedback.
296-299: Nice progressive feedback during revalidation.The container-level
opacity-50tied toisValidatingwith a smooth transition reads well.
350-350: ‘더보기’ threshold of 5 matches the PR objective.Condition reads correctly.
31-31: Motion v12 dependency and client component detected—please confirm bundling setup
- package.json declares
"motion": "^12.18.1", so you’re on Motion v12+ as expected.- ListCard.tsx begins with
"use client";and imports frommotion/react, so it’s correctly marked as a client component.- No
transpilePackagesornext-transpile-modulesconfiguration was found innext.config.ts. Please verify that your Next.js build is transpiling/bundling themotionpackage for SSR/client use (e.g. via the experimentaltranspilePackages: ['motion']setting ornext-transpile-modules).hooks/queries/useSubGoalTodosInfiniites.ts (3)
29-31: SWR key simplification is fine.Including the full path (with subGoalId embedded) keeps the key unique without a separate
subGoalIdelement.
85-99: Good pagination guard and keying for SWR Infinite.Stopping when
!hasNextand keying by[path, pageIndex, offset]is correct.Also applies to: 100-116, 118-121
216-223: Export surface looks consistent.New helper is exported alongside hooks; naming aligns with usage.
| const debounceer = <T, E>(apiRequest: typeof httpClient.request<T, E>) => { | ||
| const timeLimit = 1000; | ||
| let timer: number; | ||
| const timeLimit = 300; | ||
| const timerDictionary: { [apiFullUrl: string]: number } = {}; | ||
| let rejectTimer: (reason?: any) => void; | ||
| return ( | ||
| requestParams: Parameters<typeof httpClient.request<T, E>>[0], | ||
| ): ReturnType<typeof httpClient.request<T>> => { | ||
| const apiFullUrl = `${requestParams.path}?${requestParams.query}`; | ||
| const timer = timerDictionary[apiFullUrl]; | ||
|
|
||
| if (timer) { | ||
| clearTimeout(timer); | ||
| rejectTimer("debouncing"); | ||
| } |
There was a problem hiding this comment.
Debouncer has race/incorrect rejection issues; make timers and rejectors per-URL and reject with a cancel sentinel
- Single
rejectTimershared across URLs will reject the wrong promise under concurrency. - On clearing, rejecting with a bare string may surface as an error in consumers.
- Also,
timerDictionaryshould useReturnType<typeof setTimeout>.
Apply:
-const debounceer = <T, E>(apiRequest: typeof httpClient.request<T, E>) => {
- const timeLimit = 300;
- const timerDictionary: { [apiFullUrl: string]: number } = {};
- let rejectTimer: (reason?: any) => void;
+const debounceer = <T, E>(apiRequest: typeof httpClient.request<T, E>) => {
+ const timeLimit = 300;
+ const timerDictionary: Record<string, ReturnType<typeof setTimeout>> = {};
+ const rejectDictionary: Record<string, (reason?: any) => void> = {};
return (
- requestParams: Parameters<typeof httpClient.request<T, E>>[0],
- ): ReturnType<typeof httpClient.request<T>> => {
- const apiFullUrl = `${requestParams.path}?${requestParams.query}`;
- const timer = timerDictionary[apiFullUrl];
+ requestParams: Parameters<typeof httpClient.request<T, E>>[0],
+ ): ReturnType<typeof httpClient.request<T, E>> => {
+ const query =
+ typeof requestParams.query === "string"
+ ? requestParams.query
+ : requestParams.query
+ ? new URLSearchParams(requestParams.query as Record<string, any>).toString()
+ : "";
+ const apiFullUrl = `${requestParams.path}${query ? `?${query}` : ""}`;
+ const timer = timerDictionary[apiFullUrl];
if (timer) {
clearTimeout(timer);
- rejectTimer("debouncing");
+ rejectDictionary[apiFullUrl]?.(
+ Object.assign(new Error("request debounced"), {
+ name: "AbortError",
+ code: "ERR_CANCELED",
+ canceled: true,
+ })
+ );
}
- const apiRes: Promise<T> = new Promise((resolve, reject) => {
- rejectTimer = reject;
- timerDictionary[apiFullUrl] = Number(
- // timer = Number(
- setTimeout(async () => {
- try {
- const res = apiRequest(requestParams);
- resolve(res);
- } catch (error) {
- console.error(error);
- showToast(`API ERROR`, new Date());
- }
- }, timeLimit),
- );
+ const apiRes: Promise<T> = new Promise((resolve, reject) => {
+ rejectDictionary[apiFullUrl] = reject;
+ timerDictionary[apiFullUrl] = setTimeout(async () => {
+ try {
+ // see token/401 handling below
+ const res = await apiRequest(requestParams);
+ resolve(res);
+ } catch (error) {
+ // rejection handled below (401 retry) or propagated
+ reject(error);
+ } finally {
+ delete timerDictionary[apiFullUrl];
+ delete rejectDictionary[apiFullUrl];
+ }
+ }, timeLimit);
});Also applies to: 53-65
🤖 Prompt for AI Agents
In api/service.ts around lines 37-50 (and similarly adjust lines 53-65), the
debouncer wrongly shares a single rejectTimer across all URLs and uses number
type for timers and a bare string rejection; change timerDictionary to map
apiFullUrl to ReturnType<typeof setTimeout>, and create a per-URL entry for both
timer and its rejector (e.g., store an object { timer, reject } keyed by
apiFullUrl). When clearing a timer only clear and call the corresponding per-URL
reject function, and reject with a cancel sentinel (use a dedicated Error or
Symbol, e.g., new Error('debounced') or CANCELED symbol) instead of a raw string
so consumers can identify cancellations. Ensure you remove the per-URL entries
after resolution/rejection to avoid leaks.
| return ( | ||
| requestParams: Parameters<typeof httpClient.request<T, E>>[0], | ||
| ): ReturnType<typeof httpClient.request<T>> => { | ||
| const apiFullUrl = `${requestParams.path}?${requestParams.query}`; |
There was a problem hiding this comment.
Fix return type generic
Missing E in ReturnType<typeof httpClient.request<T, E>> causes typing drift.
The diff above adjusts the signature at this line.
🤖 Prompt for AI Agents
In api/service.ts around line 43, the function return type uses
ReturnType<typeof httpClient.request<T>> but is missing the error generic E,
causing typing drift; update the function signature to accept the error generic
(add <E = unknown> where appropriate) and change the return type to
ReturnType<typeof httpClient.request<T, E>> so the httpClient.request call's
error type is propagated; ensure any related type params and callers are updated
to include or default E to avoid breaking changes.
| const timer = timerDictionary[apiFullUrl]; | ||
|
|
||
| if (timer) { |
There was a problem hiding this comment.
Serialize requestParams.query properly
"${requestParams.query}" will become [object Object] and collide across distinct queries.
The diff above already replaces lines 44-46 with a safe URLSearchParams serialization.
🤖 Prompt for AI Agents
In api/service.ts around lines 44 to 46, requestParams.query is being
string-interpolated which yields "[object Object]" and causes collisions;
replace that interpolation by serializing the query properly (e.g., build a
URLSearchParams from requestParams.query and use its toString()) so apiFullUrl
includes the encoded query string, and then use that serialized string to look
up timerDictionary.
| tokenHandler(apiRes); | ||
|
|
||
| return apiRes; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
401 handling doesn’t retry the original request; tokenHandler is not chained, so callers still see a rejection
Current approach calls tokenHandler(apiRes) without awaiting/rewrapping, and tokenHandler doesn’t retry. Move the 401 interception into the debounced execution and retry once after refreshing tokens.
Apply:
- // 토큰 재발급 처리
- tokenHandler(apiRes);
-
return apiRes;
};
};
-// 토큰 처리
-const tokenHandler = async <T, E>(
- apiRes: ReturnType<typeof httpClient.request<T, E>>,
-) => {
- return apiRes.catch(async (e) => {
- if (e.status === 401) {
- const token = useAuthStore.getState().refreshToken;
-
- if (!token) {
- api.authController.logout();
- throw new Error("no refresh token");
- }
- const tokenRes = await api.authController.reissue({
- refreshToken: token,
- });
-
- if (!tokenRes?.accessToken || !tokenRes?.refreshToken) {
- throw new Error("token reissue error");
- }
-
- useAuthStore.setState((states) => ({
- ...states,
- accessToken: tokenRes.accessToken,
- refreshToken: tokenRes.refreshToken,
- }));
- }
- });
-};
+// Refresh tokens helper
+const refreshTokens = async () => {
+ const token = useAuthStore.getState().refreshToken;
+ if (!token) {
+ api.authController.logout();
+ throw new Error("no refresh token");
+ }
+ const tokenRes = await api.authController.reissue({ refreshToken: token });
+ if (!tokenRes?.accessToken || !tokenRes?.refreshToken) {
+ throw new Error("token reissue error");
+ }
+ useAuthStore.setState((s) => ({
+ ...s,
+ accessToken: tokenRes.accessToken,
+ refreshToken: tokenRes.refreshToken,
+ }));
+};And augment the debounced execution to retry on 401 (within the earlier diff block):
- try {
- // see token/401 handling below
- const res = await apiRequest(requestParams);
- resolve(res);
- } catch (error) {
- // rejection handled below (401 retry) or propagated
- reject(error);
- } finally {
+ try {
+ try {
+ const res = await apiRequest(requestParams);
+ resolve(res);
+ } catch (error: any) {
+ if ((error as any)?.status === 401) {
+ await refreshTokens();
+ const retried = await apiRequest(requestParams);
+ resolve(retried);
+ } else {
+ throw error;
+ }
+ }
+ } catch (finalErr) {
+ showToast(`API ERROR`, new Date());
+ reject(finalErr);
+ } finally {Also applies to: 74-100, 55-63
| {(!checkedMore ? todoItemsInfo.slice(0, 5) : todoItemsInfo).map( | ||
| (todoInfo) => { | ||
| const optimisticDataCallback = | ||
| makeSubgoalInfiniteOptimisticData(todoInfo.id); | ||
|
|
||
| return ( | ||
| <motion.div | ||
| key={todoInfo.id} | ||
| layout | ||
| transition={{ duration: 0.3 }} | ||
| initial={{ opacity: 0, scale: 0.8 }} // 나타날 때 시작 상태 | ||
| animate={{ opacity: 1, scale: 1 }} // 나타날 때 최종 상태 | ||
| exit={{ opacity: 0, scale: 0.8 }} // 사라질 때 최종 상태 | ||
| > | ||
| <TodoItem | ||
| onChecked={async () => { | ||
| await mutate( | ||
| optimisticDataCallback, | ||
| // undefined, | ||
|
|
||
| { | ||
| // optimisticData: optimisticDataCallback, | ||
| populateCache: false, | ||
| revalidate: false, | ||
| rollbackOnError: true, | ||
| }, | ||
| ); | ||
|
|
||
| onTodoCheck && onTodoCheck(todoInfo.id); | ||
|
|
||
| // const res = await todoApi.toggleTodoCompletion(todoInfo.id); | ||
| // if (res) mutate(); | ||
| }} | ||
| title={todoInfo.title} | ||
| checked={todoInfo.checked} | ||
| key={todoInfo.id} | ||
| onReportedClick={async () => { | ||
| // 일단 모달을 띄워야 함. | ||
| // postTodoResult(todoInfo.id); | ||
| setOpenBottomSheet(true); | ||
| setTodoIdForResult(todoInfo.id); | ||
| }} | ||
| reported={todoInfo.reported} | ||
| targetDate={todoInfo.targetDate} | ||
| /> | ||
| </motion.div> | ||
| ); | ||
| }, | ||
| )} | ||
| </AnimatePresence> |
There was a problem hiding this comment.
Optimistic update misconfigured: cache isn’t updated; request isn’t awaited.
- Passing a MutatorCallback with
populateCache: falseprevents the optimistic value from entering the cache, so no UI update occurs. - The server toggle (
onTodoCheck) isn’t awaited; failures won’t roll back.
Fix by using SWR’s optimisticData + in-mutate request (auto rollback on rejection) and let SWR revalidate.
Apply:
- <TodoItem
- onChecked={async () => {
- await mutate(
- optimisticDataCallback,
- {
- // optimisticData: optimisticDataCallback,
- populateCache: false,
- revalidate: false,
- rollbackOnError: true,
- },
- );
-
- onTodoCheck && onTodoCheck(todoInfo.id);
- }}
+ <TodoItem
+ onChecked={async () => {
+ await mutate(
+ async (current) => {
+ // perform server toggle; throw to trigger SWR rollback
+ await onTodoCheck?.(todoInfo.id);
+ return current; // keep optimistic cache until revalidate finishes
+ },
+ {
+ optimisticData: optimisticDataCallback,
+ populateCache: true,
+ revalidate: true,
+ rollbackOnError: true,
+ },
+ );
+ }}
title={todoInfo.title}
checked={todoInfo.checked}
- key={todoInfo.id}
+ /* key on motion.div is sufficient */Optional animation polish:
- <AnimatePresence>
+ <AnimatePresence initial={false}>📝 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.
| {(!checkedMore ? todoItemsInfo.slice(0, 5) : todoItemsInfo).map( | |
| (todoInfo) => { | |
| const optimisticDataCallback = | |
| makeSubgoalInfiniteOptimisticData(todoInfo.id); | |
| return ( | |
| <motion.div | |
| key={todoInfo.id} | |
| layout | |
| transition={{ duration: 0.3 }} | |
| initial={{ opacity: 0, scale: 0.8 }} // 나타날 때 시작 상태 | |
| animate={{ opacity: 1, scale: 1 }} // 나타날 때 최종 상태 | |
| exit={{ opacity: 0, scale: 0.8 }} // 사라질 때 최종 상태 | |
| > | |
| <TodoItem | |
| onChecked={async () => { | |
| await mutate( | |
| optimisticDataCallback, | |
| // undefined, | |
| { | |
| // optimisticData: optimisticDataCallback, | |
| populateCache: false, | |
| revalidate: false, | |
| rollbackOnError: true, | |
| }, | |
| ); | |
| onTodoCheck && onTodoCheck(todoInfo.id); | |
| // const res = await todoApi.toggleTodoCompletion(todoInfo.id); | |
| // if (res) mutate(); | |
| }} | |
| title={todoInfo.title} | |
| checked={todoInfo.checked} | |
| key={todoInfo.id} | |
| onReportedClick={async () => { | |
| // 일단 모달을 띄워야 함. | |
| // postTodoResult(todoInfo.id); | |
| setOpenBottomSheet(true); | |
| setTodoIdForResult(todoInfo.id); | |
| }} | |
| reported={todoInfo.reported} | |
| targetDate={todoInfo.targetDate} | |
| /> | |
| </motion.div> | |
| ); | |
| }, | |
| )} | |
| </AnimatePresence> | |
| <AnimatePresence initial={false}> | |
| {(!checkedMore ? todoItemsInfo.slice(0, 5) : todoItemsInfo).map( | |
| (todoInfo) => { | |
| const optimisticDataCallback = | |
| makeSubgoalInfiniteOptimisticData(todoInfo.id); | |
| return ( | |
| <motion.div | |
| key={todoInfo.id} | |
| layout | |
| transition={{ duration: 0.3 }} | |
| initial={{ opacity: 0, scale: 0.8 }} // 나타날 때 시작 상태 | |
| animate={{ opacity: 1, scale: 1 }} // 나타날 때 최종 상태 | |
| exit={{ opacity: 0, scale: 0.8 }} // 사라질 때 최종 상태 | |
| > | |
| <TodoItem | |
| onChecked={async () => { | |
| await mutate( | |
| async (current) => { | |
| // perform server toggle; throw to trigger SWR rollback | |
| await onTodoCheck?.(todoInfo.id); | |
| return current; // keep optimistic cache until revalidate finishes | |
| }, | |
| { | |
| optimisticData: optimisticDataCallback, | |
| populateCache: true, | |
| revalidate: true, | |
| rollbackOnError: true, | |
| }, | |
| ); | |
| }} | |
| title={todoInfo.title} | |
| checked={todoInfo.checked} | |
| onReportedClick={async () => { | |
| // 일단 모달을 띄워야 함. | |
| setOpenBottomSheet(true); | |
| setTodoIdForResult(todoInfo.id); | |
| }} | |
| reported={todoInfo.reported} | |
| targetDate={todoInfo.targetDate} | |
| /> | |
| </motion.div> | |
| ); | |
| }, | |
| )} | |
| </AnimatePresence> |
🤖 Prompt for AI Agents
components/details/ListCard/ListCard.tsx lines 300-349: the mutate call
currently passes a MutatorCallback with populateCache:false and revalidate:false
so the optimistic value never enters the cache and the server toggle
(onTodoCheck) isn’t awaited so failures won’t rollback; change to use SWR’s
optimisticData pattern by calling mutate with an optimisticData object
representing the toggled todo and an in-mutate async function that performs
await onTodoCheck(todoInfo.id) (or the API call) and returns the new server
data, keep rollbackOnError:true, and allow revalidation (remove
populateCache:false and revalidate:false) so the UI updates immediately, errors
roll back, and SWR revalidates afterward.
| className={` | ||
| shrink-0 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a visible keyboard focus style
Currently focus can be invisible (appearance-none and outline-0 when checked). Ensure accessible focus indication.
Add focus-visible styles in the base block:
- shrink-0
+ shrink-0
+ focus-visible:outline-2 focus-visible:outline-Color-primary-70📝 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.
| className={` | |
| shrink-0 | |
| className={` | |
| shrink-0 | |
| focus-visible:outline-2 focus-visible:outline-Color-primary-70 | |
| `} |
🤖 Prompt for AI Agents
In components/shared/Checkbox/Checkbox.tsx around lines 10 to 11, the checkbox
currently uses classes that remove visible focus (e.g., appearance-none and
outline-0), so keyboard focus can be invisible; add focus-visible styles to the
base class block to ensure an accessible focus indicator (for example add
focus-visible:outline-none? No — instead add a visible focus ring/outline like
focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-primary or
focus-visible:outline focus-visible:outline-2 focus-visible:outline-primary) and
ensure these styles are not suppressed when the input is checked; update the
className to include those focus-visible utility classes so keyboard users see a
clear focus state.
| props.checked | ||
| ? `bg-center | ||
| bg-background-strong | ||
| bg-[url("data:image/svg+xml,%3Csvg%20width%3D%2214%22%20height%3D%2214%22%20viewBox%3D%220%200%2014%2014%22%20fill%3D%22none%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%3E%3Cpath%20d%3D%22M3.58325%207.27735L6.08343%209.77734L11.0833%204.77734%22%20stroke%3D%22white%22%20stroke-width%3D%221.5%22%20stroke-linecap%3D%22round%22%20stroke-linejoin%3D%22round%22%2F%3E%3C%2Fsvg%3E")] | ||
| outline-0 | ||
| hover:bg-Color-primary-70` | ||
| : `bg-background-alternative outline-[1.50px] outline-offset-[-1.50px] outline-Color-gray-20` | ||
| } |
There was a problem hiding this comment.
Critical: Visual state tied to props.checked breaks uncontrolled checkboxes
Relying on props.checked for styling desynchronizes UI when the input is used uncontrolled (e.g., with defaultChecked). Use Tailwind’s checked: variant so the visual state follows the actual input state.
Apply this diff to remove the runtime conditional and style via pseudo-class:
- ${
- props.checked
- ? `bg-center
- bg-background-strong
- bg-[url("data:image/svg+xml,%3Csvg%20width%3D%2214%22%20height%3D%2214%22%20viewBox%3D%220%200%2014%2014%22%20fill%3D%22none%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%3E%3Cpath%20d%3D%22M3.58325%207.27735L6.08343%209.77734L11.0833%204.77734%22%20stroke%3D%22white%22%20stroke-width%3D%221.5%22%20stroke-linecap%3D%22round%22%20stroke-linejoin%3D%22round%22%2F%3E%3C%2Fsvg%3E")]
- outline-0
- hover:bg-Color-primary-70`
- : `bg-background-alternative outline-[1.50px] outline-offset-[-1.50px] outline-Color-gray-20`
- }
+ bg-background-alternative
+ checked:bg-center
+ checked:bg-background-strong
+ checked:bg-[url("data:image/svg+xml,%3Csvg%20width%3D%2214%22%20height%3D%2214%22%20viewBox%3D%220%200%2014%2014%22%20fill%3D%22none%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%3E%3Cpath%20d%3D%22M3.58325%207.27735L6.08343%209.77734L11.0833%204.77734%22%20stroke%3D%22white%22%20stroke-width%3D%221.5%22%20stroke-linecap%3D%22round%22%20stroke-linejoin%3D%22round%22%2F%3E%3C%2Fsvg%3E")]
+ checked:outline-0
+ checked:hover:bg-Color-primary-70Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/shared/Checkbox/Checkbox.tsx around lines 18-25, the component
currently toggles visual classes based on props.checked which breaks
uncontrolled usage (defaultChecked); remove the runtime conditional and instead
apply Tailwind pseudo-class variants so the UI follows the native input state —
replace the conditional class string with static classes that use checked: or
peer-checked: variants (e.g., add "peer" to the input and use
"peer-checked:bg-center peer-checked:bg-[url(...)]
peer-checked:bg-background-strong peer-checked:outline-0
hover:peer-checked:bg-Color-primary-70" and fallback outline classes for the
unchecked state), ensuring the actual <input> remains the stateful element so
uncontrolled/defaultChecked cases render correctly.
| const makeSubgoalInfiniteOptimisticData = | ||
| (todoId: string) => | ||
| (currentCache: (SubGoalTodoInfinite | undefined)[] | undefined) => { | ||
| if (!currentCache) return []; | ||
|
|
||
| const res = currentCache.map((cache) => { | ||
| if (!cache) return cache; | ||
| const targetTodoIdx = cache.content.findIndex( | ||
| (todoInfo) => todoInfo.id === todoId, | ||
| ); | ||
| // 못찾았다면 | ||
| if (targetTodoIdx < 0) return cache; | ||
|
|
||
| // 찾았다면 | ||
| const mutatedContent = cache.content.map((info, idx) => { | ||
| if (idx !== targetTodoIdx) return info; | ||
| return { | ||
| ...info, | ||
| status: | ||
| info.status === TodoRsStatusEnum.COMPLETE | ||
| ? TodoRsStatusEnum.INCOMPLETE | ||
| : TodoRsStatusEnum.COMPLETE, | ||
| }; | ||
| }); | ||
|
|
||
| return { ...cache, content: mutatedContent }; | ||
| }); | ||
|
|
||
| return res; | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Bug: returning [] when cache is undefined corrupts pagination state.
If invoked before data loads, SWR cache becomes an empty array, which can flip isReachedLast and block fetching. Return the input unchanged instead.
Apply:
- (currentCache: (SubGoalTodoInfinite | undefined)[] | undefined) => {
- if (!currentCache) return [];
+ (currentCache: (SubGoalTodoInfinite | undefined)[] | undefined) => {
+ if (!currentCache) return currentCache;Minor: keep mapping purely functional (already cloning pages) — good.
📝 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 makeSubgoalInfiniteOptimisticData = | |
| (todoId: string) => | |
| (currentCache: (SubGoalTodoInfinite | undefined)[] | undefined) => { | |
| if (!currentCache) return []; | |
| const res = currentCache.map((cache) => { | |
| if (!cache) return cache; | |
| const targetTodoIdx = cache.content.findIndex( | |
| (todoInfo) => todoInfo.id === todoId, | |
| ); | |
| // 못찾았다면 | |
| if (targetTodoIdx < 0) return cache; | |
| // 찾았다면 | |
| const mutatedContent = cache.content.map((info, idx) => { | |
| if (idx !== targetTodoIdx) return info; | |
| return { | |
| ...info, | |
| status: | |
| info.status === TodoRsStatusEnum.COMPLETE | |
| ? TodoRsStatusEnum.INCOMPLETE | |
| : TodoRsStatusEnum.COMPLETE, | |
| }; | |
| }); | |
| return { ...cache, content: mutatedContent }; | |
| }); | |
| return res; | |
| }; | |
| const makeSubgoalInfiniteOptimisticData = | |
| (todoId: string) => | |
| (currentCache: (SubGoalTodoInfinite | undefined)[] | undefined) => { | |
| if (!currentCache) return currentCache; | |
| const res = currentCache.map((cache) => { | |
| if (!cache) return cache; | |
| const targetTodoIdx = cache.content.findIndex( | |
| (todoInfo) => todoInfo.id === todoId, | |
| ); | |
| // 못찾았다면 | |
| if (targetTodoIdx < 0) return cache; | |
| // 찾았다면 | |
| const mutatedContent = cache.content.map((info, idx) => { | |
| if (idx !== targetTodoIdx) return info; | |
| return { | |
| ...info, | |
| status: | |
| info.status === TodoRsStatusEnum.COMPLETE | |
| ? TodoRsStatusEnum.INCOMPLETE | |
| : TodoRsStatusEnum.COMPLETE, | |
| }; | |
| }); | |
| return { ...cache, content: mutatedContent }; | |
| }); | |
| return res; | |
| }; |
🤖 Prompt for AI Agents
In hooks/queries/useSubGoalTodosInfiniites.ts around lines 185 to 215, the
helper currently returns [] when currentCache is undefined which corrupts SWR
pagination state; change the early-return to return currentCache (i.e., do not
replace undefined with an empty array) so the cache remains unchanged if not
loaded, and keep the existing pure mapping logic that clones pages when
transforming a found todo's status.
1 participant
최적화
기타 수정사항
Summary by CodeRabbit
New Features
Improvements
Bug Fixes