Skip to content

♻️ Refactor error handling and code readability across modules#70

Merged
romantech merged 7 commits intomainfrom
patch
Mar 1, 2026
Merged

♻️ Refactor error handling and code readability across modules#70
romantech merged 7 commits intomainfrom
patch

Conversation

@romantech
Copy link
Copy Markdown
Owner

@romantech romantech commented Mar 1, 2026

PR Type

Enhancement, Tests


Description

  • Refactor manual chunk splitting strategy for improved cache efficiency

    • Replace static dependency-based chunks with dynamic module path analysis
    • Organize packages into semantic groups (React, Router, UI, State, Animation, Utils)
    • Prevent "empty chunk" warnings by checking actual node_modules inclusion
  • Migrate ESLint from v8 to v10 with flat config format

    • Replace .eslintrc with new eslint.config.js using flat config API
    • Update all ESLint plugins to v10-compatible versions
    • Add @eslint/compat for backward compatibility with legacy plugin configs
  • Upgrade major dependencies and improve code quality

    • Update React to v19, Vite to v7, TypeScript ESLint to v8
    • Replace Jotai DevTools with lazy-loaded component for better performance
    • Fix code readability issues (unused variables, error handling, variable declarations)
  • Refactor Jotai Babel configuration and add lazy-loaded DevTools

    • Switch from individual plugins to jotai-babel/preset for cleaner setup
    • Create lazy-loaded JotaiDevTools wrapper component with Suspense
    • Only load DevTools in development environment

Diagram Walkthrough

flowchart LR
  A["vite.config.ts"] -->|"Dynamic chunk splitting"| B["Semantic package groups"]
  C["ESLint v8"] -->|"Migrate to flat config"| D["eslint.config.js"]
  E["Dependencies"] -->|"Major upgrades"| F["React v19, Vite v7, TS-ESLint v8"]
  G["Jotai DevTools"] -->|"Lazy load with Suspense"| H["JotaiDevTools component"]
  B --> I["Improved cache efficiency"]
  D --> J["Better plugin compatibility"]
  F --> K["Enhanced tooling"]
  H --> L["Reduced bundle size"]
Loading

File Walkthrough

Relevant files
Enhancement
5 files
vite.config.ts
Refactor chunk splitting and Babel config                               
+53/-36 
index.ts
Export new dev components module                                                 
+1/-0     
index.ts
Create dev components barrel export                                           
+1/-0     
jotai-devtools.tsx
Add lazy-loaded Jotai DevTools component                                 
+19/-0   
syntax-editor-root.tsx
Replace direct DevTools with lazy component                           
+2/-3     
Configuration changes
2 files
eslint.config.js
Migrate ESLint v8 to v10 flat config                                         
+129/-0 
link-particles.tsx
Update ESLint rule name for v10 compatibility                       
+1/-1     
Dependencies
1 files
package.json
Upgrade dependencies and add new packages                               
+29/-26 
Bug fix
3 files
selection.ts
Fix variable declaration scope issue                                         
+2/-5     
use-is-mounted.ts
Fix render-time state change with setTimeout                         
+5/-1     
axios.ts
Rename import to avoid naming conflicts                                   
+2/-2     
Code quality
1 files
use-local-storage.ts
Remove unused error parameter in catch blocks                       
+2/-2     
Refactoring
2 files
error-component.tsx
Remove useRef and simplify error message handling               
+4/-6     
analysis-form.tsx
Extract ESkeleton component outside function                         
+4/-4     
Additional files
3 files
.eslintrc.cjs +0/-91   
pre-commit +0/-3     
pnpm-lock.yaml +1202/-1688

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 1, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
syntax-analyzer Building Building Preview, Comment Mar 1, 2026 11:54am

@qodo-code-review
Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status:
Non-descriptive variable: The new variable name p is too generic to be self-documenting for a normalized module path
value.

Referred Code
const p = id.replace(/\\/g, '/');

// pnpm 심볼릭 링크 구조 대응: .../.pnpm/pkg@ver/node_modules/pkg/...
const inPkg = (name: string) => p.includes(`/node_modules/${name}/`);

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Swallowed error context: The new catch { ... } blocks discard the exception details, reducing debugging context for
localStorage/JSON.parse failures.

Referred Code
  } catch {
    logError('setting', key);
  }
};

