Conversation
📦 Bundle Size Comparison📈 nuxi
📈 nuxt-cli
📈 create-nuxt
|
commit: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CLI CPU profiling support across nuxi: introduces exported Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nuxi/src/commands/build.ts`:
- Around line 67-68: The issue is that spreading ctx.data?.overrides after
adding { debug: { perf: perfValue } } replaces any existing overrides.debug and
can silently discard --profile; fix by merging the debug objects instead of
replacing them: build a merged debug object from ctx.data?.overrides?.debug and
the new perf entry (e.g. merge existing debug first, then add perf if perfValue)
and then spread that merged debug into the final options while still spreading
the rest of ctx.data?.overrides; update the construction that currently uses
...(perfValue && { debug: { perf: perfValue } }), ...ctx.data?.overrides to use
the merged debug so both existing debug keys and the perf flag are preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 91ef465f-202c-404a-a1ca-c975b68814e6
📒 Files selected for processing (3)
packages/nuxi/src/commands/_shared.tspackages/nuxi/src/commands/build.tspackages/nuxi/src/dev/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/nuxi/src/dev/index.ts
- packages/nuxi/src/commands/_shared.ts
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/nuxi/bin/nuxi.mjs (1)
29-41:⚠️ Potential issue | 🟠 MajorSame profiler bootstrap error-handling bug as the sibling entrypoint.
This block also ignores
Profiler.enableerrors and resolves the promise onProfiler.starterrors, so it can cache acpuProfileSessionthat never actually started.Node.js inspector Session.post callback signature and how errors are reported for Profiler.enable / Profiler.start🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nuxi/bin/nuxi.mjs` around lines 29 - 41, The profiler bootstrap currently ignores errors from inspector.Session.post calls and may set globalThis.__nuxt_cli__.cpuProfileSession even if Profiler.enable or Profiler.start failed; update the boot logic around inspector.Session/ session.post so the Profiler.enable callback checks its first (error) argument and only calls session.post('Profiler.start') when enable succeeded, and the Profiler.start callback also checks its error argument and only then assigns globalThis.__nuxt_cli__.cpuProfileSession = session; if either callback reports an error, disconnect the session and resolve the promise without setting cpuProfileSession to avoid caching a non-started session.packages/nuxi/src/commands/build.ts (1)
71-77:⚠️ Potential issue | 🟡 Minor
ctx.data?.overridesstill overwrites the mergeddebugblock.Because the full overrides object is spread after the merged
debugvalue, any caller that already passesctx.data?.overrides.debugreplacesperfagain. That still makes--profilea no-op for composed callers.Suggested fix
- ...((perfValue || ctx.data?.overrides?.debug) && { - debug: { - ...ctx.data?.overrides?.debug, - ...(perfValue && { perf: perfValue }), - }, - }), ...ctx.data?.overrides, + ...((perfValue || ctx.data?.overrides?.debug) && { + debug: { + ...ctx.data?.overrides?.debug, + ...(perfValue && { perf: perfValue }), + }, + }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nuxi/src/commands/build.ts` around lines 71 - 77, The merged debug block is being overwritten because ...ctx.data?.overrides is spread after the constructed debug object; update the merge order so the final overrides preserve the injected perf value by either (A) spreading ctx.data?.overrides first and then spreading the constructed debug block after, or (B) build a single overrides object like overrides = { ...ctx.data?.overrides, debug: { ...ctx.data?.overrides?.debug, ...(perfValue && { perf: perfValue }) } } and use that; reference the existing symbols ctx.data?.overrides, debug and perfValue when making the change so --profile (perfValue) wins over callers' debug.debug.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nuxi/src/commands/build.ts`:
- Around line 43-47: The profiling is started unconditionally when profileArg is
set (profileArg and startCpuProfile in build.ts) but not reliably stopped on
errors; wrap the entire profiled build region (everything after startCpuProfile
up through where stopCpuProfile should run) in a try/finally so stopCpuProfile
is always called (await it if it becomes async), using profileArg to gate
start/stop and ensuring perfValue logic remains the same; locate
startCpuProfile/stopCpuProfile calls in the build command and move
stopCpuProfile into the finally block to guarantee cleanup on exceptions.
In `@packages/nuxi/src/utils/profile.ts`:
- Around line 17-31: The Profiler.enable callback currently ignores its error
and can allow Profiler.start to run on a failed session and leave a connected
session in global state; update the startup Promise so the callback for
session.post('Profiler.enable', ...) checks the error and rejects if present,
and in any rejection path disconnect the session (call session.disconnect()) and
clear the global session reference (set session = undefined) before
rethrowing/returning the error; ensure the same cleanup happens if
Profiler.start's callback returns an error so the session is not left connected
in a broken state.
- Around line 34-55: stopCpuProfile currently returns before the inspector
callback runs and can skip s.disconnect; change stopCpuProfile to return a
Promise<string|undefined> (make it async) so it waits for the Profiler.stop
callback to finish and resolves with outPath only after the file write
completes; ensure you always call s.disconnect() even on errors, reject or
resolve accordingly, and keep the early-return when there is no session by
returning Promise.resolve(undefined); update callers (e.g., build.ts and
dev/index.ts) to await stopCpuProfile(...) so exit handlers get the correct
value.
In `@packages/nuxt-cli/bin/nuxi.mjs`:
- Around line 29-41: The profiler startup promise currently resolves even if
inspector.Session.post('Profiler.enable') or session.post('Profiler.start')
return errors, which stores a broken session in
globalThis.__nuxt_cli__.cpuProfileSession; change the new Promise used around
inspector.Session() so both post callbacks check their error argument and reject
the promise on error (only resolve after Profiler.start succeeds), and only
assign globalThis.__nuxt_cli__.cpuProfileSession = session after the promise
fulfills successfully; reference inspector.Session,
session.post('Profiler.enable'), session.post('Profiler.start') and
globalThis.__nuxt_cli__.cpuProfileSession when making the change.
---
Duplicate comments:
In `@packages/nuxi/bin/nuxi.mjs`:
- Around line 29-41: The profiler bootstrap currently ignores errors from
inspector.Session.post calls and may set
globalThis.__nuxt_cli__.cpuProfileSession even if Profiler.enable or
Profiler.start failed; update the boot logic around inspector.Session/
session.post so the Profiler.enable callback checks its first (error) argument
and only calls session.post('Profiler.start') when enable succeeded, and the
Profiler.start callback also checks its error argument and only then assigns
globalThis.__nuxt_cli__.cpuProfileSession = session; if either callback reports
an error, disconnect the session and resolve the promise without setting
cpuProfileSession to avoid caching a non-started session.
In `@packages/nuxi/src/commands/build.ts`:
- Around line 71-77: The merged debug block is being overwritten because
...ctx.data?.overrides is spread after the constructed debug object; update the
merge order so the final overrides preserve the injected perf value by either
(A) spreading ctx.data?.overrides first and then spreading the constructed debug
block after, or (B) build a single overrides object like overrides = {
...ctx.data?.overrides, debug: { ...ctx.data?.overrides?.debug, ...(perfValue &&
{ perf: perfValue }) } } and use that; reference the existing symbols
ctx.data?.overrides, debug and perfValue when making the change so --profile
(perfValue) wins over callers' debug.debug.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 537bcdcf-1f7e-4d18-b05b-4c01addaaecd
📒 Files selected for processing (6)
packages/nuxi/bin/nuxi.mjspackages/nuxi/src/commands/build.tspackages/nuxi/src/dev/index.tspackages/nuxi/src/utils/profile.tspackages/nuxt-cli/bin/nuxi.mjstypes.d.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nuxi/src/dev/index.ts
packages/nuxi/src/utils/profile.ts
Outdated
| export function stopCpuProfile(outDir: string): string | undefined { | ||
| if (!session) { | ||
| return | ||
| } | ||
| const s = session | ||
| session = undefined | ||
| let outPath: string | undefined | ||
| s.post('Profiler.stop', (_err, params) => { | ||
| if (_err || !params?.profile) { | ||
| return | ||
| } | ||
| outPath = join(outDir, `profile-${profileCount++}.cpuprofile`) | ||
| try { | ||
| mkdirSync(outDir, { recursive: true }) | ||
| writeFileSync(outPath, JSON.stringify(params.profile)) | ||
| logger.info(`CPU profile written to ${colors.cyan(outPath)}`) | ||
| logger.info(`Open it in ${colors.cyan('https://www.speedscope.app')} or Chrome DevTools`) | ||
| } | ||
| catch {} | ||
| s.disconnect() | ||
| }) | ||
| return outPath |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Can asynchronous work scheduled inside process.on('exit') run in Node.js, and does inspector.Session.post complete synchronously or via a later callback?
💡 Result:
No.
-
In
process.on('exit', ...), listeners must only do synchronous work; Node exits immediately after the'exit'listeners finish, and any work you schedule onto the event loop (timers, I/O, promise continuations, etc.) is abandoned (e.g., asetTimeout(..., 0)“will not run”). [1]
If you need to extend the life of the process to finish async cleanup, use'beforeExit'(when the process is exiting naturally), because a'beforeExit'handler may schedule async work and keep the process alive. [2] -
inspector.Session.post(...)is not “synchronously complete” in the sense of “you have the result whenpost()returns”. It posts a message and the result is delivered later, via the callback (“notified when a response is received”) or, in the promises API, by the returnedPromise. [3]
So, callingsession.post(..., cb)inside an'exit'handler should not be expected to runcb, because that requires the event loop to continue. [1][3]
Sources
[1] Node.js docs — process 'exit' event: synchronous-only; queued work abandoned https://nodejs.org/api/process.html
[2] Node.js docs — process 'beforeExit' event: async work can keep process alive https://nodejs.org/api/process.html
[3] Node.js docs — inspector.Session.post: callback notified when response is received / promises API https://nodejs.org/api/inspector.html
🏁 Script executed:
# Find calls to stopCpuProfile in the codebase
rg "stopCpuProfile" --type typescript --type ts --context 3Repository: nuxt/cli
Length of output: 86
🏁 Script executed:
# Find calls to stopCpuProfile in the codebase
rg "stopCpuProfile" -A 3 -B 3Repository: nuxt/cli
Length of output: 2162
🏁 Script executed:
# Get the full context of the dev/index.ts call
rg "process.once\('exit'" -A 1 -B 2 packages/nuxi/src/dev/index.tsRepository: nuxt/cli
Length of output: 139
stopCpuProfile returns before the profile is written and does not run in exit handlers.
Profiler.stop completes asynchronously, so this function returns while outPath is still undefined and before the file write runs. In packages/nuxi/src/dev/index.ts, this is called from process.once('exit', ...), where async inspector callbacks will not execute because Node.js abandons the event loop once 'exit' listeners finish. The build flow also receives an incorrect return value. Additionally, the early return on error skips s.disconnect().
Suggested fix
-export function stopCpuProfile(outDir: string): string | undefined {
+export async function stopCpuProfile(outDir: string): Promise<string | undefined> {
if (!session) {
return
}
const s = session
session = undefined
- let outPath: string | undefined
- s.post('Profiler.stop', (_err, params) => {
- if (_err || !params?.profile) {
- return
- }
- outPath = join(outDir, `profile-${profileCount++}.cpuprofile`)
+ try {
+ const params = await new Promise<any>((resolve, reject) => {
+ s.post('Profiler.stop', (err, params) => {
+ if (err) {
+ return reject(err)
+ }
+ resolve(params)
+ })
+ })
+ if (!params?.profile) {
+ return
+ }
+ const outPath = join(outDir, `profile-${profileCount++}.cpuprofile`)
try {
mkdirSync(outDir, { recursive: true })
writeFileSync(outPath, JSON.stringify(params.profile))
logger.info(`CPU profile written to ${colors.cyan(outPath)}`)
logger.info(`Open it in ${colors.cyan('https://www.speedscope.app')} or Chrome DevTools`)
}
catch {}
- s.disconnect()
- })
- return outPath
+ return outPath
+ }
+ finally {
+ s.disconnect()
+ }
}Update call sites in build.ts and dev/index.ts to use await stopCpuProfile(...).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nuxi/src/utils/profile.ts` around lines 34 - 55, stopCpuProfile
currently returns before the inspector callback runs and can skip s.disconnect;
change stopCpuProfile to return a Promise<string|undefined> (make it async) so
it waits for the Profiler.stop callback to finish and resolves with outPath only
after the file write completes; ensure you always call s.disconnect() even on
errors, reject or resolve accordingly, and keep the early-return when there is
no session by returning Promise.resolve(undefined); update callers (e.g.,
build.ts and dev/index.ts) to await stopCpuProfile(...) so exit handlers get the
correct value.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/nuxi/src/commands/build.ts (1)
91-93: Consider adding an explanatory comment inside the catch block.The empty catch is intentional for backward compatibility, but the placeholder
//comment doesn't convey the intent. Consider adding a brief note like// Expected in Bridge or older Nuxt versions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nuxi/src/commands/build.ts` around lines 91 - 93, Update the empty catch block in packages/nuxi/src/commands/build.ts to include a concise explanatory comment explaining the intentional no-op for backward compatibility (e.g., note about Bridge or older Nuxt versions) so future readers understand this is deliberate; locate the catch associated with the build command logic and replace the placeholder `//` with a short comment referencing that it's an expected no-op for compatibility rather than swallowing an unexpected error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nuxi/src/commands/build.ts`:
- Around line 99-102: The hook registered on nuxt.hook('build:error') currently
calls logger.error(...) then process.exit(1), which bypasses the surrounding
try/finally and loses CPU profiling data; change this to either re-throw the
error (throw err) so the error propagates and the finally block runs, or call
stopCpuProfile() (the profiler shutdown function used elsewhere) before calling
process.exit(1) so profiling is saved; update the handler attached to
'build:error' (and remove direct process.exit(1) usage) to ensure finally logic
that writes profiling data is always executed.
---
Nitpick comments:
In `@packages/nuxi/src/commands/build.ts`:
- Around line 91-93: Update the empty catch block in
packages/nuxi/src/commands/build.ts to include a concise explanatory comment
explaining the intentional no-op for backward compatibility (e.g., note about
Bridge or older Nuxt versions) so future readers understand this is deliberate;
locate the catch associated with the build command logic and replace the
placeholder `//` with a short comment referencing that it's an expected no-op
for compatibility rather than swallowing an unexpected error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07f7b316-de8d-49bc-a883-c7a39843f6ff
📒 Files selected for processing (3)
packages/nuxi/src/commands/build.tspackages/nuxi/src/utils/profile.tspackages/nuxt-cli/bin/nuxi.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/nuxt-cli/bin/nuxi.mjs
- packages/nuxi/src/utils/profile.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/nuxi/src/utils/profile.ts (1)
48-70:⚠️ Potential issue | 🟠 MajorReturn the written path from
stopCpuProfile.This function is typed as
Promise<string | undefined>, but the success path never returnsoutPath, so callers always getundefinedeven after the profile is flushed.Suggested fix
- const outPath = join(outDir, `profile-${profileCount++}.cpuprofile`) try { - await new Promise<any>((resolve, reject) => { + const params = await new Promise<any>((resolve, reject) => { s.post('Profiler.stop', (err, params) => { if (err) { return reject(err) } - - if (!params?.profile) { - return resolve(params) - } - - try { - mkdirSync(outDir, { recursive: true }) - writeFileSync(outPath, JSON.stringify(params.profile)) - logger.info(`CPU profile written to ${colors.cyan(outPath)}`) - logger.info(`Open it in ${colors.cyan('https://www.speedscope.app')} or Chrome DevTools`) - } - catch {} - resolve(params) }) }) + if (!params?.profile) { + return + } + + const outPath = join(outDir, `profile-${profileCount++}.cpuprofile`) + try { + mkdirSync(outDir, { recursive: true }) + writeFileSync(outPath, JSON.stringify(params.profile)) + logger.info(`CPU profile written to ${colors.cyan(outPath)}`) + logger.info(`Open it in ${colors.cyan('https://www.speedscope.app')} or Chrome DevTools`) + return outPath + } + catch {} } finally { s.disconnect() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nuxi/src/utils/profile.ts` around lines 48 - 70, The function stopCpuProfile currently resolves the internal Promise with params even when a profile file is written, causing callers to get undefined despite the function being typed Promise<string | undefined>; change the resolve path so that when params?.profile is present and the file write succeeds you resolve(outPath) (use the existing outPath local variable), keep resolving undefined (or resolve(params) if you prefer undefined) when params?.profile is absent, and ensure the outer stopCpuProfile returns the awaited value so callers receive the written path; look for the s.post('Profiler.stop') callback, the params.profile check, and the outPath/profileCount identifiers to locate where to change resolve(params) to resolve(outPath) inside the try that writes the file.packages/nuxi/src/commands/build.ts (1)
99-105:⚠️ Potential issue | 🟠 MajorDon’t exit from the
build:errorhook while profiling is active.
stopCpuProfile()is async, but this hook fire-and-forgets it and then immediately callsprocess.exit(1), so thefinallyblock never gets a chance to await the profile flush on failing builds. Let the build failure propagate to the surroundingtry/finallyinstead of shutting down here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nuxi/src/commands/build.ts` around lines 99 - 105, The build:error hook currently logs the error, calls stopCpuProfile(cwd) (which is async) and then calls process.exit(1), preventing the surrounding try/finally from awaiting profile flush; change the handler so it no longer terminates the process or attempts to synchronously stop profiling: in the nuxt.hook('build:error', (err) => { ... }) callback keep the logger.error(...) but remove the call to process.exit(1) and do not fire-and-forget stopCpuProfile(cwd) here; instead let the error propagate (rethrow the err or omit swallowing it) so the outer try/finally can await stopCpuProfile(cwd) and ensure the CPU profile is flushed correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/nuxi/src/commands/build.ts`:
- Around line 99-105: The build:error hook currently logs the error, calls
stopCpuProfile(cwd) (which is async) and then calls process.exit(1), preventing
the surrounding try/finally from awaiting profile flush; change the handler so
it no longer terminates the process or attempts to synchronously stop profiling:
in the nuxt.hook('build:error', (err) => { ... }) callback keep the
logger.error(...) but remove the call to process.exit(1) and do not
fire-and-forget stopCpuProfile(cwd) here; instead let the error propagate
(rethrow the err or omit swallowing it) so the outer try/finally can await
stopCpuProfile(cwd) and ensure the CPU profile is flushed correctly.
In `@packages/nuxi/src/utils/profile.ts`:
- Around line 48-70: The function stopCpuProfile currently resolves the internal
Promise with params even when a profile file is written, causing callers to get
undefined despite the function being typed Promise<string | undefined>; change
the resolve path so that when params?.profile is present and the file write
succeeds you resolve(outPath) (use the existing outPath local variable), keep
resolving undefined (or resolve(params) if you prefer undefined) when
params?.profile is absent, and ensure the outer stopCpuProfile returns the
awaited value so callers receive the written path; look for the
s.post('Profiler.stop') callback, the params.profile check, and the
outPath/profileCount identifiers to locate where to change resolve(params) to
resolve(outPath) inside the try that writes the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c6666f3-4aaa-46cb-8216-14b1384ab967
📒 Files selected for processing (4)
packages/nuxi/bin/nuxi.mjspackages/nuxi/src/commands/build.tspackages/nuxi/src/utils/profile.tstypes.d.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/nuxi/bin/nuxi.mjs
- types.d.ts
1 participant
🔗 Linked issue
📚 Description
paired with nuxt/nuxt#34468 - this allows enabling profiling nuxt builds with a new
--profileflag