Conversation
commit: |
📦 Bundle Size Comparison📈 nuxi
📈 nuxt-cli
➡️ create-nuxt
|
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
|
onmax
force-pushed
the
feat/doctor
branch
3 times, most recently
from
3868c71 to
308dce5
Compare
📝 WalkthroughWalkthroughAdds a new Nuxt CLI command "doctor" (packages/nuxi/src/commands/doctor.ts) that runs diagnostic checks (versions, config, modules), defines DoctorCheck and DoctorCheckContext types, and augments Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 3
🤖 Fix all issues with AI agents
In `@packages/nuxi/test/unit/commands/doctor.spec.ts`:
- Around line 295-303: Guard against undefined when reading the console spy
output: ensure consoleSpy.mock.calls[0] and its first element exist before
passing to JSON.parse. In the test around the consoleSpy usage, change the
rawOutput assignment to safely access the value (e.g., use optional chaining or
a fallback like const rawOutput = consoleSpy.mock.calls[0]?.[0] ?? '') and only
JSON.parse when rawOutput is non-empty; reference the consoleSpy variable and
the rawOutput/JSON.parse usage to locate where to apply the guard, then keep the
consoleSpy.mockRestore() call as-is.
- Around line 332-351: The test assumes consoleSpy.mock.calls[0][0] and the
found testCheck are always defined; guard against undefined to satisfy
TypeScript by checking the mock call exists (consoleSpy.mock.calls.length > 0
and consoleSpy.mock.calls[0]?.[0]) before JSON.parse and asserting testCheck is
defined (or using a non-null assertion/expect(testCheck).toBeDefined()) before
accessing its properties; update the references around the output and testCheck
variables (consoleSpy, output, testCheck) accordingly so the subsequent
expect(...) property checks are type-safe.
- Around line 270-280: The test accesses consoleSpy.mock.calls[0][0] without
guarding for undefined, causing a TypeScript error; before reading the call
value in the test (where consoleSpy is used and rawOutput is assigned), add an
assertion that a call exists (e.g. expect(consoleSpy).toHaveBeenCalled() or
expect(consoleSpy.mock.calls.length).toBeGreaterThan(0)) so
consoleSpy.mock.calls[0] is guaranteed defined, then proceed to assign rawOutput
and parse it; update references to consoleSpy and rawOutput in the same block to
rely on that assertion.
🧹 Nitpick comments (2)
packages/nuxi/src/commands/doctor.ts (1)
243-247: Silent early return may confuse users.When
nuxtVersionis undefined, the function returns without adding any check result. This leaves "Modules" absent from the output, which could be confusing. Consider adding a warning or informational check.💡 Suggested improvement
async function checkModuleCompat(checks: DoctorCheck[], nuxt: Nuxt, cwd: string): Promise<void> { const nuxtVersion = await resolveNuxtVersion(cwd) if (!nuxtVersion) { + checks.push({ + name: 'Modules', + status: 'warning', + message: 'could not determine Nuxt version for compatibility check', + }) return }packages/nuxi/test/unit/commands/doctor.spec.ts (1)
161-190: Restoreprocess.versioninafterEachto avoid test pollution.If the test fails before line 189,
process.versionremains modified for subsequent tests. UseafterEachfor cleanup to ensure restoration regardless of test outcome.♻️ Suggested fix
Add an
afterEachhook at the top level of the describe block:afterEach(() => { Object.defineProperty(process, 'version', { value: originalProcessVersion, configurable: true }) })Then remove line 189 from the test body.
|
|
||
| expect(consoleSpy).toHaveBeenCalledOnce() | ||
| const rawOutput = consoleSpy.mock.calls[0][0] | ||
| expect(() => JSON.parse(rawOutput)).not.toThrow() | ||
| const output = JSON.parse(rawOutput) | ||
| expect(output[0].status).toBe('error') | ||
| expect(output[0].message).toContain('Failed to load Nuxt') | ||
|
|
||
| consoleSpy.mockRestore() |
There was a problem hiding this comment.
Guard against undefined to fix TypeScript error.
Same issue as line 272.
🔧 Proposed fix
expect(consoleSpy).toHaveBeenCalledOnce()
- const rawOutput = consoleSpy.mock.calls[0][0]
+ const rawOutput = consoleSpy.mock.calls[0]?.[0]
+ expect(rawOutput).toBeDefined()
expect(() => JSON.parse(rawOutput)).not.toThrow()📝 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.
| expect(consoleSpy).toHaveBeenCalledOnce() | |
| const rawOutput = consoleSpy.mock.calls[0][0] | |
| expect(() => JSON.parse(rawOutput)).not.toThrow() | |
| const output = JSON.parse(rawOutput) | |
| expect(output[0].status).toBe('error') | |
| expect(output[0].message).toContain('Failed to load Nuxt') | |
| consoleSpy.mockRestore() | |
| expect(consoleSpy).toHaveBeenCalledOnce() | |
| const rawOutput = consoleSpy.mock.calls[0]?.[0] | |
| expect(rawOutput).toBeDefined() | |
| expect(() => JSON.parse(rawOutput)).not.toThrow() | |
| const output = JSON.parse(rawOutput) | |
| expect(output[0].status).toBe('error') | |
| expect(output[0].message).toContain('Failed to load Nuxt') | |
| consoleSpy.mockRestore() |
🧰 Tools
🪛 GitHub Check: ci (macos-latest)
[failure] 297-297:
Object is possibly 'undefined'.
🪛 GitHub Check: ci (ubuntu-latest)
[failure] 297-297:
Object is possibly 'undefined'.
🪛 GitHub Check: ci (windows-latest)
[failure] 297-297:
Object is possibly 'undefined'.
🤖 Prompt for AI Agents
In `@packages/nuxi/test/unit/commands/doctor.spec.ts` around lines 295 - 303,
Guard against undefined when reading the console spy output: ensure
consoleSpy.mock.calls[0] and its first element exist before passing to
JSON.parse. In the test around the consoleSpy usage, change the rawOutput
assignment to safely access the value (e.g., use optional chaining or a fallback
like const rawOutput = consoleSpy.mock.calls[0]?.[0] ?? '') and only JSON.parse
when rawOutput is non-empty; reference the consoleSpy variable and the
rawOutput/JSON.parse usage to locate where to apply the guard, then keep the
consoleSpy.mockRestore() call as-is.
37ff846 to
6c142ed
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/nuxi/test/unit/commands/doctor.spec.ts`:
- Around line 161-190: The test is mocking process.version but the code under
test checks process.versions.node; update the test "should error on Node.js <
18" to set process.versions.node to '16.20.0' (and keep it configurable) before
importing the doctor command and restore the original process.versions.node
after the test so the version check in doctor.run triggers and mockExit is
called; ensure you update the mocked property rather than process.version and
keep references to mockLoadNuxt, mockExit, and the imported doctor command
consistent.
5f08226 to
771f133
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/nuxi/src/commands/doctor.ts`:
- Around line 118-125: Wrap the nuxt.callHook('doctor:check', ...) invocation in
a try-catch so that thrown exceptions from module hooks do not abort the doctor
command; on catch, log a warning and continue. When handling addCheck in the
hook payload, validate incoming DoctorCheck objects before pushing into the
checks array: ensure required fields (e.g., id/name/message) exist and that
check.status is one of the expected enum values used by
displayResults/statusStyles; if invalid, skip the entry and emit a warning.
Ensure displayResults is only called with the validated checks array.
packages/nuxi/src/commands/doctor.ts
Outdated
| // 2. Let modules contribute via hook | ||
| await nuxt.callHook('doctor:check', { | ||
| addCheck: (c: DoctorCheck) => checks.push(c), | ||
| nuxt, | ||
| }) | ||
|
|
||
| // 3. Display results | ||
| displayResults(checks, { verbose: ctx.args.verbose, json: ctx.args.json }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/nuxi/src/commands/doctor.ts | sed -n '115,130p'Repository: nuxt/cli
Length of output: 568
🏁 Script executed:
# Find the displayResults function to understand how it handles checks
rg -A 20 "function displayResults|const displayResults|export.*displayResults" packages/nuxi/src/commands/doctor.tsRepository: nuxt/cli
Length of output: 856
🏁 Script executed:
# Search for DoctorCheck type definition
rg -B 5 -A 10 "type DoctorCheck|interface DoctorCheck" packages/nuxi/srcRepository: nuxt/cli
Length of output: 1969
🏁 Script executed:
# Check if there's any error handling around the callHook in the current code
rg -B 5 -A 10 "doctor:check" packages/nuxi/src/commands/doctor.tsRepository: nuxt/cli
Length of output: 939
🏁 Script executed:
# Search for callHook usage patterns elsewhere to understand exception behavior
rg "await.*callHook" packages/nuxi/src/commands/ -A 3 | head -40Repository: nuxt/cli
Length of output: 771
🌐 Web query:
Nuxt hookable callHook exception handling error propagation
💡 Result:
In Nuxt, the hooks system is built on @unjs/hookable. In hookable v5+, error handling changed in an important way:
- If any hook handler throws (or returns a rejected promise),
callHook(...)rejects (it no longer “swallows” the error and logs / emits a globalerrorhook like hookable < v5 did). This means errors now propagate to the caller and must be handled withawait/try-catch(or.catch(...)). [1][2]
What this means in practice
1) Errors propagate to the callHook caller
try {
await nuxtApp.callHook('my:hook', payload)
} catch (err) {
// err came from one of the hook callbacks
}If you do nuxtApp.callHook(...) without awaiting/handling the returned promise, you can end up with an unhandled promise rejection (because the promise is rejected when a hook throws). [1][2]
2) If you don’t want a hook to fail the whole hook call, catch inside the hook
nuxtApp.hook('my:hook', async () => {
try {
await doWork()
} catch (e) {
// handle/log, but don’t rethrow if you want to prevent propagation
}
})3) Nuxt-level error hooks are separate from hookable propagation
Nuxt documents framework hooks like:
app:error(startup/plugin/app init failures)vue:error(top-level Vue rendering errors)
These are for observing/reporting errors at the Nuxt/Vue layer, but they don’t change the fact that a thrown error in a hook handler can rejectcallHookand should be caught where you call it. [3]
Sources: hookable v5 breaking change on callHook rejection behavior [1][2]; Nuxt error hooks (app:error, vue:error) [3].
Guard module hook failures and validate contributed checks to prevent doctor from crashing.
In @unjs/hookable v5+, callHook() propagates exceptions from hook handlers to the caller. Without a try-catch, a thrown module hook will unhandled-reject and abort the command. Additionally, displayResults() accesses statusStyles[check.status] without validating the status value at runtime—a malformed check from a module can break output rendering.
Wrap the hook call in try-catch and validate payloads before pushing:
🔧 Suggested hardening
- await nuxt.callHook('doctor:check', {
- addCheck: (c: DoctorCheck) => checks.push(c),
- nuxt,
- })
+ const addCheck = (c: DoctorCheck) => {
+ const validStatus = c?.status === 'success' || c?.status === 'warning' || c?.status === 'error'
+ if (!c?.name || !c?.message || !validStatus) {
+ checks.push({
+ id: 'INVALID_DOCTOR_CHECK',
+ name: 'Doctor',
+ status: 'error',
+ message: 'Invalid doctor:check payload from module',
+ source: c?.source,
+ data: { received: c },
+ })
+ return
+ }
+ checks.push(c)
+ }
+
+ try {
+ await nuxt.callHook('doctor:check', { addCheck, nuxt })
+ }
+ catch (err) {
+ checks.push({
+ id: 'DOCTOR_HOOK_FAILED',
+ name: 'Doctor',
+ status: 'error',
+ message: `doctor:check hook failed: ${err instanceof Error ? err.message : String(err)}`,
+ })
+ }📝 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.
| // 2. Let modules contribute via hook | |
| await nuxt.callHook('doctor:check', { | |
| addCheck: (c: DoctorCheck) => checks.push(c), | |
| nuxt, | |
| }) | |
| // 3. Display results | |
| displayResults(checks, { verbose: ctx.args.verbose, json: ctx.args.json }) | |
| // 2. Let modules contribute via hook | |
| const addCheck = (c: DoctorCheck) => { | |
| const validStatus = c?.status === 'success' || c?.status === 'warning' || c?.status === 'error' | |
| if (!c?.name || !c?.message || !validStatus) { | |
| checks.push({ | |
| id: 'INVALID_DOCTOR_CHECK', | |
| name: 'Doctor', | |
| status: 'error', | |
| message: 'Invalid doctor:check payload from module', | |
| source: c?.source, | |
| data: { received: c }, | |
| }) | |
| return | |
| } | |
| checks.push(c) | |
| } | |
| try { | |
| await nuxt.callHook('doctor:check', { addCheck, nuxt }) | |
| } | |
| catch (err) { | |
| checks.push({ | |
| id: 'DOCTOR_HOOK_FAILED', | |
| name: 'Doctor', | |
| status: 'error', | |
| message: `doctor:check hook failed: ${err instanceof Error ? err.message : String(err)}`, | |
| }) | |
| } | |
| // 3. Display results | |
| displayResults(checks, { verbose: ctx.args.verbose, json: ctx.args.json }) |
🤖 Prompt for AI Agents
In `@packages/nuxi/src/commands/doctor.ts` around lines 118 - 125, Wrap the
nuxt.callHook('doctor:check', ...) invocation in a try-catch so that thrown
exceptions from module hooks do not abort the doctor command; on catch, log a
warning and continue. When handling addCheck in the hook payload, validate
incoming DoctorCheck objects before pushing into the checks array: ensure
required fields (e.g., id/name/message) exist and that check.status is one of
the expected enum values used by displayResults/statusStyles; if invalid, skip
the entry and emit a warning. Ensure displayResults is only called with the
validated checks array.
1 participant
Closes #1205
Hook-based diagnostic command. Core checks built-in, modules contribute via
doctor:checkhook.CLI Flags
--verbose-v--jsonModule integration
Demo
nuxt-doctor-showcase
Output (normal)
Output (--verbose)
Output (--json)
[ { "name": "Versions", "status": "success", "message": "Node v24.12.0, Nuxt 4.3.0" }, { "name": "Config", "status": "warning", "message": "1 issue found", "details": ["missing \"compatibilityDate\" - add to nuxt.config.ts"], "suggestion": "Review nuxt.config.ts and fix the issues above", "url": "https://nuxt.com/docs/getting-started/configuration" }, { "name": "Modules", "status": "success", "message": "2 modules loaded" }, { "id": "ANALYTICS_MISSING_ID", "name": "Analytics", "status": "error", "message": "missing required config", "source": "analytics", "details": ["trackingId is required but not set"], "suggestion": "Add analytics: { trackingId: \"UA-XXXXX-Y\" } to nuxt.config.ts", "url": "https://analytics.google.com/" } ]Related
doctor:checkhook types nuxt#34221 - types + docs