-
Notifications
You must be signed in to change notification settings - Fork 1
chore: 성능 최적화(i18n 인스턴스 캐싱, 폰트 로딩 방식 개선, next.js 패키지 최적화, 메모 캘린더 prefetch, 로그인 중복 로직 제거) #179
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
Conversation
""" Walkthrough인증이 필요한 페이지 목록에 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp
participant AuthCheck
participant PathConstants
User->>WebApp: 요청 (특정 경로 접근)
WebApp->>PathConstants: NEED_AUTH_PAGES 목록 확인
WebApp->>AuthCheck: request.nextUrl.pathname에 NEED_AUTH_PAGES의 일부 포함 여부 검사
alt 포함됨
AuthCheck-->>WebApp: 인증 필요
WebApp->>User: 인증 요구/처리
else 미포함
AuthCheck-->>WebApp: 인증 불필요
WebApp->>User: 정상 처리
end
sequenceDiagram
participant Server
participant i18nCache
participant i18next
Server->>i18nCache: 언어별 인스턴스 조회
alt 캐시에 있음
i18nCache-->>Server: 캐시된 인스턴스 반환
else 없음
Server->>i18next: 새 인스턴스 생성 및 초기화
i18next-->>i18nCache: 인스턴스 저장
i18nCache-->>Server: 새 인스턴스 반환
end
Possibly related PRs
Poem
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: packages/web/src/app/[lng]/(auth)/memos/layout.tsx
Did you find this useful? React with a 👍 or 👎 |
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: 3
🧹 Nitpick comments (1)
packages/web/src/modules/i18n/util.server.ts (1)
8-8
: 메모리 관리 고려사항캐시가 무제한으로 증가할 수 있어 메모리 누수 가능성이 있습니다. 애플리케이션의 사용 패턴을 고려하여 캐시 크기 제한이나 TTL(Time To Live) 메커니즘을 도입하는 것을 고려해보세요.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/shared/src/constants/Path.ts
(1 hunks)packages/web/src/app/[lng]/(auth)/memos/layout.tsx
(2 hunks)packages/web/src/modules/i18n/util.server.ts
(2 hunks)packages/web/src/modules/supabase/util.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*`: Continuously improve code rules by monitoring for new code patterns, rep...
**/*
: Continuously improve code rules by monitoring for new code patterns, repeated implementations, common errors, new libraries, and emerging best practices across the entire codebase.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/self_improve.mdc)
List of files the instruction was applied to:
packages/web/src/modules/supabase/util.ts
packages/web/src/app/[lng]/(auth)/memos/layout.tsx
packages/shared/src/constants/Path.ts
packages/web/src/modules/i18n/util.server.ts
`packages/**`: The 'packages/' directory is for shared packages that can be used by both the Chrome extension and other applications in the monorepo.
packages/**
: The 'packages/' directory is for shared packages that can be used by both the Chrome extension and other applications in the monorepo.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/tech-stack.mdc)
List of files the instruction was applied to:
packages/web/src/modules/supabase/util.ts
packages/web/src/app/[lng]/(auth)/memos/layout.tsx
packages/shared/src/constants/Path.ts
packages/web/src/modules/i18n/util.server.ts
🧠 Learnings (1)
packages/web/src/app/[lng]/(auth)/memos/layout.tsx (2)
Learnt from: CR
PR: guesung/Web-Memo#0
File: .cursor/rules/coding-preferences.mdc:0-0
Timestamp: 2025-06-24T08:12:51.130Z
Learning: Wrap client components in Suspense with a fallback in React.
Learnt from: CR
PR: guesung/Web-Memo#0
File: .cursor/rules/coding-preferences.mdc:0-0
Timestamp: 2025-06-24T08:12:51.130Z
Learning: Minimize 'use client' usage in React, only using it for Web API access.
🧬 Code Graph Analysis (3)
packages/web/src/modules/supabase/util.ts (1)
packages/shared/src/constants/Path.ts (1)
NEED_AUTH_PAGES
(17-21)
packages/web/src/app/[lng]/(auth)/memos/layout.tsx (1)
packages/web/src/app/[lng]/(auth)/memos/_utils/Sentry.ts (1)
initSentryUserInfo
(8-19)
packages/web/src/modules/i18n/util.server.ts (1)
packages/web/src/modules/i18n/type.ts (1)
Language
(3-3)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build-extension
- GitHub Check: tests_e2e
- GitHub Check: build-web
- GitHub Check: build-web
- GitHub Check: tests_e2e
🔇 Additional comments (1)
packages/shared/src/constants/Path.ts (1)
17-21
: 인증 필요 페이지 목록 확장이 적절합니다.
PATHS.memosWish
를NEED_AUTH_PAGES
배열에 추가하고 다중 라인 형식으로 변경한 것은 코드 가독성과 유지보수성을 향상시킵니다.
const isNeedAuthPage = NEED_AUTH_PAGES.some((page) => | ||
request.nextUrl.pathname.includes(page), | ||
); |
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.
경로 매칭 로직에서 잠재적인 보안 문제가 있습니다.
includes
를 사용한 substring 매칭은 의도하지 않은 경로도 인증 필요 페이지로 인식할 수 있습니다. 예를 들어, PATHS.memos
가 "/memos"
일 때 "/memos-archive"
와 같은 경로도 매칭될 수 있습니다.
더 정확한 매칭을 위해 다음과 같은 방식을 고려해주세요:
-const isNeedAuthPage = NEED_AUTH_PAGES.some((page) =>
- request.nextUrl.pathname.includes(page),
-);
+const isNeedAuthPage = NEED_AUTH_PAGES.some((page) =>
+ request.nextUrl.pathname === page || request.nextUrl.pathname.startsWith(page + '/'),
+);
또는 정확한 경로 매칭을 위해 정규표현식을 사용하는 것을 고려해보세요.
📝 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 isNeedAuthPage = NEED_AUTH_PAGES.some((page) => | |
request.nextUrl.pathname.includes(page), | |
); | |
const isNeedAuthPage = NEED_AUTH_PAGES.some((page) => | |
request.nextUrl.pathname === page || | |
request.nextUrl.pathname.startsWith(page + '/'), | |
); |
🤖 Prompt for AI Agents
In packages/web/src/modules/supabase/util.ts around lines 36 to 38, the current
use of includes for path matching can mistakenly match unintended paths, causing
security issues. Replace the includes check with a stricter matching method such
as exact equality or using regular expressions to ensure only the intended paths
trigger authentication. Adjust the logic to compare the pathname exactly or use
regex patterns anchored to the start and end of the string to avoid partial
matches.
|
||
await initSentryUserInfo({ lng }); | ||
initSentryUserInfo({ lng }); |
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.
비동기 함수 호출에서 await가 누락되었습니다.
initSentryUserInfo
는 비동기 함수이지만 await
없이 호출되고 있습니다. 관련 코드 스니펫(packages/web/src/app/[lng]/(auth)/memos/_utils/Sentry.ts
)을 보면 이 함수는 async
로 선언되어 있습니다.
다음 중 하나의 방식으로 수정해주세요:
옵션 1: await 추가
-initSentryUserInfo({ lng });
+await initSentryUserInfo({ lng });
옵션 2: 비동기 호출을 의도적으로 무시하는 경우
-initSentryUserInfo({ lng });
+void initSentryUserInfo({ lng });
🤖 Prompt for AI Agents
In packages/web/src/app/[lng]/(auth)/memos/layout.tsx at line 23, the async
function initSentryUserInfo is called without await. Fix this by adding await
before initSentryUserInfo to properly handle the asynchronous call, or if the
async call is intentionally fire-and-forget, explicitly handle or document that
behavior to avoid unhandled promise issues.
import resourcesToBackend from "i18next-resources-to-backend"; | ||
import { initReactI18next } from "react-i18next/initReactI18next"; | ||
|
||
import type { Language } from "./type"; | ||
import { getOptions } from "./util"; | ||
|
||
const translationCache = new Map<string, i18n>(); |
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
캐시 키에 namespace가 포함되지 않아 잠재적 문제가 있습니다.
현재 캐시 키로 language
만 사용하고 있지만, initI18next
함수는 namespace
매개변수도 받습니다. 같은 언어라도 다른 namespace를 사용하는 경우 잘못된 인스턴스가 반환될 수 있습니다.
다음과 같이 수정하여 캐시 키에 namespace도 포함하는 것을 권장합니다:
-const translationCache = new Map<string, i18n>();
+const translationCache = new Map<string, i18n>();
-if (translationCache.has(language))
- return translationCache.get(language) as i18n;
+const cacheKey = `${language}-${namespace || 'default'}`;
+if (translationCache.has(cacheKey))
+ return translationCache.get(cacheKey) as i18n;
-translationCache.set(language, i18nInstance);
+translationCache.set(cacheKey, i18nInstance);
Also applies to: 11-12, 25-25
🤖 Prompt for AI Agents
In packages/web/src/modules/i18n/util.server.ts around lines 8, 11-12, and 25,
the cache key for the translationCache map currently uses only the language,
which can cause incorrect instances to be returned when different namespaces are
involved. Modify the cache key to include both language and namespace, for
example by concatenating them with a separator, to ensure unique keys per
language-namespace pair. Update all relevant cache get and set operations
accordingly to use this combined key.
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: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/shared/src/utils/web/Supabase.ts
(1 hunks)packages/web/next.config.mjs
(2 hunks)packages/web/package.json
(1 hunks)packages/web/src/app/[lng]/(auth)/memos/_components/MemoView/index.tsx
(2 hunks)packages/web/src/app/[lng]/(auth)/memos/layout.tsx
(3 hunks)packages/web/src/app/[lng]/layout.tsx
(2 hunks)packages/web/src/app/layout.tsx
(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/web/package.json
- packages/web/src/app/[lng]/layout.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/web/src/app/layout.tsx
- packages/web/src/app/[lng]/(auth)/memos/layout.tsx
🧰 Additional context used
📓 Path-based instructions (2)
`**/*`: Continuously improve code rules by monitoring for new code patterns, rep...
**/*
: Continuously improve code rules by monitoring for new code patterns, repeated implementations, common errors, new libraries, and emerging best practices across the entire codebase.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/self_improve.mdc)
List of files the instruction was applied to:
packages/web/src/app/[lng]/(auth)/memos/_components/MemoView/index.tsx
packages/shared/src/utils/web/Supabase.ts
packages/web/next.config.mjs
`packages/**`: The 'packages/' directory is for shared packages that can be used by both the Chrome extension and other applications in the monorepo.
packages/**
: The 'packages/' directory is for shared packages that can be used by both the Chrome extension and other applications in the monorepo.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/tech-stack.mdc)
List of files the instruction was applied to:
packages/web/src/app/[lng]/(auth)/memos/_components/MemoView/index.tsx
packages/shared/src/utils/web/Supabase.ts
packages/web/next.config.mjs
🧠 Learnings (2)
packages/web/src/app/[lng]/(auth)/memos/_components/MemoView/index.tsx (4)
Learnt from: guesung
PR: guesung/Web-Memo#100
File: packages/web/src/hooks/useLanguage.ts:15-21
Timestamp: 2024-11-22T04:01:17.402Z
Learning: Next.js에서 페이지 이동 시 컴포넌트는 언마운트되고 다시 마운트되므로, `useEffect`의 의존성 배열에 `pathname`과 같은 의존성을 포함하지 않아도 됩니다.
Learnt from: CR
PR: guesung/Web-Memo#0
File: .cursor/rules/coding-preferences.mdc:0-0
Timestamp: 2025-06-24T08:12:51.130Z
Learning: Prefer named exports for components.
Learnt from: CR
PR: guesung/Web-Memo#0
File: .cursor/rules/tech-stack.mdc:0-0
Timestamp: 2025-06-24T08:13:27.483Z
Learning: State management and data fetching are handled using TanStack Query (React Query) v5.59.0 and React Hook Form 7.53.2, promoting robust and scalable state handling.
Learnt from: CR
PR: guesung/Web-Memo#0
File: .cursor/rules/coding-preferences.mdc:0-0
Timestamp: 2025-06-24T08:12:51.130Z
Learning: Use declarative JSX in React components.
packages/shared/src/utils/web/Supabase.ts (1)
Learnt from: CR
PR: guesung/Web-Memo#0
File: .cursor/rules/coding-preferences.mdc:0-0
Timestamp: 2025-06-24T08:12:51.130Z
Learning: Minimize 'use client' usage in React, only using it for Web API access.
🧬 Code Graph Analysis (1)
packages/shared/src/utils/web/Supabase.ts (1)
packages/shared/src/types/supabase.ts (1)
Database
(9-170)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: build-extension
- GitHub Check: build-web
- GitHub Check: tests_e2e
- GitHub Check: build-web
- GitHub Check: tests_e2e
- GitHub Check: build-extension
🔇 Additional comments (4)
packages/shared/src/utils/web/Supabase.ts (1)
7-7
: 타입 안전성 향상이 잘 적용되었습니다.
createBrowserClient
에 스키마별 타입 매개변수를 추가하여memo
스키마에 대한 타입 안전성이 향상되었습니다. 이는 컴파일 타임에 타입 오류를 잡을 수 있도록 도와줍니다.packages/web/next.config.mjs (3)
1-1
: 번들 분석 도구 추가가 적절합니다.성능 최적화를 위한 번들 분석 도구가 추가되었습니다. 개발 단계에서 번들 크기를 모니터링할 수 있어 유용합니다.
17-19
: 패키지 임포트 최적화 설정이 우수합니다.
@web-memo/ui
패키지에 대한 트리 쉐이킹 최적화가 추가되어 불필요한 코드가 번들에서 제외됩니다. 이는 번들 크기 감소와 로딩 성능 향상에 기여할 것입니다.
22-26
: 플러그인 조합이 적절히 구성되었습니다.
withBundleAnalyzer
와withSentryConfig
가 올바른 순서로 조합되어 있습니다. 환경 변수ANALYZE=true
를 통해 필요할 때만 번들 분석이 활성화되는 것도 효율적입니다.
@@ -5,9 +5,9 @@ import type { LanguageType } from "@src/modules/i18n"; | |||
import { useMemosQuery } from "@web-memo/shared/hooks"; | |||
import dynamic from "next/dynamic"; | |||
import { useSearchParams } from "next/navigation"; | |||
import { useEffect } from "react"; |
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
불필요한 import 제거 필요
useEffect가 제거되면 이 import도 함께 제거해야 합니다.
-import { useEffect } from "react";
📝 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.
import { useEffect } from "react"; |
🤖 Prompt for AI Agents
In packages/web/src/app/[lng]/(auth)/memos/_components/MemoView/index.tsx at
line 8, the import statement for useEffect is unnecessary because useEffect is
no longer used in the file. Remove the import { useEffect } from "react"; line
entirely to clean up unused imports.
useEffect(() => { | ||
if (view === "grid") import("./MemoCalendar"); | ||
}, [view]); |
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.
로직 오류: 잘못된 조건과 중복된 import
이 useEffect에는 다음과 같은 문제가 있습니다:
- 잘못된 조건:
view === "grid"
일 때 MemoCalendar를 import하고 있지만, 실제로는view === "calendar"
일 때 렌더링됩니다 (61번째 줄 참조) - 중복된 import: 20-22번째 줄에서 이미 MemoCalendar를 dynamic import로 불러오고 있어 이 useEffect는 불필요합니다
- 비효율적인 구현: Next.js의 dynamic import가 이미 코드 분할과 지연 로딩을 처리하므로 추가적인 성능 이점이 없습니다
다음 중 하나의 해결책을 적용하세요:
해결책 1: useEffect 제거 (권장)
- useEffect(() => {
- if (view === "grid") import("./MemoCalendar");
- }, [view]);
해결책 2: 조건 수정 (필요한 경우)
- useEffect(() => {
- if (view === "grid") import("./MemoCalendar");
- }, [view]);
+ useEffect(() => {
+ if (view === "calendar") import("./MemoCalendar");
+ }, [view]);
하지만 상단의 dynamic import가 이미 필요한 최적화를 제공하므로 해결책 1을 권장합니다.
📝 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(() => { | |
if (view === "grid") import("./MemoCalendar"); | |
}, [view]); |
🤖 Prompt for AI Agents
In packages/web/src/app/[lng]/(auth)/memos/_components/MemoView/index.tsx around
lines 42 to 44, the useEffect hook incorrectly imports MemoCalendar when view is
"grid" but it should be "calendar", and this import is redundant because
MemoCalendar is already dynamically imported at lines 20-22. To fix this, remove
the entire useEffect block as the dynamic import at the top already handles code
splitting and lazy loading efficiently.
Summary by CodeRabbit
버그 수정
성능 개선
기타 개선