Conversation
02c6f66 to
2b47a11
Compare
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/nuxt/src/components/plugins/lazy-hydration-transform.ts (1)
95-105: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winNormalise the file path for
NUXT_B3005.Line 95 emits the raw transform
id, while Line 105 already aliases the same file before reportingNUXT_B3006. That makes the new diagnostics environment-dependent and exposes absolute workspace paths for one warning but not the other.Suggested fix
- const components = new Set(options.getComponents().map(c => c.pascalName)) + const components = new Set(options.getComponents().map(c => c.pascalName)) + const file = nuxt ? resolveToAlias(id, nuxt) : id await walk(ast, (node) => { @@ if (strategy) { - componentDiagnostics.NUXT_B3005({ component: node.name, file: id }) + componentDiagnostics.NUXT_B3005({ component: node.name, file }) } else { strategy = hydrationStrategyMap[prop as keyof typeof hydrationStrategyMap] } @@ if (strategy && !/^(?:Lazy|lazy-)/.test(node.name)) { if (node.name !== 'template' && (nuxt?.options.dev || nuxt?.options.test)) { - const relativePath = resolveToAlias(id, nuxt) - componentDiagnostics.NUXT_B3006({ component: node.name, file: relativePath, lazyName: `Lazy${pascalCase(node.name)}` }) + componentDiagnostics.NUXT_B3006({ component: node.name, file, lazyName: `Lazy${pascalCase(node.name)}` }) } return }🤖 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 `@packages/nuxt/src/components/plugins/lazy-hydration-transform.ts` around lines 95 - 105, Normalize the file path used by NUXT_B3005 in lazy-hydration-transform so it matches the aliased path format already used for NUXT_B3006. Update the componentDiagnostics.NUXT_B3005 call to pass resolveToAlias(id, nuxt) (or an equivalent shared normalized value) instead of the raw id, so both diagnostics report consistent, environment-independent file references.packages/nuxt/src/pages/plugins/page-meta.ts (1)
103-112: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore the throw on empty page-meta modules.
Line 107 now only reports
NUXT_B4001and then returns the fallback transform result. That turns a hard build-time failure into a log-only path, so empty?macro=truemodules can slip through with generated empty metadata instead of stopping the build.Suggested fix
if (!hasMacro && !code.includes('export { default }') && !code.includes('__nuxt_page_meta')) { if (!code) { s.append(options.dev ? (CODE_DEV_EMPTY + CODE_HMR) : CODE_EMPTY) const { pathname } = parseModuleId(id) - pageDiagnostics.NUXT_B4001({ pathname }, { method: 'error' }) + throw pageDiagnostics.NUXT_B4001({ pathname }) } else { s.overwrite(0, code.length, options.dev ? (CODE_DEV_EMPTY + CODE_HMR) : CODE_EMPTY) }🤖 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 `@packages/nuxt/src/pages/plugins/page-meta.ts` around lines 103 - 112, Restore the hard failure path in page-meta handling: in the empty-module branch inside the page transform logic, after `pageDiagnostics.NUXT_B4001` is emitted, throw again instead of returning `result()`. Update the `page-meta.ts` flow around `hasMacro`, `parseModuleId`, and `pageDiagnostics.NUXT_B4001` so empty `?macro=true` modules still stop the build rather than continuing with the fallback transform output.
🧹 Nitpick comments (1)
packages/nuxt/src/app/composables/asyncData.ts (1)
400-420: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the emitting helper in
NUXT_E3004.Line 420 now only reports the key and warning list, so the duplicate-options warning is harder to trace back than before. You already carry
_functionNamethrough dev-only async-data options, so please pass it into this diagnostic and include it in the template as well.Possible follow-up
- dataDiagnostics.NUXT_E3004({ key: key.value, warnings: warnings.map(w => `- ${w}`).join('\n') }) + dataDiagnostics.NUXT_E3004({ + key: key.value, + fn: opts._functionName || 'useAsyncData', + warnings: warnings.map(w => `- ${w}`).join('\n'), + })// packages/nuxt/src/app/diagnostics/data.ts why: (p: { key: string, fn: string, warnings: string }) => `Incompatible options detected for "${p.key}" in \`${p.fn}\`:\n${p.warnings}`,🤖 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 `@packages/nuxt/src/app/composables/asyncData.ts` around lines 400 - 420, The NUXT_E3004 diagnostic currently omits the async-data function name, making duplicate-options warnings harder to trace. In asyncData.ts, pass the dev-only _functionName through to dataDiagnostics.NUXT_E3004 alongside key and warnings, and update the NUXT_E3004 template in diagnostics/data.ts to include that fn field in the emitted message.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@packages/kit/src/diagnostics/_shared.ts`:
- Around line 11-12: The docs URL in docsBase is being exposed as a raw
interpolated template literal, which Lychee interprets as a literal URL and
trips over the placeholder. Refactor docsBase in _shared.ts to construct the
path from a separate base constant or with new URL() plus string concatenation,
so the source no longer contains the full interpolated URL verbatim while still
returning the same final docs link.
- Around line 16-21: The exported `reporters` constant in `_shared.ts` relies on
an inferred tuple type, which can interfere with declaration generation in
`tsdown` with `dts: { oxc: true }`. Add an explicit type annotation for
`reporters` so the exported shape is stable and declaration emit can succeed.
Use the existing `reporters` export and the
`createConsoleReporter`/`ansiFormatter` expression as the location to update,
without changing runtime behavior.
In `@packages/kit/src/diagnostics/components.ts`:
- Line 5: Add an explicit type annotation to the exported diagnostics object so
declaration emit under isolatedDeclarations does not rely on inference. Update
componentDiagnostics to declare its type at the export, and apply the same fix
to the other defineDiagnostics(...) exports in packages/kit/src/diagnostics/*.ts
so all exported diagnostics objects have explicit types.
In `@packages/kit/src/diagnostics/config.ts`:
- Line 5: The `configDiagnostics` export in `defineDiagnostics` is missing an
explicit exported type, which triggers declaration-generation issues under
isolated declarations. Update the `configDiagnostics` definition to use a named,
exported type annotation so TypeScript can emit declarations cleanly for
`@nuxt/kit`, and keep the symbol name `configDiagnostics` unchanged so existing
imports continue to work.
In `@packages/kit/src/diagnostics/head.ts`:
- Line 5: The exported diagnostics catalogue in headDiagnostics needs an
explicit declared type so declaration emit can succeed. Update the export at
headDiagnostics to include a concrete type annotation using the appropriate
diagnostics catalogue type from defineDiagnostics, keeping the existing
initialization logic intact. Use the headDiagnostics symbol and the
defineDiagnostics call to locate the export and add the annotation without
changing its runtime behavior.
In `@packages/kit/src/module/install.ts`:
- Around line 347-351: The module-missing check in install.ts is using a broad
includes() match, which can incorrectly treat sibling packages with the same
prefix as the requested module and raise NUXT_B8018 for the wrong failure.
Update the logic around MissingModuleMatcher and kitDiagnostics.NUXT_B8018 to
compare the extracted module against nuxtModule exactly, or only accept direct
subpath variants of the requested specifier. Keep the existing subdependency
guard, but make the requested-module detection precise so only the intended
module name triggers this diagnostic.
In `@packages/nuxt/src/app/components/nuxt-island.ts`:
- Around line 239-241: The diagnostic detail in nuxt-island’s catch block is too
narrow because it reads e.message from an any, which becomes undefined for
string or plain-object throwables. Update the error normalization in the
NUXT_E4012 path so the detail is derived from the thrown value itself (like the
previous toString() behavior) before calling renderDiagnostics.NUXT_E4012,
keeping the existing status/name/cause handling intact.
In `@packages/nuxt/src/app/composables/once.ts`:
- Around line 25-29: The callback validation in callOnce should remain
unconditional, because fn is required and allowing undefined makes
callOnce('key') fail later with a raw runtime error instead of NUXT_E7008.
Update the guard in once.ts so the NUXT_E7008 check still rejects any
non-function value, including undefined, before execution reaches the later call
site in callOnce.
In `@packages/nuxt/src/app/composables/router.ts`:
- Around line 182-186: The `navigateTo` open-path still throws a plain `Error`
before reaching the new diagnostics, so the script-protocol case is not using
the stable contract. Update the `navigateTo` logic in `router.ts` so the earlier
`options.open` guard uses the same `NUXT_E2002` diagnostic as the `toPath` URL
check, and make sure the check reports the resolved protocol consistently for
both paths.
In `@packages/nuxt/src/app/diagnostics/_shared.ts`:
- Around line 17-18: The docsBase helper is embedding an unresolved template
literal URL, which the link-checker treats as a real link. Update docsBase to
build the docs URL from a base constant or via new URL() instead of
interpolating the path directly, and keep the code.replace('NUXT_',
'').toLowerCase() logic intact while moving the URL construction into a
non-literal form.
In `@packages/nuxt/src/app/diagnostics/navigation.ts`:
- Around line 15-18: The `NUXT_E2002` diagnostic fix text is too narrow because
`isScriptProtocol(protocol)` only blocks script-like schemes, not all non-http
URLs. Update the `fix` message in the `NUXT_E2002` entry to explain that
`javascript:`-style protocols are blocked for security, while safe schemes like
`mailto:` and `tel:` remain valid, and direct users to use a safe URL/protocol
appropriate for the link target.
In `@packages/nuxt/src/app/diagnostics/render.ts`:
- Around line 66-68: Update the NUXT_E4012 guidance in the diagnostics mapping
so it matches the actual failure path in nuxt-island.ts: the parse error comes
from r.json(), not HTML parsing. Keep the why message in render.ts as-is, but
change the fix text to instruct users to verify the island endpoint returns
valid JSON/island payload, and mention it may have returned an HTML error page
instead.
In `@packages/nuxt/src/app/diagnostics/state.ts`:
- Around line 36-39: The NUXT_E7006 diagnostic currently exposes sensitive
cookie values in its why message; update the diagnostic in state.ts so the
NUXT_E7006 entry only mentions the cookie name or uses redacted placeholders
instead of p.previous and p.next. Keep the fix in the NUXT_E7006 catalog entry
and adjust the why function signature or formatting in that diagnostic so SSR
logs never contain raw cookie contents.
In `@packages/nuxt/src/components/scan.ts`:
- Around line 101-102: The `scan.ts` diagnostics for `NUXT_B3009` and
`NUXT_B3011` are still using raw filesystem paths, unlike the other component
warnings in this file. Update the `componentDiagnostics.NUXT_B3009` and
`componentDiagnostics.NUXT_B3011` calls in `scanComponents` to pass the path
through `resolveToAlias(...)` first, matching the existing usage near the other
diagnostics so `filePath` is consistent across machines.
In `@packages/nuxt/src/core/app.ts`:
- Around line 68-70: The `compileTemplate()` rejection handler in `app.ts` is
wrapping every failure in `NUXT_B1001`, which overrides the more specific
`NUXT_B1002`/`NUXT_B1003` diagnostics. Update the
`compileTemplate(...).catch(...)` logic so `NUXT_B1001` is only emitted for
unexpected exceptions, and let already-coded template failures pass through with
their original diagnostic. Apply the same change in the other `compileTemplate`
call site referenced by the comment.
In `@packages/nuxt/src/pages/runtime/plugins/router.ts`:
- Around line 250-252: The NUXT_E2004 navigation diagnostic in the router
middleware path is missing the registered named middleware context, so the dev
error no longer shows which middleware names are available. Update the call site
in the router plugin to pass the current named middleware set into
navigationDiagnostics.NUXT_E2004, or preserve the existing dev-only suffix that
includes that list, using the surrounding middleware handling logic and the
NUXT_E2004 symbol to locate the fix.
In `@packages/vite/src/plugins/optimize-deps-hint.ts`:
- Around line 126-128: The stale-deps hint branch in optimize-deps-hint should
preserve the stale dependency names instead of emitting an empty NUXT_B7002
payload. Update the hasStale path in the logic around formatStaleDepsHint so it
still passes the userStale/moduleStale information (or equivalent stale dep
names) into bundlerDiagnostics.NUXT_B7002, keeping the warning actionable when
no new deps are discovered.
---
Outside diff comments:
In `@packages/nuxt/src/components/plugins/lazy-hydration-transform.ts`:
- Around line 95-105: Normalize the file path used by NUXT_B3005 in
lazy-hydration-transform so it matches the aliased path format already used for
NUXT_B3006. Update the componentDiagnostics.NUXT_B3005 call to pass
resolveToAlias(id, nuxt) (or an equivalent shared normalized value) instead of
the raw id, so both diagnostics report consistent, environment-independent file
references.
In `@packages/nuxt/src/pages/plugins/page-meta.ts`:
- Around line 103-112: Restore the hard failure path in page-meta handling: in
the empty-module branch inside the page transform logic, after
`pageDiagnostics.NUXT_B4001` is emitted, throw again instead of returning
`result()`. Update the `page-meta.ts` flow around `hasMacro`, `parseModuleId`,
and `pageDiagnostics.NUXT_B4001` so empty `?macro=true` modules still stop the
build rather than continuing with the fallback transform output.
---
Nitpick comments:
In `@packages/nuxt/src/app/composables/asyncData.ts`:
- Around line 400-420: The NUXT_E3004 diagnostic currently omits the async-data
function name, making duplicate-options warnings harder to trace. In
asyncData.ts, pass the dev-only _functionName through to
dataDiagnostics.NUXT_E3004 alongside key and warnings, and update the NUXT_E3004
template in diagnostics/data.ts to include that fn field in the emitted message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 60ca06a3-6ab8-4270-9805-477a5ff74a17
⛔ Files ignored due to path filters (3)
packages/kit/package.jsonis excluded by!**/package.jsonpackages/nuxt/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (122)
docs/errors/.navigation.ymldocs/errors/b5001.mddocs/errors/b5003.mddocs/errors/b5004.mddocs/errors/e1001.mddocs/errors/e1006.mddocs/errors/e1007.mddocs/errors/e2001.mddocs/errors/e2002.mddocs/errors/e2003.mddocs/errors/e2004.mddocs/errors/e2005.mddocs/errors/e2007.mddocs/errors/e6001.mdeslint.config.mjspackages/kit/src/compatibility.tspackages/kit/src/components.tspackages/kit/src/context.tspackages/kit/src/dependency.tspackages/kit/src/diagnostics/_shared.tspackages/kit/src/diagnostics/build.tspackages/kit/src/diagnostics/bundler.tspackages/kit/src/diagnostics/components.tspackages/kit/src/diagnostics/config.tspackages/kit/src/diagnostics/head.tspackages/kit/src/diagnostics/kit-api.tspackages/kit/src/diagnostics/pages.tspackages/kit/src/diagnostics/plugins.tspackages/kit/src/index.tspackages/kit/src/layout.tspackages/kit/src/loader/nuxt.tspackages/kit/src/module/define.tspackages/kit/src/module/install.tspackages/kit/src/nitro.tspackages/kit/src/pages.tspackages/kit/src/plugin.tspackages/kit/src/resolve.tspackages/kit/src/template.tspackages/kit/test/plugin.test.tspackages/nitro-server/src/index.tspackages/nuxt/src/app/compat/interval.tspackages/nuxt/src/app/components/client-fallback.server.tspackages/nuxt/src/app/components/island-renderer.tspackages/nuxt/src/app/components/nuxt-island.tspackages/nuxt/src/app/components/nuxt-layout.tspackages/nuxt/src/app/components/nuxt-link.tspackages/nuxt/src/app/components/route-provider.tspackages/nuxt/src/app/components/test-component-wrapper.tspackages/nuxt/src/app/components/utils.tspackages/nuxt/src/app/composables/asyncData.tspackages/nuxt/src/app/composables/component.tspackages/nuxt/src/app/composables/cookie.tspackages/nuxt/src/app/composables/fetch.tspackages/nuxt/src/app/composables/manifest.tspackages/nuxt/src/app/composables/once.tspackages/nuxt/src/app/composables/payload.tspackages/nuxt/src/app/composables/preload.tspackages/nuxt/src/app/composables/router.tspackages/nuxt/src/app/composables/ssr.tspackages/nuxt/src/app/composables/state.tspackages/nuxt/src/app/diagnostics/_shared.tspackages/nuxt/src/app/diagnostics/core.tspackages/nuxt/src/app/diagnostics/data.tspackages/nuxt/src/app/diagnostics/head.tspackages/nuxt/src/app/diagnostics/manifest.tspackages/nuxt/src/app/diagnostics/navigation.tspackages/nuxt/src/app/diagnostics/render.tspackages/nuxt/src/app/diagnostics/state.tspackages/nuxt/src/app/entry.tspackages/nuxt/src/app/nuxt.tspackages/nuxt/src/app/plugins/check-if-layout-used.tspackages/nuxt/src/app/plugins/payload.client.tspackages/nuxt/src/app/plugins/router.tspackages/nuxt/src/compiler/module.tspackages/nuxt/src/compiler/plugins/keyed-function-factories.tspackages/nuxt/src/compiler/plugins/keyed-functions.tspackages/nuxt/src/compiler/runtime/index.tspackages/nuxt/src/components/module.tspackages/nuxt/src/components/plugins/islands-transform.tspackages/nuxt/src/components/plugins/lazy-hydration-transform.tspackages/nuxt/src/components/plugins/loader.tspackages/nuxt/src/components/plugins/transform.tspackages/nuxt/src/components/scan.tspackages/nuxt/src/core/app.tspackages/nuxt/src/core/builder.tspackages/nuxt/src/core/cache.tspackages/nuxt/src/core/external-config-files.tspackages/nuxt/src/core/features.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/src/core/plugins/plugin-metadata.tspackages/nuxt/src/core/schema.tspackages/nuxt/src/core/server.tspackages/nuxt/src/head/plugins/unhead-imports.tspackages/nuxt/src/head/runtime/components.tspackages/nuxt/src/head/runtime/composables.tspackages/nuxt/src/imports/module.tspackages/nuxt/src/pages/module.tspackages/nuxt/src/pages/plugins/page-meta.tspackages/nuxt/src/pages/runtime/composables.tspackages/nuxt/src/pages/runtime/page-placeholder.tspackages/nuxt/src/pages/runtime/plugins/check-if-page-unused.tspackages/nuxt/src/pages/runtime/plugins/router.tspackages/nuxt/src/pages/utils.tspackages/nuxt/test/compiler.test.tspackages/nuxt/test/diagnostics-catalog.test.tspackages/nuxt/test/island-transform.test.tspackages/nuxt/test/keyed-functions.test.tspackages/nuxt/test/page-metadata.test.tspackages/nuxt/test/plugin-metadata.test.tspackages/vite/src/css.tspackages/vite/src/plugins/analyze.tspackages/vite/src/plugins/decorators.tspackages/vite/src/plugins/optimize-deps-hint.tspackages/vite/src/plugins/vite-node.tspackages/vite/src/plugins/vue-jsx.tspackages/vite/src/utils/config.tspackages/vite/test/optimize-deps-hint.test.tspackages/webpack/src/plugins/vue/server.tspackages/webpack/src/plugins/vue/util.tspackages/webpack/src/utils/config.tspackages/webpack/src/utils/postcss.tspackages/webpack/src/webpack.ts
| if (LAZY_COMPONENT_NAME_REGEX.test(pascalName)) { | ||
| logger.warn(`The component \`${pascalName}\` (in \`${filePath}\`) is using the reserved "Lazy" prefix used for dynamic imports, which may cause it to break at runtime.`) | ||
| componentDiagnostics.NUXT_B3009({ component: pascalName, filePath }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Resolve component paths before emitting NUXT_B3009 and NUXT_B3011.
Line 102 and Line 174 still pass raw filesystem paths, but Line 50 and Line 142 in the same file already use resolveToAlias(...). Keeping these two diagnostics raw makes the new output inconsistent across machines and less useful for users copying paths back into config.
Suggested fix
if (LAZY_COMPONENT_NAME_REGEX.test(pascalName)) {
- componentDiagnostics.NUXT_B3009({ component: pascalName, filePath })
+ componentDiagnostics.NUXT_B3009({ component: pascalName, filePath: resolveToAlias(filePath) })
}
@@
function warnAboutDuplicateComponent (componentName: string, filePath: string, duplicatePath: string) {
- componentDiagnostics.NUXT_B3011({ component: componentName, filePath, duplicatePath })
+ componentDiagnostics.NUXT_B3011({
+ component: componentName,
+ filePath: resolveToAlias(filePath),
+ duplicatePath: resolveToAlias(duplicatePath),
+ })
}Also applies to: 173-174
🤖 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 `@packages/nuxt/src/components/scan.ts` around lines 101 - 102, The `scan.ts`
diagnostics for `NUXT_B3009` and `NUXT_B3011` are still using raw filesystem
paths, unlike the other component warnings in this file. Update the
`componentDiagnostics.NUXT_B3009` and `componentDiagnostics.NUXT_B3011` calls in
`scanComponents` to pass the path through `resolveToAlias(...)` first, matching
the existing usage near the other diagnostics so `filePath` is consistent across
machines.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@packages/nuxt/src/app/composables/asyncData.ts`:
- Around line 359-362: The validation guards in asyncData should preserve the
existing TypeError contract instead of switching to Error-based diagnostics.
Update the checks in useAsyncData so the _key and _handler failures still throw
TypeError while keeping the same diagnostic messages, and make sure the existing
NUXT_E3008 and NUXT_E3009 paths continue to satisfy instanceof TypeError
expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c95362d7-11d5-4ae3-9a39-fc90e946b597
📒 Files selected for processing (6)
packages/nuxt/src/app/composables/asyncData.tspackages/nuxt/src/app/composables/ssr.tspackages/nuxt/src/app/diagnostics/core.tspackages/nuxt/src/app/diagnostics/data.tspackages/nuxt/src/app/diagnostics/manifest.tspackages/nuxt/src/app/diagnostics/state.ts
💤 Files with no reviewable changes (2)
- packages/nuxt/src/app/diagnostics/state.ts
- packages/nuxt/src/app/diagnostics/manifest.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nuxt/src/app/diagnostics/core.ts
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
Merging this PR will regress 1 benchmark
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | initial production build in the minimal test fixture |
1.8 s | 2 s | -13.88% |
| ⚡ | loadNuxtConfig in the basic test fixture (types) |
22 ms | 16.9 ms | +30.26% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing feat/nostics-errors (0456be1) with main (60ce78b)
Footnotes
-
3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
|
||
| const serverStats = await analyzeSizes(['**/*.mjs', '!_libs'], serverDir, rootDir) | ||
| expect.soft(roundToKilobytes(serverStats.totalBytes)).toMatchInlineSnapshot(`"69.0k"`) | ||
| expect.soft(roundToKilobytes(serverStats.totalBytes)).toMatchInlineSnapshot(`"80.5k"`) |
There was a problem hiding this comment.
this seems too big of a bump 🤔
There was a problem hiding this comment.
I thought it didn’t change 😮
Was this introduced in my PR or is it during a rebase?
There was a problem hiding this comment.
no, this was introduced in this PR. I backported the test: false change into main, to confirm.
There was a problem hiding this comment.
I Will have to check. I light have forgotten some defineProdDiagnostics
There was a problem hiding this comment.
I checked and it's weird it goes up so much. Even when using the prod diagnostics, it goes to 75k. If this is important, I can enforce the defineProdDiagnostics() which removes all of the error messages and reporters (no logs) to have only the error code appear (useful for thrown errors)
There was a problem hiding this comment.
Getting a bit further with this, especially for the client side bundle. Some increases are intentional, some are because we include more information in the errors like here
I think we can refactor the method: 'error' to apply to most of these
WDYT @danielroe ?
There was a problem hiding this comment.
why are jsdocs appearing in the prod bundle? 🤔 this must be a bug on our end, right?
There was a problem hiding this comment.
because the server bundle is not minified. It feels intentional but can probably be configured
|
summarising my recent pushes:
let me know if I've misunderstood any of that or we should reconsider ... and thank you so much for all your work on this - it's looking amazing! 🚀 |
|
This sounds great! I'm glad you took a look. I'm going to give it another look |
f07374b to
54b933e
Compare
| @@ -0,0 +1,18 @@ | ||
| --- |
There was a problem hiding this comment.
In the end, are we agreeing on always having a page? I think adding it to contributing or to an agents.md would ease the maintainance
There was a problem hiding this comment.
I added more pages just because (IIUC) prod errors always link to a page even if you pass docs: false (because this is tree-shaken out)....
I'm happy to ship this PR without all pages, but I think it makes sense eventually to have pages for every one.
There was a problem hiding this comment.
Yes, sorry I missed that you already said it in the review! This PR is huge so I'm losing myself in it sometimes.
I think you took the better direction
| }) | ||
|
|
||
| expect(() => factory('a', 1)).toThrowErrorMatchingInlineSnapshot(`[Error: [nuxt] \`createUseFetch\` is a compiler macro and cannot be called at runtime.]`) | ||
| expect(() => factory('a', 1)).toThrowErrorMatchingInlineSnapshot(`[NUXT_E1007: NUXT_E1007]`) |
There was a problem hiding this comment.
maybe worth flagging:
- do we want
[Error: NUXT_E1007]? (that would be a nostics change) - should we just add a simple console.error log in prod and remove the manual
if (!importe.meta.dev) console.error(diangostics.NUXT_E1007().name)?
There was a problem hiding this comment.
preserving the source of truth in diagnostics seems valuable, rather than inlining a console.error
[NUXT_E1007: NUXT_E1007] seems strange - I think just [NUXT_E1007] would be better
There was a problem hiding this comment.
I agree on the strangeness of the [...: ...]
Then, I can add a custom reporter for this in prod only
There was a problem hiding this comment.
added in other places but here, it's the production throw, so no real control over how it's displayed
There was a problem hiding this comment.
why does nostics show it like this?
Add the nostics dependency and a build-time diagnostics catalog (packages/kit/src/diagnostics.ts). Migrate the first call site (addPlugin src validation) to throw diagnostics.NUXT_B2011 and add the b2011 error reference doc plus the errors/ navigation stub.
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Roe <daniel@roe.dev> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

🔗 Linked issue
Closes #34719
📚 Description
Initial migration of errors to nostics with stable code errors
TODO:
why+fix