AI 문서 정비, lint/deps 정리, auth 로그 보안 보강#69
Conversation
selected template 조회 시 storage 값이 없거나 예상과 다른 경우를 방어합니다. popup 초기 진입이나 데이터 불일치 상황에서 예외가 전파되지 않도록 정리했습니다. template 선택 상태 복원 로직을 안전하게 유지하는 것이 목적입니다.
AI 에이전트와 기여자가 바로 참고할 수 있도록 문서 구조를 보강했습니다. AGENTS.md, docs/ARCHITECTURE.md, docs/CONTRIBUTING.md 를 추가 및 정리했습니다. docs 디렉터리를 추적 대상에 포함하고 개인 작업 문서는 /.docs 사용을 권장합니다.
Editor, Settings, template 관련 코드에서 ESLint 에러를 해소했습니다. 동작 변경보다는 타입 정리와 unsafe 패턴 제거에 가깝습니다. lint 통과를 위한 구조 정리가 포함되어 있습니다.
실제 런타임에 사용하지 않는 패키지를 제거했습니다. 개발 전용 의존성은 devDependencies 로 이동해 의도를 명확히 했습니다. 설치 크기와 의존성 관리 부담을 줄이는 목적입니다.
OAuth 처리 과정에서 민감 정보가 로그에 남지 않도록 제거했습니다. token, auth code 등 사용자 인증 데이터가 콘솔에 출력되지 않도록 정리했습니다. 보안과 개인정보 보호 관점의 수정입니다.
auth flow 디버깅 로그를 개발 환경으로 한정했습니다. 운영 환경에서는 불필요한 로그 노출을 줄이고, 개발 시에는 추적 가능성을 유지합니다. 로그 정책을 auth 영역 기준으로 정리한 변경입니다.
local build 와 backend 연동에 필요한 환경 변수 전제를 문서에 보강했습니다. VITE_API_BASE_URL 이 없는 경우 검증 범위가 제한된다는 점을 명시했습니다. 기여자가 auth, templates, alerts 검증 조건을 빠르게 이해할 수 있게 정리했습니다.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive documentation for AI agents and contributors, cleans up unused dependencies, and improves security by centralizing logging through a new debug utility. Refactoring efforts focused on React component stability and more robust error handling in utility functions. Feedback from the review suggests centralizing the logging logic to reduce duplication, improving the observability of OAuth failures by including non-sensitive error details, and refining component keys to prevent unnecessary re-mounts and state loss.
| function debugLog(message: string, ...args: unknown[]) { | ||
| if (IS_DEV) { | ||
| console.log(message, ...args); | ||
| } | ||
| } |
There was a problem hiding this comment.
debugLog 함수가 background/index.ts, background/handlers/oauth.ts, utils/oauth.ts 등 여러 파일에 동일한 로직으로 중복 정의되어 있습니다. 코드 중복을 줄이고 로깅 정책을 중앙에서 일관되게 관리할 수 있도록, 이를 src/utils/logger.ts와 같은 공통 유틸리티 파일로 분리하여 재사용하는 것을 권장합니다.
| await tokenResponse.text(); | ||
| console.error("[Background] Token exchange failed"); |
There was a problem hiding this comment.
토큰 교환 실패 시 tokenResponse.text()를 호출하여 응답 본문을 가져오지만, 이를 로그에 남기지 않고 있습니다. 보안을 위해 민감 정보 노출을 방지하는 것도 중요하지만, OAuth 에러 응답(예: invalid_grant, expired_code)은 장애 발생 시 원인 파악에 매우 중요한 정보입니다. 에러 본문 전체를 로깅하는 것이 우려된다면, 최소한 HTTP 상태 코드와 함께 에러 메시지의 핵심 내용이라도 로그에 포함하여 운영 환경에서의 유지보수성을 확보하는 것이 좋습니다.
| <Dialog open={open} onOpenChange={onOpenChange}> | ||
| {open && ( | ||
| <QuickAddDialogContent | ||
| key={`${firstIconId ?? 'none'}-${state.defaultIcons.length}-${state.userIcons.length}`} |
There was a problem hiding this comment.
key 값에 아이콘 목록의 길이를 포함하고 있어, 다이얼로그가 열려 있는 동안 아이콘 데이터 로딩이 완료되면 컴포넌트가 재마운트되면서 사용자가 입력 중이던 내용이 초기화될 수 있습니다. 다이얼로그가 열릴 때 폼을 초기화하는 기능은 이미 open 상태에 따른 조건부 렌더링({open && ...})으로 수행되고 있으므로, 불필요하게 상태 변화에 민감한 key를 사용하는 대신 더 안정적인 값을 사용하거나 key 설정을 재검토할 필요가 있습니다.
| if (!selectedItem) return; | ||
| return ( | ||
| <ItemPropertiesPanelForm | ||
| key={getSelectedItemKey(selectedItem)} |
There was a problem hiding this comment.
key 값에 아이템의 모든 데이터(name, siteUrl, size, position 등)를 포함하면, 사용자가 수정한 내용을 저장할 때마다 컴포넌트가 완전히 언마운트 후 재마운트됩니다. 이는 불필요한 렌더링 비용을 발생시킬 뿐만 아니라, 저장 버튼의 포커스가 사라지는 등 사용자 경험(UX)에 부정적인 영향을 줄 수 있습니다. 폼 초기화는 선택된 아이템 자체가 바뀔 때만 일어나면 되므로, key 값으로는 고유 식별자인 selectedItem.templateItemId만 사용하는 것이 적절합니다.
| key={getSelectedItemKey(selectedItem)} | |
| key={selectedItem.templateItemId} |
| const result = await storage.local.get([STORAGE_KEY]); | ||
| const templateId = result[STORAGE_KEY]; | ||
|
|
||
| console.log("[useSelectedTemplate] Loaded from storage:", { |
There was a problem hiding this comment.
이 파일에는 여전히 console.log를 이용한 상세한 로깅이 여러 곳에 남아 있습니다. PR의 취지인 '운영 환경에서의 로그 최소화 및 보안 강화'를 일관되게 적용하기 위해, 이 로그들도 다른 파일들처럼 debugLog를 사용하도록 변경하거나 불필요한 경우 제거하는 것을 권장합니다.
|
@codex plz code review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
| const IS_DEV = import.meta.env.DEV; | ||
|
|
||
| function debugLog(message: string, ...args: unknown[]) { | ||
| if (IS_DEV) { | ||
| console.log(message, ...args); | ||
| } | ||
| } |
There was a problem hiding this comment.
요거는 순수함수고 다른 데서도 사용될 여지가 많으니 utils로 빼두는 것이 좋아 보여요
| const IS_DEV = import.meta.env.DEV; | ||
|
|
||
| function debugLog(message: string, ...args: unknown[]) { | ||
| if (IS_DEV) { | ||
| console.log(message, ...args); | ||
| } | ||
| } |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review this |
|
@gemini-code-assist review this |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48478caedc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }, [loadPostedTemplates, toast]); | ||
|
|
||
| useEffect(() => { | ||
| loadTemplates(); | ||
| }, [loadTemplates]); |
There was a problem hiding this comment.
loadTemplates is memoized with toast in its dependency list, but useToast() returns a freshly created toast callback on every render (src/components/ui/use-toast.ts). That makes loadTemplates change identity every render, so the useEffect watching [loadTemplates] re-runs continuously and keeps re-fetching templates, which can create repeated API traffic and UI churn whenever this page is open.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive project documentation (AGENTS.md, ARCHITECTURE.md, and CONTRIBUTING.md) and implements a centralized logging system in src/utils/logger.ts to replace direct console calls, enforced by a new ESLint rule. Significant refactoring was performed on the Template Editor components, specifically QuickAddDialog and ItemPropertiesPanel, to improve state management by utilizing component keys for resets instead of useEffect. Additionally, the PR reorganizes contexts and hooks for better modularity and cleans up unused dependencies. Feedback is provided to enhance the new logger by adding explicit indicators when data is truncated in arrays or objects to prevent confusion during debugging.
| if (Array.isArray(value)) { | ||
| return value | ||
| .slice(0, MAX_ARRAY_LENGTH) | ||
| .map((entry) => sanitizeValue(entry, depth + 1, seen)); |
There was a problem hiding this comment.
배열이 MAX_ARRAY_LENGTH를 초과하여 잘릴 때, 로그상에서 데이터가 누락되었음을 알 수 있는 표시가 없습니다. 잘린 항목의 개수나 ... 같은 식별자를 추가하여 디버깅 시 오해를 방지하는 것이 좋습니다.
if (Array.isArray(value)) {
const truncated = value.slice(0, MAX_ARRAY_LENGTH).map((entry) => sanitizeValue(entry, depth + 1, seen));
if (value.length > MAX_ARRAY_LENGTH) {
truncated.push(`... and ${value.length - MAX_ARRAY_LENGTH} more items` as any);
}
return truncated;
}| const entries = Object.entries(value).slice(0, MAX_OBJECT_KEYS); | ||
| return entries.reduce<Record<string, unknown>>((acc, [key, entryValue]) => { | ||
| acc[key] = SENSITIVE_KEY_PATTERN.test(key) | ||
| ? REDACTED | ||
| : sanitizeValue(entryValue, depth + 1, seen); | ||
| return acc; | ||
| }, {}); |
There was a problem hiding this comment.
객체의 키 개수가 MAX_OBJECT_KEYS를 초과할 경우 나머지 키들이 자동으로 누락되지만, 로그 출력물만 봐서는 전체 데이터인지 일부인지 알 수 없습니다. _truncated 같은 메타 필드를 추가하여 정보가 생략되었음을 명시하는 것이 좋습니다.
const allKeys = Object.keys(value);
const entries = Object.entries(value).slice(0, MAX_OBJECT_KEYS);
const result = entries.reduce<Record<string, unknown>>((acc, [key, entryValue]) => {
acc[key] = SENSITIVE_KEY_PATTERN.test(key)
? REDACTED
: sanitizeValue(entryValue, depth + 1, seen);
return acc;
}, {});
if (allKeys.length > MAX_OBJECT_KEYS) {
result._truncated = `(${allKeys.length - MAX_OBJECT_KEYS} more keys)`;
}
return result;There was a problem hiding this comment.
Pull request overview
This PR tightens operational/security logging across the extension (especially OAuth/auth flows) by introducing a centralized sanitizer-aware logger, enforcing no-console via ESLint, and updating key docs (AGENTS/ARCHITECTURE/CONTRIBUTING) to reflect the new conventions and runtime constraints.
Changes:
- Add
src/utils/logger.ts(sanitization + env-gated debug/info) and migrate existingconsole.*usage todebugLog/warnLog/errorLog. - Enforce
no-consolein ESLint (with targeted exceptions) and clean up dependencies/devDependencies. - Refactor some context access into dedicated hooks/context objects and add guards for non-extension (dev) environments.
Reviewed changes
Copilot reviewed 59 out of 61 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/todo/customTodo.ts | Route errors through errorLog; simplify D-Day helper signature. |
| src/utils/templateStorage.ts | Replace console.* with errorLog; preserve error causes on throw. |
| src/utils/template.ts | Replace console warnings/errors with centralized logger. |
| src/utils/oauth.ts | Reduce auth-flow log sensitivity; use sanitized error logging. |
| src/utils/logger.ts | New centralized logger with redaction/sanitization and env gating. |
| src/utils/crypto.ts | Use errorLog and attach cause to surfaced errors. |
| src/utils/credentials.ts | Replace console errors with errorLog. |
| src/utils/clientId.ts | Replace console errors with errorLog. |
| src/utils/chrome.ts | Replace console errors with errorLog for script injection failures. |
| src/utils/analytics.ts | Move GA logging to centralized logger; avoid leaking API secret fragments. |
| src/pages/TemplateListPage.tsx | Convert to useCallback-based loader; migrate logs to new logger. |
| src/pages/GalleryPage.tsx | Replace console errors with errorLog. |
| src/pages/EditorPage.tsx | Switch to useEditorContext hook while keeping EditorProvider. |
| src/hooks/useSelectedTemplate.ts | Add chrome storage guards for dev; migrate logs to new logger. |
| src/hooks/usePostedTemplatesContext.ts | New hook wrapper around posted-templates context. |
| src/hooks/usePostedTemplates.ts | Use new context hook; migrate logs to errorLog. |
| src/hooks/useEditorContext.ts | New hook wrapper around editor context. |
| src/hooks/useClipboardCopy.ts | Replace console errors with errorLog. |
| src/contexts/PostedTemplatesContextObject.ts | New file: shared context object/types split-out. |
| src/contexts/PostedTemplatesContext.tsx | Remove inline context/hook; reuse extracted context object. |
| src/contexts/EditorContextObject.ts | New file: shared editor context object/types split-out. |
| src/contexts/EditorContext.tsx | Remove inline context/hook; adjust reducer cases; migrate logs. |
| src/components/ui/label.tsx | Simplify props typing to a type alias. |
| src/components/ui/button.tsx | Stop exporting variants (internalize) to reduce surface area. |
| src/components/ui/badge.tsx | Stop exporting variants (internalize) to reduce surface area. |
| src/components/Tabs/TodoList/TodoList.tsx | Replace console errors with errorLog. |
| src/components/Tabs/TodoList/TodoAddDialog.tsx | Replace console errors with errorLog. |
| src/components/Tabs/TodoList/LoginDialog.tsx | Replace console errors with errorLog. |
| src/components/Tabs/Alerts/SubscriptionManager.tsx | Replace console errors with errorLog. |
| src/components/Tabs/Alerts/MyAlertsView.tsx | Replace console errors with errorLog. |
| src/components/Tabs/Alerts/Alerts.tsx | Replace console errors with errorLog. |
| src/components/SettingsDialog.tsx | Avoid setState-after-unmount for credentials load; use errorLog. |
| src/components/Labs/useServerTime.ts | Replace console errors with errorLog. |
| src/components/Labs/QRGeneratorSection.tsx | Replace console warnings with warnLog. |
| src/components/EmailVerificationDialog.tsx | Replace console errors with errorLog. |
| src/components/Editor/ItemPropertiesPanel/ItemPropertiesPanel.tsx | Refactor to keyed form subcomponent; switch to useEditorContext hook. |
| src/components/Editor/EditorSidebar/StagingItem.tsx | Switch to useEditorContext hook. |
| src/components/Editor/EditorSidebar/StagingArea.tsx | Switch to useEditorContext hook. |
| src/components/Editor/EditorSidebar/QuickAddDialog.tsx | Refactor dialog content to mount only when open; switch to useEditorContext. |
| src/components/Editor/EditorSidebar/IconUploadDialog.tsx | Replace console errors with errorLog. |
| src/components/Editor/EditorSidebar/EditorSidebar.tsx | Switch to useEditorContext hook. |
| src/components/Editor/EditorHeader/EditorHeader.tsx | Switch to useEditorContext hook; migrate logs to new logger. |
| src/components/Editor/EditorHeader/BackButton.tsx | Switch to useEditorContext hook. |
| src/components/Editor/EditorCanvas/EditorCanvas.tsx | Switch to useEditorContext hook. |
| src/components/Editor/EditorCanvas/DraggableItem.tsx | Switch to useEditorContext hook. |
| src/background/index.ts | Use dev-gated + sanitized logger; reduce sensitive logging. |
| src/background/handlers/oauth.ts | Remove token/code logging; log sanitized HTTP error details only. |
| src/apis/external/rss-parser.ts | Replace console errors with errorLog. |
| src/apis/external/library.ts | Convert to type-only imports; replace console errors with errorLog. |
| src/apis/external/html-parser.ts | Replace console logs/warns/errors with centralized logger. |
| src/apis/external/ecampus.ts | Replace console errors with errorLog. |
| src/apis/client.ts | Use sanitized logger for auth/reauth and parsing failures. |
| src/apis/alerts.ts | Replace ad-hoc console formatting with errorLog. |
| src/App.tsx | Gate “welcome” logs behind dev logger; keep analytics pageview. |
| pnpm-lock.yaml | Reflect dependency/devDependency pruning and moves. |
| package.json | Remove unused deps; move build/type tooling into devDependencies. |
| eslint.config.js | Enforce no-console with exceptions for logger + Vite config. |
| docs/CONTRIBUTING.md | New/updated contributor guidelines incl. logging policy. |
| docs/ARCHITECTURE.md | New/updated architecture map and runtime constraints. |
| AGENTS.md | New/updated coding-agent orientation and project rules. |
| .gitignore | Stop ignoring docs/ so documentation can be committed. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
src/pages/TemplateListPage.tsx:68
useToast()returns a newtoastfunction each render (seesrc/components/ui/use-toast.ts), so includingtoastinloadTemplates'suseCallbackdeps makesloadTemplatesunstable. Because there is auseEffect(() => loadTemplates(), [loadTemplates]), this will re-run on every render and can cause repeated fetches / render loops. Fix by makingtoaststable (e.g., memoize it inuseToastwithuseCallback), or avoid depending ontoasthere (e.g., calluseToast()inside the catch block, or keep the mount effect as[]with an explicit exhaustive-deps suppression).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@gemini-code-assist review this |
There was a problem hiding this comment.
Code Review
This pull request implements a centralized logging system to replace standard console methods with a sanitized logger that masks sensitive information like emails and OAuth tokens, enforced by a new ESLint rule. The PR also introduces extensive documentation for architecture and contributions, refactors several React contexts and hooks for better structure, and optimizes form state management in the editor. Feedback focuses on the new logger utility, specifically addressing a regex syntax error, improving token detection in URL fragments, preventing stack overflows from circular array references, and ensuring the primary log message is sanitized.
|
|
||
| const EMAIL_PATTERN = | ||
| /\b([A-Z0-9._%+-])([A-Z0-9._%+-]*)(@[A-Z0-9.-]+\.[A-Z]{2,})\b/gi; | ||
| const BEARER_PATTERN = /Bearer\s+[A-Z0-9._~+/-]+=*/gi; |
There was a problem hiding this comment.
정규식 리터럴 내에서 슬래시(/)가 이스케이프되지 않아 구문 오류(Syntax Error)가 발생합니다. 또한 Bearer 토큰(특히 JWT)에서 흔히 사용되는 하이픈(-) 문자가 누락되어 있어 마스킹이 불완전할 수 있습니다. 하이픈을 문자 클래스([]) 내부에 추가할 때는 이스케이프하거나 맨 뒤에 배치해야 합니다.
| const BEARER_PATTERN = /Bearer\s+[A-Z0-9._~+/-]+=*/gi; | |
| const BEARER_PATTERN = /Bearer\s+[A-Z0-9._~+\/-]+=*/gi; |
| /\b([A-Z0-9._%+-])([A-Z0-9._%+-]*)(@[A-Z0-9.-]+\.[A-Z]{2,})\b/gi; | ||
| const BEARER_PATTERN = /Bearer\s+[A-Z0-9._~+/-]+=*/gi; | ||
| const TOKEN_QUERY_PATTERN = | ||
| /([?&])(code|access_token|refresh_token|id_token|token)=([^&\s]+)/gi; |
There was a problem hiding this comment.
OAuth 인증 시 토큰이 URL의 해시 프래그먼트(#)에 포함되는 경우가 많습니다 (예: Implicit Flow). 현재 정규식은 쿼리 파라미터(?, &)만 처리하고 있어 프래그먼트에 포함된 민감 정보가 노출될 수 있습니다. 또한 값 매칭 시 #를 구분자로 추가하여 프래그먼트가 값에 포함되지 않도록 개선이 필요합니다.
const TOKEN_QUERY_PATTERN =
/([?&#])(code|access_token|refresh_token|id_token|token)=([^&#\s]+)/gi;| if (Array.isArray(value)) { | ||
| const sanitizedEntries = value | ||
| .slice(0, MAX_ARRAY_LENGTH) | ||
| .map((entry) => sanitizeValue(entry, depth + 1, seen)); |
There was a problem hiding this comment.
배열(Array)에 대한 순환 참조(Circular Reference) 체크가 누락되어 있습니다. 배열 내부에 자기 자신이나 상위 배열이 포함된 경우 무한 재귀로 인한 스택 오버플로우가 발생할 수 있습니다. seen 세트를 사용하여 체크를 추가해야 합니다.
if (Array.isArray(value)) {
if (seen.has(value)) {
return "[Circular]";
}
seen.add(value);
const sanitizedEntries = value
.slice(0, MAX_ARRAY_LENGTH)
.map((entry) => sanitizeValue(entry, depth + 1, seen));| function emitLog(level: LogLevel, message: string, args: unknown[]): void { | ||
| if (!shouldLog(level)) { | ||
| return; | ||
| } | ||
|
|
||
| const sanitizedArgs = args.map((arg) => sanitizeValue(arg)); | ||
|
|
||
| switch (level) { | ||
| case "debug": | ||
| console.log(message, ...sanitizedArgs); | ||
| break; | ||
| case "info": | ||
| console.info(message, ...sanitizedArgs); | ||
| break; | ||
| case "warn": | ||
| console.warn(message, ...sanitizedArgs); | ||
| break; | ||
| case "error": | ||
| console.error(message, ...sanitizedArgs); | ||
| break; | ||
| } |
There was a problem hiding this comment.
emitLog 함수에서 message 문자열 자체에 대한 마스킹 처리가 누락되어 있습니다. 개발자가 로그 메시지에 직접 민감 정보를 포함하는 경우(예: debugLog(User ${email} login))를 대비해 message도 sanitizeString으로 처리하는 것이 안전합니다.
function emitLog(level: LogLevel, message: string, args: unknown[]): void {
if (!shouldLog(level)) {
return;
}
const sanitizedMessage = sanitizeString(message);
const sanitizedArgs = args.map((arg) => sanitizeValue(arg));
switch (level) {
case "debug":
console.log(sanitizedMessage, ...sanitizedArgs);
break;
case "info":
console.info(sanitizedMessage, ...sanitizedArgs);
break;
case "warn":
console.warn(sanitizedMessage, ...sanitizedArgs);
break;
case "error":
console.error(sanitizedMessage, ...sanitizedArgs);
break;
}
}
|
@gemini-code-assist 리뷰해줭 |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive documentation suite (AGENTS.md, ARCHITECTURE.md, and CONTRIBUTING.md) and implements a centralized, sanitized logging system to replace standard console calls, enforced by new ESLint rules. It also refactors several React components and contexts to improve state management and modularity, alongside dependency cleanups in package.json. Feedback focuses on improving the user experience in the template editor by preventing unnecessary component remounts, ensuring UI consistency when switching templates, and adjusting log length limits to preserve useful debugging information.
| <ItemPropertiesPanelForm | ||
| key={getSelectedItemKey(selectedItem)} |
There was a problem hiding this comment.
The key for ItemPropertiesPanelForm includes all item properties (name, URL, icon, etc.). This means that whenever the user clicks "Save" and the context updates, the entire form component will remount. This causes a loss of input focus and cursor position, which is a poor user experience if the user wants to continue editing other fields.
Consider using only item.templateItemId as the key to ensure the form resets only when a different item is selected. To keep the form in sync with external changes or after a save, use a useEffect inside the form component to update local state when props change.
| <ItemPropertiesPanelForm | |
| key={getSelectedItemKey(selectedItem)} | |
| <ItemPropertiesPanelForm | |
| key={selectedItem.templateItemId} |
| } else { | ||
| // Save selection | ||
| await chrome.storage.local.set({ [STORAGE_KEY]: templateId }); | ||
| await storage.local.set({ [STORAGE_KEY]: templateId }); | ||
| setSelectedTemplateId(templateId); |
There was a problem hiding this comment.
When a new templateId is selected, the templateData and linkItems are not reset or set to a loading state. Since the actual data fetching happens asynchronously (likely in a useEffect reacting to selectedTemplateId), the UI will continue to show the data from the previously selected template until the new request completes. This can be confusing for users.
It is recommended to clear the current template data immediately when a new ID is selected to prevent showing stale information.
} else {
// Save selection
await storage.local.set({ [STORAGE_KEY]: templateId });
setTemplateData(null);
setSelectedTemplateId(templateId);
}| @@ -0,0 +1,280 @@ | |||
| const IS_DEV = import.meta.env.DEV; | |||
|
|
|||
| const MAX_STRING_LENGTH = 400; | |||
There was a problem hiding this comment.
A MAX_STRING_LENGTH of 400 characters is quite restrictive for logging error stack traces. Most stack traces will be truncated significantly, making them difficult to use for debugging production issues.
Consider increasing this limit (e.g., to 2000 or 4000) or implementing a specific exception for error objects in the sanitizeValue logic to preserve more of the stack trace while still applying sanitization.
| const MAX_STRING_LENGTH = 400; | |
| const MAX_STRING_LENGTH = 2000; |
|
@gemini-code-assist review this, |
There was a problem hiding this comment.
Code Review
This pull request introduces a centralized logging utility to replace standard console calls, ensuring sensitive information like tokens and emails are redacted. It also adds comprehensive documentation for architecture and contribution guidelines, enforces a 'no-console' ESLint rule, and cleans up unused dependencies. Several components and contexts were refactored to improve state management and stability, particularly in the template editor and storage handling. Feedback focuses on restoring the export of UI variants for reusability, removing a now-unused variable in the todo utility, and further simplifying component logic by utilizing React keys for state resets.
| <ItemPropertiesPanelForm | ||
| selectedItem={selectedItem} |
There was a problem hiding this comment.
ItemPropertiesPanelForm 컴포넌트에 key를 적용하여 selectedItem이 변경될 때마다 컴포넌트가 새로 마운트되도록 개선할 수 있습니다. 이렇게 하면 하위 컴포넌트에서 useEffect를 사용해 props를 state로 복사하는 복잡한 로직을 제거할 수 있으며, React의 선언적인 상태 관리 방식에도 더 부합합니다.
| <ItemPropertiesPanelForm | |
| selectedItem={selectedItem} | |
| <ItemPropertiesPanelForm | |
| key={selectedItem.templateItemId} | |
| selectedItem={selectedItem} |
| useEffect(() => { | ||
| setName(selectedItem.name); | ||
| setUrl(selectedItem.siteUrl); | ||
| setSelectedIconId(selectedItem.icon.iconId); | ||
| setWidth(selectedItem.size.width.toString()); | ||
| setHeight(selectedItem.size.height.toString()); | ||
| setPosX(selectedItem.position.x.toString()); | ||
| setPosY(selectedItem.position.y.toString()); | ||
| }, [selectedItem]); |
There was a problem hiding this comment.
부모 컴포넌트에서 key를 사용하여 컴포넌트를 초기화하도록 변경한다면, 이 useEffect 블록은 더 이상 필요하지 않습니다. Props가 변경될 때마다 컴포넌트 인스턴스가 새로 생성되므로 useState 초기값으로 충분합니다.
| const dueTime = todo.dueTime || "23:59"; | ||
| const dDay = calculateDDay(normalizedDate, dueTime); | ||
| const dDay = calculateDDay(normalizedDate); |
There was a problem hiding this comment.
calculateDDay 함수에서 dueTime 파라미터가 제거되었으므로, 여기서 선언된 dueTime 변수는 더 이상 사용되지 않습니다. 코드 정리를 위해 삭제하는 것이 좋습니다.
| const dueTime = todo.dueTime || "23:59"; | |
| const dDay = calculateDDay(normalizedDate, dueTime); | |
| const dDay = calculateDDay(normalizedDate); | |
| const dDay = calculateDDay(normalizedDate); |
| } | ||
|
|
||
| export { Badge, badgeVariants } | ||
| export { Badge } |
There was a problem hiding this comment.
badgeVariants를 export에서 제외하면 다른 컴포넌트에서 해당 스타일 정의를 재사용할 수 없게 됩니다. shadcn/ui 구성 요소의 유연성을 유지하기 위해 variants를 다시 export하는 것을 권장합니다.
| export { Badge } | |
| export { Badge, badgeVariants } |
| } | ||
|
|
||
| export { Button, buttonVariants } | ||
| export { Button } |
There was a problem hiding this comment.
buttonVariants는 다른 요소(예: Link)를 버튼 스타일로 렌더링할 때 자주 사용됩니다. 이를 export에서 제거하면 UI 일관성을 유지하기 위한 스타일 재사용이 어려워지므로 다시 추가하는 것이 좋습니다.
| export { Button } | |
| export { Button, buttonVariants } |
|
@gemini-code-assist 진짜 찐막 오류 가능성 찾아봐......! |
There was a problem hiding this comment.
Code Review
This pull request implements a centralized logging utility in src/utils/logger.ts to sanitize sensitive data and replaces all console logs throughout the codebase. It also adds extensive documentation, including architecture and contribution guides. Key refactorings include splitting context objects to prevent circular dependencies, improving the useSelectedTemplate hook with stale request handling, and refactoring dialog components to use keys for state resetting. Project dependencies were also cleaned up and optimized. I have no feedback to provide as no review comments were submitted.
변경 요약
변경 배경
확인 사항
😽 링쿠를 더 멋지게 만들어보아요