Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 48 additions & 42 deletions .github/scripts/performance-report.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,54 +30,60 @@ const INTERNAL_ROUTES = new Set(['/_not-found', '/_global-error']);
const BUNDLE_WARN_KB = 200;
const BUNDLE_ERROR_KB = 350;

const sumDirGzipSize = (dir) => {
if (!fs.existsSync(dir)) return 0;
return fs.readdirSync(dir).reduce((sum, file) => {
const full = path.join(dir, file);
return sum + (fs.statSync(full).isFile() ? gzipSize(full) : 0);
}, 0);
const PROJECT_MODULE_PREFIX = '[project]/apps/timo-web/app';

const extractClientReferenceManifest = (jsFilePath) => {
if (!jsFilePath || !fs.existsSync(jsFilePath)) return null;
try {
const content = fs.readFileSync(jsFilePath, 'utf8');
const match = content.match(/"\]\s*=\s*(\{[\s\S]*\});\s*$/);
return match ? JSON.parse(match[1]) : null;
} catch {
return null;
}
};
Comment on lines +35 to 44

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

정규식 파싱의 안정성 개선을 권장합니다

extractClientReferenceManifest의 정규식 "\]\s*=\s*(\{[\s\S]*\});\s*$에서 [\s\S]*가 greedy하게 동작합니다. 매니페스트 파일에 여러 할당문이 존재할 경우, 첫 번째 {부터 마지막 }까지 모두 캡처하여 중간 할당문 내용이 포함되고 JSON.parse가 실패합니다. 실패 시 null이 반환되어 해당 라우트의 크기가 0바이트로 보고되는 침묵 오류가 발생합니다.

lastIndexOf를 활용한 방식이 더 견고합니다.

♻️ 제안하는 리팩터
 const extractClientReferenceManifest = (jsFilePath) => {
   if (!jsFilePath || !fs.existsSync(jsFilePath)) return null;
   try {
     const content = fs.readFileSync(jsFilePath, 'utf8');
-    const match = content.match(/"\]\s*=\s*(\{[\s\S]*\});\s*$/);
-    return match ? JSON.parse(match[1]) : null;
+    const lastAssign = content.lastIndexOf('"]');
+    if (lastAssign === -1) return null;
+    const jsonStart = content.indexOf('{', lastAssign);
+    const jsonEnd = content.lastIndexOf('}');
+    if (jsonStart === -1 || jsonEnd === -1 || jsonStart > jsonEnd) return null;
+    return JSON.parse(content.slice(jsonStart, jsonEnd + 1));
   } catch {
     return null;
   }
 };
📝 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.

Suggested change
const extractClientReferenceManifest = (jsFilePath) => {
if (!jsFilePath || !fs.existsSync(jsFilePath)) return null;
try {
const content = fs.readFileSync(jsFilePath, 'utf8');
const match = content.match(/"\]\s*=\s*(\{[\s\S]*\});\s*$/);
return match ? JSON.parse(match[1]) : null;
} catch {
return null;
}
};
const extractClientReferenceManifest = (jsFilePath) => {
if (!jsFilePath || !fs.existsSync(jsFilePath)) return null;
try {
const content = fs.readFileSync(jsFilePath, 'utf8');
const lastAssign = content.lastIndexOf('"]');
if (lastAssign === -1) return null;
const jsonStart = content.indexOf('{', lastAssign);
const jsonEnd = content.lastIndexOf('}');
if (jsonStart === -1 || jsonEnd === -1 || jsonStart > jsonEnd) return null;
return JSON.parse(content.slice(jsonStart, jsonEnd + 1));
} catch {
return null;
}
};
🧰 Tools
🪛 ast-grep (0.44.1)

[warning] 37-37: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(jsFilePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/scripts/performance-report.js around lines 35 - 44, The greedy regex
in extractClientReferenceManifest can capture too much when the manifest
contains multiple assignments, causing JSON.parse to fail and silently return
null. Replace the regex-based extraction with a more robust approach in
extractClientReferenceManifest, such as locating the target assignment using
lastIndexOf and slicing only the intended JSON object before parsing. Keep the
null fallback for missing files or parse errors, but ensure the captured
substring is limited to the actual manifest payload.


const stripRouteGroups = (routePath) =>
routePath
.split('/')
.filter((segment) => !(segment.startsWith('(') && segment.endsWith(')')))
.join('/') || '/';

// Turbopack (Next.js 16's default bundler) does not emit `app-build-manifest.json`
// or `static/chunks/app/<route>/`, so per-route sizes come from each route's
// `page_client-reference-manifest.js` (entryJSFiles already accumulates the page's
// own chunk plus every parent layout chunk, deduped against the shared root chunks).
const analyzeBundle = () => {
const buildManifest = readJson(path.join(BUILD_DIR, 'build-manifest.json'));
if (!buildManifest) return null;

const routesManifest = readJson(path.join(BUILD_DIR, 'routes-manifest.json'));
const appBuildManifest = readJson(path.join(BUILD_DIR, 'app-build-manifest.json'));

const sharedBytes = (buildManifest.rootMainFiles ?? []).reduce(
(sum, chunk) => sum + gzipSize(path.join(BUILD_DIR, chunk)),
0
);

let routes;
if (appBuildManifest) {
routes = Object.entries(appBuildManifest.pages ?? {})
.filter(([route]) => !INTERNAL_ROUTES.has(route))
.map(([route, chunks]) => {
const pageBytes = chunks.reduce((sum, c) => sum + gzipSize(path.join(BUILD_DIR, c)), 0);
const firstLoad = pageBytes + sharedBytes;
return { path: route, size: formatBytes(pageBytes), firstLoad: formatBytes(firstLoad), firstLoadKb: firstLoad / 1024 };
});
} else {
const allRoutes = [
...(routesManifest?.staticRoutes ?? []),
...(routesManifest?.dynamicRoutes ?? []),
];
const chunksDir = path.join(BUILD_DIR, 'static', 'chunks', 'app');
routes = allRoutes
.filter(({ page }) => !INTERNAL_ROUTES.has(page))
.map(({ page }) => {
const pageBytes = sumDirGzipSize(path.join(chunksDir, page === '/' ? '' : page));
const firstLoad = pageBytes + sharedBytes;
return { path: page, size: formatBytes(pageBytes), firstLoad: formatBytes(firstLoad), firstLoadKb: firstLoad / 1024 };
});

if (routes.length === 0 && sharedBytes > 0) {
routes = [{ path: '/', size: '0 B', firstLoad: formatBytes(sharedBytes), firstLoadKb: sharedBytes / 1024 }];
}
const appPathsManifest = readJson(path.join(BUILD_DIR, 'server', 'app-paths-manifest.json'));
if (!buildManifest || !appPathsManifest) return null;

// Turbopack의 산출물 구조는 비공식이라 rootMainFiles/entryJSFiles 키가
// 바뀌면 번들 크기가 조용히 0으로 계산될 수 있다. CI에서 바로 드러나도록 명시적으로 검증한다.
if (!Array.isArray(buildManifest.rootMainFiles)) {
console.warn('[bundle] build-manifest.json에 rootMainFiles 배열이 없습니다. Next.js 빌드 산출물 구조가 변경되었을 수 있습니다.');
return null;
}

const sharedFiles = new Set(buildManifest.rootMainFiles);
const sharedBytes = [...sharedFiles].reduce((sum, chunk) => sum + gzipSize(path.join(BUILD_DIR, chunk)), 0);

const routes = Object.entries(appPathsManifest)
.filter(([entryKey]) => !INTERNAL_ROUTES.has(entryKey.replace(/\/page$/, '')))
.map(([entryKey, relPath]) => {
const routePath = stripRouteGroups(entryKey.replace(/\/page$/, ''));
const manifestPath = path.join(BUILD_DIR, 'server', relPath.replace(/\.js$/, '_client-reference-manifest.js'));
const manifest = extractClientReferenceManifest(manifestPath);
if (!manifest?.entryJSFiles || typeof manifest.entryJSFiles !== 'object') {
console.warn(`[bundle] ${routePath} 라우트의 entryJSFiles를 찾을 수 없습니다 (${manifestPath}).`);
}
const pageFiles = (manifest?.entryJSFiles?.[`${PROJECT_MODULE_PREFIX}${entryKey}`] ?? [])
.filter((file) => !sharedFiles.has(file));
const pageBytes = pageFiles.reduce((sum, file) => sum + gzipSize(path.join(BUILD_DIR, file)), 0);
const firstLoad = pageBytes + sharedBytes;
return { path: routePath, size: formatBytes(pageBytes), firstLoad: formatBytes(firstLoad), firstLoadKb: firstLoad / 1024 };
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return { routes, sharedSize: formatBytes(sharedBytes) };
};

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/performance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: ✨ Performance Report

on:
pull_request:
types: [opened, reopened]
types: [opened, reopened, synchronize]
branches: [main, develop]
workflow_dispatch:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,13 @@ export const NavigationSidebar = () => {
>
<div className="flex h-full w-45 shrink-0 flex-col gap-7.5">
<Link href={ROUTES.HOME}>
<Image src={timoTextLogo} alt="Timo" width={92} height={35} />
<Image
src={timoTextLogo}
alt="Timo"
width={92}
height={35}
priority
/>
</Link>
<nav className="flex flex-1 flex-col justify-between">
<div className="flex flex-col gap-2">
Expand Down
17 changes: 8 additions & 9 deletions apps/timo-web/lighthouserc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@
module.exports = {
ci: {
collect: {
// "/en", "/en/onboarding", and the "/en/settings*" routes are still
// placeholder pages (`return <></>`) with no content to paint, so
// Lighthouse always times out with NO_FCP there — omit them until
// they're implemented.
url: [
"http://localhost:3000",
"http://localhost:3000/home",
"http://localhost:3000/today",
"http://localhost:3000/focus",
"http://localhost:3000/statistics",
"http://localhost:3000/onboarding",
"http://localhost:3000/settings",
"http://localhost:3000/settings/account",
"http://localhost:3000/settings/term",
"http://localhost:3000/en/home",
"http://localhost:3000/en/today",
"http://localhost:3000/en/focus",
"http://localhost:3000/en/statistics",
Comment on lines +12 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR 설명에 존재하지 않던 /settings/term을 실제 라우트 /settings/policy로 수정했습니다" 라고 적어주셨는데 해당 페이지가 아직 머지되지않아 의도적으로 뺴두신게 맞을까요?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

옙`~~ 페이지 내부에 아무 콘텐츠가 없으면 CI가 실패하는 문제가 있어서, 퍼블리싱 시작되는 대로 추가해야 할 것 같아요.

],
numberOfRuns: 3,
settings: {
Expand Down
Loading