const safeGetItem = <T>(key: string, defaultValue: T) => {
  if (!isLocalStorageAvailable()) return defaultValue;

  try {
    const storedValue = window.localStorage.getItem(key);
    if (storedValue) return JSON.parse(storedValue) as T;
  } catch {
    logError('getting', key);
  }

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
User-facing raw error: The UI directly renders error?.message and route statusText, which may expose
internal/sensitive implementation details to end users.

Referred Code
let errorMessage = error?.message ?? '문제가 발생했어요';

const isRouteError = isRouteErrorResponse(routerError);
if (isRouteError) {
  const { status, statusText } = routerError;
  errorMessage = `${status} | ${statusText}`;
}

const onActionButtonClick = isRouteError
  ? () => setShouldRedirect.on()
  : resetErrorBoundary;

const buttonText = isRouteError ? '돌아가기' : '다시 시도하기';

if (shouldRedirect) return <Navigate to=".." replace />;

return (
  <Layout
    display="flex"
    flexDirection="column"
    justifyContent="center"


 ... (clipped 19 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Logging content unknown: The PR calls logError('setting'|'getting', key) but the structure and
sensitivity of the resulting log output cannot be verified from this diff.

Referred Code
    logError('setting', key);
  }
};

const safeGetItem = <T>(key: string, defaultValue: T) => {
  if (!isLocalStorageAvailable()) return defaultValue;

  try {
    const storedValue = window.localStorage.getItem(key);
    if (storedValue) return JSON.parse(storedValue) as T;
  } catch {
    logError('getting', key);
  }

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Consider re-evaluating the removal of the pre-commit hook

The PR removes the .husky/pre-commit file, which acts as a critical quality
gate. The suggestion advises to either explain this removal or restore the file
to prevent potential issues with code quality.

Examples:

.husky/pre-commit [1]
pnpx lint-staged

Solution Walkthrough:

Before:

# .husky/pre-commit
#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

# Run lint-staged to check staged files
npx lint-staged

After:

# .husky/pre-commit
# This file is deleted in the PR.
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies the removal of the .husky/pre-commit hook, a critical quality gate whose absence could degrade code quality across the entire project.

High
Possible issue
Update React types for compatibility

Update the @types/react dependency to a version compatible with React 19 to
ensure type safety and prevent build errors.

package.json [72]

-"@types/react": "^18.3.28",
+"@types/react": "^19.0.0",
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical version mismatch between react and @types/react which would lead to type errors and potentially break the build.

High
Remove unnecessary and potentially buggy setTimeout

Remove the setTimeout wrapper from the setMounted call inside useEffect. A
direct state update is safer and more idiomatic, as useEffect runs after
rendering is complete.

src/base/hooks/use-is-mounted.ts [11-21]

 useEffect(() => {
-  // 마운트 직후(다음 tick)에만 true로 바꿔서 "렌더 중 state 변경" ESLint 경고 대응
-  const timer = setTimeout(() => {
-    setMounted(true);
-  }, 0);
+  setMounted(true);
 
   return () => {
-    clearTimeout(timer);
     setMounted(false);
   };
 }, []);
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that using setTimeout is a workaround that can introduce subtle bugs, and reverting to a direct state update in useEffect is more idiomatic and safer. The PR author added this to fix a lint warning, but this suggestion provides a better long-term solution.

Low
Fix incorrect import sorting configuration

Correct the value of pathGroupsExcludedImportTypes in the ESLint configuration.
The value should be the pattern '{react,react-dom,react-dom/*}' instead of
'react' to properly exclude React imports from alphabetization.

eslint.config.js [82-108]

 'import-x/order': [
   'warn',
   {
     // 그룹 순서 지정
     groups: [
       'builtin', // Built-in imports go first
       'external', // External imports
       'internal', // Absolute imports
       ['parent', 'sibling'], // Relative imports from siblings and parents can mix
       'index', // index imports
       'object',
       'type',
     ],
     'newlines-between': 'always',
     // 패턴으로 세부적인 순서 지정
     pathGroups: [
       {
         pattern: '{react,react-dom,react-dom/*}',
         group: 'external',
         position: 'before',
       },
     ],
     // 리액트 패키지는 external 그룹에서 알파벳 순이 아닌 상단에 위치시키기 위해 예외 처리
-    pathGroupsExcludedImportTypes: ['react'],
+    pathGroupsExcludedImportTypes: ['{react,react-dom,react-dom/*}'],
     alphabetize: { order: 'asc', caseInsensitive: true },
   },
 ],
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: This suggestion correctly identifies and fixes a misconfiguration in the eslint-plugin-import-x rules, ensuring that the import sorting logic for React packages works as intended by the developer.

Low
Correct ESLint disable rule

Correct the ESLint disable directive from import-x/no-named-as-default to
import/no-named-as-default to fix a typo.

src/base/components/ui/link-particles.tsx [6]

-// eslint-disable-next-line import-x/no-named-as-default
+// eslint-disable-next-line import/no-named-as-default
  • Apply / Chat
Suggestion importance[1-10]: 3

__

Why: The suggestion correctly identifies and fixes a typo in an ESLint disable directive (import-x instead of import) that was introduced in the PR, ensuring the linter configuration is effective.

Low
General
Exclude development-only preset from production build

Conditionally apply the jotai-babel/preset only in development mode within
vite.config.ts. This preset includes development-only features and should be
excluded from production builds for optimization.

vite.config.ts [56-75]

 export default defineConfig(({ mode }) => ({
   plugins: [
     /**
      * vite-plugin-react
      * @see https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react/README.md
      * */
     react({
       babel: {
         /**
          * plugin-react-refresh + plugin-debug-label 둘 다 포함
          * [jotai/babel/plugin-react-refresh]
          * Jotai 아톰에 대한 React Refresh 지원 플러그인
          * React Refresh 핫리로딩은 코드 변경사항을 반영하면서 상태는 유지하는 기능
          *
          * [jotai/babel/plugin-debug-label]
          * Jotai 는 리코일 처럼 키(key)가 아닌 객체 참조 기반 작동 -> 아톰 식별자 없음
          * 수동으로 debugLabel 을 추가할 수 있지만 번거로움.
          * 아래 플러그인을 사용하면 모든 아톰에 debugLabel 추가해줌(개발자 도구에서 확인 可)
          * */
-        presets: ['jotai-babel/preset'],
+        presets: mode !== 'production' ? ['jotai-babel/preset'] : [],
       },
     }),
     tsconfigPaths(),
     mode === 'analyze' ? (visualizer() as unknown as PluginOption) : undefined,

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: This is a valid optimization that correctly identifies that the jotai-babel/preset is for development only and should be excluded from production builds to reduce bundle size and overhead.

Medium
Prevent unnecessary imports in production

Move the lazy definition for LazyDevTools inside the JotaiDevTools component to
ensure the dynamic imports are only triggered in a development environment.

src/base/components/dev/jotai-devtools.tsx [5-19]

-const LazyDevTools = lazy(() =>
-  Promise.all([
-    import('jotai-devtools'),
-    import('jotai-devtools/styles.css'),
-  ]).then(([m]) => ({ default: m.DevTools })),
-);
-
 export function JotaiDevTools(props: DevToolsProps) {
   if (!import.meta.env.DEV) return null;
+
+  const LazyDevTools = lazy(() =>
+    Promise.all([
+      import('jotai-devtools'),
+      import('jotai-devtools/styles.css'),
+    ]).then(([m]) => ({ default: m.DevTools })),
+  );
+
   return (
     <Suspense fallback={null}>
       <LazyDevTools {...props} />
     </Suspense>
   );
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential performance issue where dev-only dependencies might be bundled in production and proposes moving the lazy import inside the component to prevent this.

Low
Compute message with useMemo

Refactor the errorMessage derivation to use the useMemo hook instead of a
mutable let variable for a more declarative style.

src/features/misc/pages/error-component.tsx [35-41]

-let errorMessage = error?.message ?? '문제가 발생했어요';
-if (isRouteError) {
-  const { status, statusText } = routerError;
-  errorMessage = `${status} | ${statusText}`;
-}
+const errorMessage = useMemo(() => {
+  if (isRouteError) {
+    const { status, statusText } = routerError;
+    return `${status} | ${statusText}`;
+  }
+  return error?.message ?? '문제가 발생했어요';
+}, [isRouteError, routerError, error?.message]);
  • Apply / Chat
Suggestion importance[1-10]: 2

__

Why: The suggestion to use useMemo is a stylistic preference and offers no performance benefit here, as the variable is recalculated on every render in both the existing and suggested code.

Low
  • More

@romantech romantech linked an issue Mar 1, 2026 that may be closed by this pull request
@romantech romantech merged commit d72a1f6 into main Mar 1, 2026
5 checks passed
@romantech romantech deleted the patch branch March 1, 2026 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ESLint 8 > 10 Migration

1 participant