Feat/early hints app head#34635
Closed
jozrftamson wants to merge 23 commits into
Closed
Conversation
- use generateTypeDeclarations for shared auto-import d.ts output - add optimizeDeps include diff (+/-) in runtime hint output - harden import protection for alias-based Vue app imports in nitro/shared contexts Refs: nuxt#34578 nuxt#34624 nuxt#34576
|
|
Contributor
WalkthroughThis pull request introduces comprehensive enhancements across Nitro server, Nuxt framework, and Vite plugin tooling. It adds new utilities for Nitro to handle import exclusion patterns, asset URL globals, early HTTP/2 hints generation, and missing source map error detection. Nuxt changes include router type augmentations, transpile path normalisation for build directories, enhanced import protection with app-root boundary checking, workspace-aware transform plugin configuration, and hot-update module deduplication. Vite improvements add worker asset resolution in the dev server, optimised dependency inclusion diff formatting, stable entry chunk hash seed generation, and improved error formatting. Supporting test suites and fixture updates validate the new functionality across all packages. Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (8)
packages/nuxt/test/load-nuxt.test.ts (1)
176-190: Always close the Nuxt instance even when assertions fail.If the assertion on Line 187 throws, cleanup on Line 189 is skipped. Wrap the assertion in
try/finallyto avoid cross-test leakage.As per coding guidelines "Use error handling patterns consistently".Proposed fix
const nuxt = await loadNuxt({ cwd: repoRoot, ready: true, overrides: { buildDir, build: { transpile: [templatePath], }, }, }) - expect(nuxt.options.build.transpile).toContain(normalize(templatePath)) - - await nuxt.close() + try { + expect(nuxt.options.build.transpile).toContain(normalize(templatePath)) + } finally { + await nuxt.close() + } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nuxt/test/load-nuxt.test.ts` around lines 176 - 190, The test creates a Nuxt instance via loadNuxt and currently calls await nuxt.close() after an assertion, which will be skipped if the expect throws; modify the test so the code that references nuxt (the expect on nuxt.options.build.transpile) is executed inside a try block and always call await nuxt.close() in a finally block to ensure cleanup; ensure nuxt is declared in a scope visible to both try and finally (e.g., const nuxt = await loadNuxt(...) before try) and preserve the existing assertion and build overrides.packages/nuxt/test/cache.test.ts (1)
147-170: Consider file placement and missingbuildCacheoption.This test validates build artifact generation rather than cache behaviour, yet it resides in
cache.test.ts. Additionally, unlike the other tests in this file, it omitsexperimental: { buildCache: true }. If the intent is to test that these artifacts are generated correctly when caching is enabled, consider addingbuildCache: truefor consistency; otherwise, this test may be better suited to a separate test file (e.g.,build-artifacts.test.ts).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nuxt/test/cache.test.ts` around lines 147 - 170, The test "should generate route-rules and app-manifest build artifacts" in cache.test.ts is misplaced and missing the experimental buildCache flag; either move this test to a more appropriate file (e.g., create build-artifacts.test.ts) or update the test to enable caching by adding experimental: { buildCache: true } to the loadNuxt overrides; locate the test by its description and the loadNuxt call in the file and apply the change to ensure consistency with other tests that assert cache-related behavior.packages/vite/test/dev-server.test.ts (1)
16-52: Good coverage of core scenarios.The tests validate the main use cases: standard packages, scoped packages, and non-worker rejection. Consider adding tests for:
- Cache behaviour (pass a cache Map and verify hits)
- URLs with query parameters (e.g.,
/assets/worker-abc.js?v=123)- Multiple or non-existent module roots
These are optional improvements and not blockers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite/test/dev-server.test.ts` around lines 16 - 52, Tests for resolveNodeModuleWorkerAssetPath currently cover standard, scoped, and non-worker cases but miss edge cases; add unit tests verifying cache behavior (pass a cache Map to resolveNodeModuleWorkerAssetPath and assert a cache hit/entry is used), URLs with query parameters (call with '/assets/worker-abc.js?v=123' and expect it still resolves to the same worker file), and multiple or non-existent module roots (provide an array of roots including valid and invalid paths and assert resolution still finds the correct worker or returns null when none exist). Ensure tests reference the same temp dir setup pattern used in existing cases and reuse the function resolveNodeModuleWorkerAssetPath to locate the code under test.packages/vite/src/plugins/optimize-deps-hint.ts (1)
156-167: Keep CJS annotation behaviour consistent in stale-only hints.Line 160 and Line 167 currently omit
cjsDeps, so stale-only output can lose(CJS)labelling that is present in the full hint path.♻️ Proposed consistency patch
} else if (hasStale) { hasShownStaleHint = true + const cjsDeps = getCjsDeps() const snippetDeps = getSnippetDeps() const parts: string[] = [] parts.push(formatStaleDepsHint([...userStale], [...moduleStale])) - const includeDiff = formatIncludeDiff(getUserInclude(), snippetDeps) + const includeDiff = formatIncludeDiff(getUserInclude(), snippetDeps, cjsDeps) if (includeDiff) { parts.push(includeDiff) } parts.push( `Update your \`nuxt.config.ts\`:\n\n` + - configBlock(snippetDeps), + configBlock(snippetDeps, cjsDeps), ) logger.warn(parts.join('\n\n')) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite/src/plugins/optimize-deps-hint.ts` around lines 156 - 167, The stale-only hint path is dropping CJS labels because `cjsDeps` isn't included when building messages; update the calls that build the stale-only output so they include CJS info: when calling `formatStaleDepsHint` pass the `cjsDeps` (or the combined snippet deps that include CJS metadata) alongside `userStale`/`moduleStale`, and ensure the `snippetDeps` passed into `configBlock` (and into `formatIncludeDiff` if relevant) preserves CJS annotation; update `getSnippetDeps` usage or explicitly merge `cjsDeps` into `snippetDeps` before calling `formatStaleDepsHint`, `formatIncludeDiff`, and `configBlock` so the `(CJS)` labels remain present.packages/vite/src/plugins/stable-entry.ts (1)
18-25: Prefer unambiguous serialisation over delimiter joining for hash seed.Line 23 to Line 25 can theoretically collide when values include
|(different inputs producing the same seed string). Using a canonical serialisation avoids that class of collision.Suggested tweak
export function getStableEntryChunkHashSeed (chunk: ChunkLike) { const dynamicImports = chunk.dynamicImports || [] const importedCss = chunk.viteMetadata?.importedCss ? [...chunk.viteMetadata.importedCss] : [] const importedAssets = chunk.viteMetadata?.importedAssets ? [...chunk.viteMetadata.importedAssets] : [] - return [...dynamicImports, ...importedCss, ...importedAssets] - .sort() - .join('|') + return JSON.stringify( + [...dynamicImports, ...importedCss, ...importedAssets].sort(), + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite/src/plugins/stable-entry.ts` around lines 18 - 25, The seed construction in getStableEntryChunkHashSeed concatenates dynamicImports, importedCss, and importedAssets with a '|' delimiter which can collide if items contain '|'; instead produce an unambiguous canonical serialization of the combined array (e.g., serialize as JSON array or use length-prefixed encoding) after sorting to guarantee uniqueness and determinism; update the function that builds the seed from dynamicImports/importedCss/importedAssets to serialize the array unambiguously rather than using .join('|').packages/vite/test/stable-entry.test.ts (1)
6-33: Add a small edge-case test for missing metadata paths.Line 6 to Line 33 only covers populated metadata; adding
{}/ missingviteMetadatacases would guard the optional-branch behaviour ingetStableEntryChunkHashSeed.Suggested additional test
describe('getStableEntryChunkHashSeed', () => { it('is stable for order changes and sensitive to dependency changes', () => { @@ expect(first).toBe(reordered) expect(changed).not.toBe(first) }) + + it('handles missing optional metadata consistently', () => { + expect(getStableEntryChunkHashSeed({})).toBe('') + expect(getStableEntryChunkHashSeed({ dynamicImports: ['chunk-a.js'] })).toBe('chunk-a.js') + }) })As per coding guidelines, "Write unit tests for core functionality using
vitest."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite/test/stable-entry.test.ts` around lines 6 - 33, Add a test case to cover the edge case where viteMetadata is absent or empty when calling getStableEntryChunkHashSeed: invoke getStableEntryChunkHashSeed with an input that omits or passes an empty viteMetadata (e.g., {}) and assert it returns a stable value and does not throw; also add a second call with same missing metadata to verify idempotence (equal results) and a call with populated viteMetadata to ensure difference when dependencies change. Ensure you add these assertions in the same test suite that contains the existing tests for getStableEntryChunkHashSeed so the optional-branch behavior is exercised.packages/nitro-server/src/runtime/handlers/error.ts (1)
10-16: Consider using a more flexible error type.The
ErrorLiketype may be too restrictive compared to Nitro'sHTTPError<unknown>. Consider making it compatible with the actual error types passed to the handler, possibly by using a broader type or extracting from Nitro's types.Alternative approach
-type ErrorLike = { - code?: string - message?: string - cause?: { - code?: string - } -} +// Use Record to allow any additional properties from HTTPError +type ErrorLike = { + code?: string + message?: string + cause?: { + code?: string + } + [key: string]: unknown +}Alternatively, consider importing or referencing Nitro's actual error type if available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nitro-server/src/runtime/handlers/error.ts` around lines 10 - 16, The ErrorLike type is too narrow for errors passed into the handler; replace or extend it so the handler accepts Nitro's actual error shape (e.g., use HTTPError<unknown> or a union with Error) by importing Nitro's HTTPError type if available, or change the local ErrorLike to a broader type (e.g., unknown | Error | { code?: string; message?: string; cause?: any }) and update any handler signatures that reference ErrorLike (search for ErrorLike usage in this file, e.g., the error handler function) so the function accepts the wider type and safely narrows it where properties like code or message are accessed.PROGRESS.md (1)
36-70: Prefer repo-relative commands in progress logs.Several entries use machine-specific absolute paths (for example around Line 36, Line 41, Line 45, Line 50, Line 55, Line 60, Line 65, Line 70). Switching to repo-relative paths keeps the log reproducible and avoids leaking local workstation details.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@PROGRESS.md` around lines 36 - 70, The progress log in PROGRESS.md contains machine-specific absolute paths in the test command entries (e.g., the repeated "pnpm vitest run /home/josef/Projekte/Github/nuxt/..." command strings); update those entries to use repo-relative commands (like "pnpm vitest run packages/nuxt/..." or omit the absolute root and rely on --root . or relative paths) so logs no longer leak local workstation paths, ensuring each mention of the full absolute test invocation (the pnpm vitest run lines) is replaced with a repo-relative equivalent or a short canonical form.
🤖 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/nitro-server/src/index.ts`:
- Line 9: The module uses normalize() from pathe but it isn't imported; update
the existing import statement that currently lists dirname, join, relative,
resolve to also include normalize so calls to normalize() in this file (e.g.,
where paths are normalized before joining/resolving) compile successfully;
locate the import line that contains dirname, join, relative, resolve and add
normalize to that named import list.
- Around line 467-470: The current guard that checks for existing definitions of
defineAppConfig (using /\b(import|const|let|var|function)\s+defineAppConfig\b/)
misses named import forms like "import { defineAppConfig } from '...'", so the
shim gets prepended; update the guard in the block that inspects the code string
(variable code) to also detect import statements that include defineAppConfig
inside braces or any import that mentions defineAppConfig (e.g. "import {
defineAppConfig } ..." or "import defineAppConfig ...")—adjust the regex to
match import ... defineAppConfig as well as the existing const/let/var/function
patterns so the shim (the prepend of 'const defineAppConfig = (config) =>
config\n') is only added when no binding or import of defineAppConfig exists.
In `@packages/nitro-server/src/runtime/handlers/error.ts`:
- Around line 18-26: The function isMissingSourceMapRequestError needs an
explicit boolean return type, must guard against undefined path/url, and must
handle ErrorLike vs HTTPError<unknown> cause typing; change the signature to
export function isMissingSourceMapRequestError(error: ErrorLike, event:
Pick<H3Event, 'path' | 'req'>): boolean, compute requestPath with event.path ??
event.req?.url (e.g. const requestPath = String(event.path ?? event.req?.url ??
'').split('?')[0]) to avoid undefined, and resolve the cause typing by either
widening ErrorLike to allow cause?: unknown | { code?: string } or locally
reading the code with a safe cast (e.g. const errorCode = ((error.cause as
any)?.code) || (error as any).code) so the check uses a string-compatible value.
In `@packages/nitro-server/src/runtime/utils/renderer/asset-url-globals.ts`:
- Around line 3-8: Add an explicit return type to the setAssetURLGlobals
function to satisfy --isolatedDeclarations; update the function signature for
setAssetURLGlobals to include ": void" so TypeScript knows it returns nothing
while leaving the body (assignments to globalThis.__buildAssetsURL and
globalThis.__publicAssetsURL) unchanged.
In `@packages/nuxt/src/core/nuxt.ts`:
- Around line 771-777: The buildDir check incorrectly uses
startsWith(normalizedBuildDir) which will match sibling paths sharing the same
prefix; update the guard around normalizedPath to ensure the match is a true
path boundary by checking equality or that normalizedPath starts with
normalizedBuildDir + path.sep (taking into account trailing separators) or by
using a path-relative test (e.g., path.relative(normalizedBuildDir,
normalizedPath) that does not start with '..'); modify the logic around the
symbols normalizedBuildDir and normalizedPath so only paths inside the buildDir
are treated as internal entries.
In `@packages/nuxt/src/core/plugins/import-protection.ts`:
- Around line 112-113: The fallback regex used to detect upward relative imports
(the expression testing normalizedRelative) is missing the "utils" directory and
so doesn't match imports into utils even though appRoots includes it; update the
regex in the return line that tests normalizedRelative to include "utils"
alongside app, pages, layouts, middleware, plugins, components, composables so
relative paths like ../.../utils/... are caught by the same check.
In `@packages/nuxt/src/pages/plugins/page-meta.ts`:
- Around line 47-49: The type mismatch comes from HotUpdateModuleLike declaring
"id?: string" while Vite's ModuleNode uses "id: string | null"; update
HotUpdateModuleLike's id to allow null (e.g., make id optionally string or null)
so the interface accepts ModuleNode instances; change the id declaration in
HotUpdateModuleLike (the interface named HotUpdateModuleLike) to include null
(resulting in string | null | undefined) so TypeScript no longer reports
incompatibility when passing ModuleNode objects.
In `@packages/vite/src/plugins/dev-server.ts`:
- Around line 41-48: The TypeScript error comes from pathname possibly being
undefined; ensure pathname is always a string before using RegExp.test and
Map.has by coalescing null/undefined values: when computing pathname from url in
dev-server.ts (the variable named pathname produced from
String(url).split(/[?#]/)[0]), replace the expression with a nullish-safe
version (e.g., use url ?? '' and coalesce the split result to an empty string)
so pathname is typed as string, then the subsequent calls to the RegExp.test on
pathname and cache.has(pathname) / cache.get(pathname) will type-check.
In `@packages/vite/src/vite-node-runner.ts`:
- Line 48: The function formatViteError currently lacks an explicit return type
which breaks --isolatedDeclarations; update the formatViteError function
signature to include an explicit return type that exactly matches the shape of
the object it returns (include all properties it returns such as id and any
optional fields like message, stack, frame, loc, etc.), so TypeScript can
generate isolated declaration files without inference errors.
---
Nitpick comments:
In `@packages/nitro-server/src/runtime/handlers/error.ts`:
- Around line 10-16: The ErrorLike type is too narrow for errors passed into the
handler; replace or extend it so the handler accepts Nitro's actual error shape
(e.g., use HTTPError<unknown> or a union with Error) by importing Nitro's
HTTPError type if available, or change the local ErrorLike to a broader type
(e.g., unknown | Error | { code?: string; message?: string; cause?: any }) and
update any handler signatures that reference ErrorLike (search for ErrorLike
usage in this file, e.g., the error handler function) so the function accepts
the wider type and safely narrows it where properties like code or message are
accessed.
In `@packages/nuxt/test/cache.test.ts`:
- Around line 147-170: The test "should generate route-rules and app-manifest
build artifacts" in cache.test.ts is misplaced and missing the experimental
buildCache flag; either move this test to a more appropriate file (e.g., create
build-artifacts.test.ts) or update the test to enable caching by adding
experimental: { buildCache: true } to the loadNuxt overrides; locate the test by
its description and the loadNuxt call in the file and apply the change to ensure
consistency with other tests that assert cache-related behavior.
In `@packages/nuxt/test/load-nuxt.test.ts`:
- Around line 176-190: The test creates a Nuxt instance via loadNuxt and
currently calls await nuxt.close() after an assertion, which will be skipped if
the expect throws; modify the test so the code that references nuxt (the expect
on nuxt.options.build.transpile) is executed inside a try block and always call
await nuxt.close() in a finally block to ensure cleanup; ensure nuxt is declared
in a scope visible to both try and finally (e.g., const nuxt = await
loadNuxt(...) before try) and preserve the existing assertion and build
overrides.
In `@packages/vite/src/plugins/optimize-deps-hint.ts`:
- Around line 156-167: The stale-only hint path is dropping CJS labels because
`cjsDeps` isn't included when building messages; update the calls that build the
stale-only output so they include CJS info: when calling `formatStaleDepsHint`
pass the `cjsDeps` (or the combined snippet deps that include CJS metadata)
alongside `userStale`/`moduleStale`, and ensure the `snippetDeps` passed into
`configBlock` (and into `formatIncludeDiff` if relevant) preserves CJS
annotation; update `getSnippetDeps` usage or explicitly merge `cjsDeps` into
`snippetDeps` before calling `formatStaleDepsHint`, `formatIncludeDiff`, and
`configBlock` so the `(CJS)` labels remain present.
In `@packages/vite/src/plugins/stable-entry.ts`:
- Around line 18-25: The seed construction in getStableEntryChunkHashSeed
concatenates dynamicImports, importedCss, and importedAssets with a '|'
delimiter which can collide if items contain '|'; instead produce an unambiguous
canonical serialization of the combined array (e.g., serialize as JSON array or
use length-prefixed encoding) after sorting to guarantee uniqueness and
determinism; update the function that builds the seed from
dynamicImports/importedCss/importedAssets to serialize the array unambiguously
rather than using .join('|').
In `@packages/vite/test/dev-server.test.ts`:
- Around line 16-52: Tests for resolveNodeModuleWorkerAssetPath currently cover
standard, scoped, and non-worker cases but miss edge cases; add unit tests
verifying cache behavior (pass a cache Map to resolveNodeModuleWorkerAssetPath
and assert a cache hit/entry is used), URLs with query parameters (call with
'/assets/worker-abc.js?v=123' and expect it still resolves to the same worker
file), and multiple or non-existent module roots (provide an array of roots
including valid and invalid paths and assert resolution still finds the correct
worker or returns null when none exist). Ensure tests reference the same temp
dir setup pattern used in existing cases and reuse the function
resolveNodeModuleWorkerAssetPath to locate the code under test.
In `@packages/vite/test/stable-entry.test.ts`:
- Around line 6-33: Add a test case to cover the edge case where viteMetadata is
absent or empty when calling getStableEntryChunkHashSeed: invoke
getStableEntryChunkHashSeed with an input that omits or passes an empty
viteMetadata (e.g., {}) and assert it returns a stable value and does not throw;
also add a second call with same missing metadata to verify idempotence (equal
results) and a call with populated viteMetadata to ensure difference when
dependencies change. Ensure you add these assertions in the same test suite that
contains the existing tests for getStableEntryChunkHashSeed so the
optional-branch behavior is exercised.
In `@PROGRESS.md`:
- Around line 36-70: The progress log in PROGRESS.md contains machine-specific
absolute paths in the test command entries (e.g., the repeated "pnpm vitest run
/home/josef/Projekte/Github/nuxt/..." command strings); update those entries to
use repo-relative commands (like "pnpm vitest run packages/nuxt/..." or omit the
absolute root and rely on --root . or relative paths) so logs no longer leak
local workstation paths, ensuring each mention of the full absolute test
invocation (the pnpm vitest run lines) is replaced with a repo-relative
equivalent or a short canonical form.
🪄 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: 1a78a645-8301-4de4-86cc-03b772e15718
📒 Files selected for processing (39)
PROGRESS.mdpackages/nitro-server/src/imports-exclude.tspackages/nitro-server/src/index.tspackages/nitro-server/src/runtime/handlers/error.tspackages/nitro-server/src/runtime/handlers/island.tspackages/nitro-server/src/runtime/handlers/renderer.tspackages/nitro-server/src/runtime/utils/renderer/asset-url-globals.tspackages/nitro-server/src/runtime/utils/renderer/early-hints.tspackages/nitro-server/test/asset-url-globals.test.tspackages/nitro-server/test/early-hints.test.tspackages/nitro-server/test/error-handler.test.tspackages/nitro-server/test/imports-exclude.test.tspackages/nuxt/src/app/types/augments.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/src/core/plugins/import-protection.tspackages/nuxt/src/head/runtime/plugins/unhead.tspackages/nuxt/src/imports/module.tspackages/nuxt/src/imports/transform.tspackages/nuxt/src/pages/plugins/page-meta.tspackages/nuxt/test/auto-imports.test.tspackages/nuxt/test/cache.test.tspackages/nuxt/test/import-protection.test.tspackages/nuxt/test/load-nuxt.test.tspackages/nuxt/test/page-metadata.test.tspackages/vite/src/plugins/dev-server.tspackages/vite/src/plugins/optimize-deps-hint.tspackages/vite/src/plugins/stable-entry.tspackages/vite/src/shared/client.tspackages/vite/src/vite-node-runner.tspackages/vite/test/dev-server.test.tspackages/vite/test/optimize-deps-hint.test.tspackages/vite/test/shared-client.test.tspackages/vite/test/stable-entry.test.tspackages/vite/test/vite-node-runner.test.tstest/basic.test.tstest/bundle.test.tstest/fixtures/basic-types/app/components/routing-instance-types.vuetest/fixtures/basic/app/error.vuetest/fixtures/basic/nuxt.config.ts
| import { randomUUID } from 'node:crypto' | ||
| import type { Nuxt, NuxtOptions } from '@nuxt/schema' | ||
| import { join, relative, resolve } from 'pathe' | ||
| import { dirname, join, relative, resolve } from 'pathe' |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find packages -name "index.ts" | grep nitro-serverRepository: nuxt/nuxt
Length of output: 87
🏁 Script executed:
head -20 packages/nitro-server/src/index.tsRepository: nuxt/nuxt
Length of output: 1277
🏁 Script executed:
sed -n '450,475p' packages/nitro-server/src/index.tsRepository: nuxt/nuxt
Length of output: 960
🏁 Script executed:
sed -n '1,15p' packages/nitro-server/src/index.tsRepository: nuxt/nuxt
Length of output: 1050
Import normalize from pathe before using it.
Lines 456 and 463 use normalize(), which is not imported. This causes a compile error. Add normalize to the import from pathe:
Suggested fix
-import { dirname, join, relative, resolve } from 'pathe'
+import { dirname, join, normalize, relative, resolve } from 'pathe'📝 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
| import { dirname, join, relative, resolve } from 'pathe' | |
| import { dirname, join, normalize, relative, resolve } from 'pathe' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nitro-server/src/index.ts` at line 9, The module uses normalize()
from pathe but it isn't imported; update the existing import statement that
currently lists dirname, join, relative, resolve to also include normalize so
calls to normalize() in this file (e.g., where paths are normalized before
joining/resolving) compile successfully; locate the import line that contains
dirname, join, relative, resolve and add normalize to that named import list.
Comment on lines
+467
to
+470
| if (/\b(import|const|let|var|function)\s+defineAppConfig\b/.test(code)) { | ||
| return | ||
| } | ||
| return 'const defineAppConfig = (config) => config\n' + code |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the relevant lines
cat -n packages/nitro-server/src/index.ts | sed -n '460,480p'Repository: nuxt/nuxt
Length of output: 959
🏁 Script executed:
#!/bin/bash
node <<'EOF'
// Test the original regex pattern
const originalRegex = /\b(import|const|let|var|function)\s+defineAppConfig\b/;
// Test cases
const testCases = [
{ name: "named import", code: "import { defineAppConfig } from 'app'", expected: true, shouldMatch: true },
{ name: "default import", code: "import defineAppConfig from 'app'", expected: true, shouldMatch: true },
{ name: "const declaration", code: "const defineAppConfig = () => {}", expected: true, shouldMatch: true },
{ name: "let declaration", code: "let defineAppConfig = () => {}", expected: true, shouldMatch: true },
{ name: "var declaration", code: "var defineAppConfig = () => {}", expected: true, shouldMatch: true },
{ name: "function declaration", code: "function defineAppConfig(config) {}", expected: true, shouldMatch: true },
{ name: "no definition", code: "console.log(defineAppConfig)", expected: false, shouldMatch: false },
];
console.log("Testing ORIGINAL regex pattern:");
console.log("Pattern: " + originalRegex);
console.log("---");
testCases.forEach(testCase => {
const matches = originalRegex.test(testCase.code);
const status = matches === testCase.expected ? "✓" : "✗";
console.log(`${status} ${testCase.name}: matches=${matches}, expected=${testCase.expected}`);
console.log(` Code: "${testCase.code}"`);
});
// Test the suggested regex
console.log("\n\nTesting SUGGESTED regex pattern (named import detection):");
const namedImportRegex = /\bimport\s*{[^}]*\bdefineAppConfig\b[^}]*}\s*from\b/;
console.log("Pattern: " + namedImportRegex);
console.log("---");
const namedImportTests = [
{ name: "named import", code: "import { defineAppConfig } from 'app'", shouldMatch: true },
{ name: "named import with alias", code: "import { defineAppConfig as dac } from 'app'", shouldMatch: true },
{ name: "named import with others", code: "import { foo, defineAppConfig, bar } from 'app'", shouldMatch: true },
{ name: "default import", code: "import defineAppConfig from 'app'", shouldMatch: false },
{ name: "const declaration", code: "const defineAppConfig = () => {}", shouldMatch: false },
];
namedImportTests.forEach(testCase => {
const matches = namedImportRegex.test(testCase.code);
const status = matches === testCase.shouldMatch ? "✓" : "✗";
console.log(`${status} ${testCase.name}: matches=${matches}, shouldMatch=${testCase.shouldMatch}`);
console.log(` Code: "${testCase.code}"`);
});
// Test combined pattern
console.log("\n\nTesting COMBINED pattern (original + suggested):");
const combinedRegex = /\b(import|const|let|var|function)\s+defineAppConfig\b/ || /\bimport\s*{[^}]*\bdefineAppConfig\b[^}]*}\s*from\b/;
// Note: This is wrong. Should use |
const correctedCombined = /\b(import|const|let|var|function)\s+defineAppConfig\b|\bimport\s*{[^}]*\bdefineAppConfig\b[^}]*}\s*from\b/;
console.log("Pattern: " + correctedCombined);
console.log("---");
const allTests = [
{ name: "named import", code: "import { defineAppConfig } from 'app'", shouldMatch: true },
{ name: "default import", code: "import defineAppConfig from 'app'", shouldMatch: true },
{ name: "const declaration", code: "const defineAppConfig = () => {}", shouldMatch: true },
{ name: "no definition", code: "console.log(defineAppConfig)", shouldMatch: false },
];
allTests.forEach(testCase => {
const matches = correctedCombined.test(testCase.code);
const status = matches === testCase.shouldMatch ? "✓" : "✗";
console.log(`${status} ${testCase.name}: matches=${matches}, shouldMatch=${testCase.shouldMatch}`);
console.log(` Code: "${testCase.code}"`);
});
EOFRepository: nuxt/nuxt
Length of output: 1992
Handle named imports before prepending the shim.
The current guard doesn't match named imports like import { defineAppConfig } from ..., so the shim is incorrectly prepended, creating a duplicate binding in otherwise valid app-config files.
Suggested fix
- if (/\b(import|const|let|var|function)\s+defineAppConfig\b/.test(code)) {
+ if (
+ /\b(import|const|let|var|function)\s+defineAppConfig\b/.test(code) ||
+ /\bimport\s*{[^}]*\bdefineAppConfig\b[^}]*}\s*from\b/.test(code)
+ ) {
return
}📝 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
| if (/\b(import|const|let|var|function)\s+defineAppConfig\b/.test(code)) { | |
| return | |
| } | |
| return 'const defineAppConfig = (config) => config\n' + code | |
| if ( | |
| /\b(import|const|let|var|function)\s+defineAppConfig\b/.test(code) || | |
| /\bimport\s*{[^}]*\bdefineAppConfig\b[^}]*}\s*from\b/.test(code) | |
| ) { | |
| return | |
| } | |
| return 'const defineAppConfig = (config) => config\n' + code |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nitro-server/src/index.ts` around lines 467 - 470, The current guard
that checks for existing definitions of defineAppConfig (using
/\b(import|const|let|var|function)\s+defineAppConfig\b/) misses named import
forms like "import { defineAppConfig } from '...'", so the shim gets prepended;
update the guard in the block that inspects the code string (variable code) to
also detect import statements that include defineAppConfig inside braces or any
import that mentions defineAppConfig (e.g. "import { defineAppConfig } ..." or
"import defineAppConfig ...")—adjust the regex to match import ...
defineAppConfig as well as the existing const/let/var/function patterns so the
shim (the prepend of 'const defineAppConfig = (config) => config\n') is only
added when no binding or import of defineAppConfig exists.
Comment on lines
+18
to
+26
| export function isMissingSourceMapRequestError (error: ErrorLike, event: Pick<H3Event, 'path' | 'req'>) { | ||
| const requestPath = String(event.path || event.req.url || '').split('?')[0] | ||
| if (!requestPath.endsWith('.map')) { | ||
| return false | ||
| } | ||
|
|
||
| const errorCode = error.cause?.code || error.code | ||
| return errorCode === 'ENOENT' || error.message?.includes('ENOENT: no such file or directory') || false | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what error type is passed to error handlers in Nitro
rg -n "NitroErrorHandler" --type ts -A 5 | head -50
# Check HTTPError type definition
ast-grep --pattern 'type HTTPError<$_> = { $$$ }'Repository: nuxt/nuxt
Length of output: 1542
🏁 Script executed:
cat -n packages/nitro-server/src/runtime/handlers/error.tsRepository: nuxt/nuxt
Length of output: 6355
🏁 Script executed:
# Search for NitroErrorHandler type definition
rg "type NitroErrorHandler" --type ts -A 5
# Also check if there are any type errors when compiling this file
rg -A 20 "export default.*NitroErrorHandler" packages/nitro-server/src/runtime/handlers/error.ts | head -30Repository: nuxt/nuxt
Length of output: 938
🏁 Script executed:
# Look for how the error parameter is actually typed/used elsewhere in the codebase
rg "error\.status\|error\.headers\|error\.data" packages/nitro-server/src/runtime/handlers/error.ts -B 2 -A 2
# Check if there's a NitroErrorHandler definition in the repo or if we need to check external types
find . -name "*.d.ts" -o -name "types.ts" | xargs grep -l "NitroErrorHandler" 2>/dev/null | head -5
# Check node_modules or see what's available
fd "nitro" --type d --max-depth 2 | head -10Repository: nuxt/nuxt
Length of output: 75
🏁 Script executed:
# Check tsconfig to see if --isolatedDeclarations is enabled
cat tsconfig.json 2>/dev/null | grep -A 10 "isolatedDeclarations"
# Search for how error is used in this file and what properties are accessed
rg "error\." packages/nitro-server/src/runtime/handlers/error.ts | head -20
# Look for any build output or error logs that might show actual type errors
find . -name "tsconfig*.json" -path "*/nitro-server/*" | head -5Repository: nuxt/nuxt
Length of output: 634
🏁 Script executed:
# Find the tsconfig for nitro-server
find packages/nitro-server -name "tsconfig*.json" -exec cat {} \;
# Also check the root tsconfig
cat tsconfig.json | grep -A 5 -B 5 "isolatedDeclarations\|packages/nitro-server"Repository: nuxt/nuxt
Length of output: 35
🏁 Script executed:
# Check if there's a build error log or CI configuration that shows these errors
ls -la .github/workflows/ 2>/dev/null | head -20
# Look at the actual structure of what's imported from nitro
rg "from 'nitro" packages/nitro-server/src/runtime/handlers/error.ts
# Check if there's any documentation about NitroErrorHandler
rg -i "nitroerrorhandler" --type md . | head -10Repository: nuxt/nuxt
Length of output: 1437
🏁 Script executed:
# Check the CI workflow for TypeScript build steps
head -100 .github/workflows/ci.yml | grep -A 20 "typescript\|build\|type"
# Look for any nitro type definitions or stubs
find . -name "*.d.ts" | xargs grep -l "interface.*Error\|type.*Error" 2>/dev/null | head -10
# Check if there's a package.json with type checking scripts
cat packages/nitro-server/package.json 2>/dev/null | grep -A 5 "scripts"Repository: nuxt/nuxt
Length of output: 988
🏁 Script executed:
# Check the root package.json for typecheck command
cat package.json | grep -A 20 '"typecheck"'
# Look at the actual error types more carefully - check how error is used elsewhere
rg "error\.(status|headers|data|code|message|cause)" packages/nitro-server/src/runtime/handlers/error.ts -B 1 -A 1
# Look for any type guards or type narrowing
rg "is.*Error\|as.*Error" packages/nitro-server/src/runtime/handlers/error.tsRepository: nuxt/nuxt
Length of output: 1707
🏁 Script executed:
# Check the full tsconfig or vue-tsc configuration
find . -name "vue-tsc.config.*" -o -name "tsconfig*.json" | xargs cat 2>/dev/null | grep -A 30 "isolatedDeclarations\|noEmit\|strict"
# Look for any actual type errors reported in build logs or check if the nitro types are available
rg "interface.*Error\|export.*type.*Error" node_modules/nitro/dist 2>/dev/null | head -20
# Check what the actual nitro types export
ls -la node_modules/nitro/dist/ 2>/dev/null | grep -i "type\|\.d\.ts"Repository: nuxt/nuxt
Length of output: 954
🏁 Script executed:
# Check if strict mode is actually enabled for this package
cat packages/nitro-server/tsconfig.json 2>/dev/null || cat tsconfig.json | grep -A 50 "compilerOptions"
# Look at the error types more carefully - what is passed to isMissingSourceMapRequestError
# The issue is: error from errorhandler has properties like .status, .headers, .data
# But ErrorLike type doesn't include those. This means either:
# 1. ErrorLike is incomplete, or
# 2. The cast is unsafe
# Let's see what properties are actually required by looking at usage
rg "error\." packages/nitro-server/src/runtime/handlers/error.ts | cut -d: -f2 | sort -uRepository: nuxt/nuxt
Length of output: 1731
🏁 Script executed:
# Check the actual build output to see if there are real type errors
pnpm typecheck 2>&1 | grep -A 5 "error.ts" | head -30
# Also check if there's a build script that would catch this
pnpm build 2>&1 | grep -A 5 "error.ts" | head -30Repository: nuxt/nuxt
Length of output: 6555
Add explicit return type and resolve type incompatibility issues.
Multiple type errors prevent compilation:
- TS9007 (line 18): Function needs explicit return type annotation with
--isolatedDeclarations. - TS18048 (line 20):
requestPathis possibly undefined. - TS2345 (line 29):
HTTPError<unknown>is not assignable toErrorLike. Thecauseproperty inHTTPErroris typed asunknown, butErrorLikeexpects{ code?: string } | undefined.
Adjust the ErrorLike type to be compatible with HTTPError's cause property, or use a more permissive type that accepts unknown for the cause field.
Proposed fixes
+type ErrorLike = {
+ code?: string
+ message?: string
+ cause?: unknown
+}
+
-export function isMissingSourceMapRequestError (error: ErrorLike, event: Pick<H3Event, 'path' | 'req'>) {
- const requestPath = String(event.path || event.req.url || '').split('?')[0]
+export function isMissingSourceMapRequestError (error: ErrorLike, event: Pick<H3Event, 'path' | 'req'>): boolean {
+ const requestPath = (String(event.path || event.req.url || '') || '').split('?')[0]🧰 Tools
🪛 GitHub Actions: autofix.ci
[error] 18-18: PARSE_ERROR: TS9007: Function must have an explicit return type annotation with --isolatedDeclarations.
🪛 GitHub Check: build
[failure] 20-20:
'requestPath' is possibly 'undefined'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nitro-server/src/runtime/handlers/error.ts` around lines 18 - 26,
The function isMissingSourceMapRequestError needs an explicit boolean return
type, must guard against undefined path/url, and must handle ErrorLike vs
HTTPError<unknown> cause typing; change the signature to export function
isMissingSourceMapRequestError(error: ErrorLike, event: Pick<H3Event, 'path' |
'req'>): boolean, compute requestPath with event.path ?? event.req?.url (e.g.
const requestPath = String(event.path ?? event.req?.url ?? '').split('?')[0]) to
avoid undefined, and resolve the cause typing by either widening ErrorLike to
allow cause?: unknown | { code?: string } or locally reading the code with a
safe cast (e.g. const errorCode = ((error.cause as any)?.code) || (error as
any).code) so the check uses a string-compatible value.
Comment on lines
+3
to
+8
| export function setAssetURLGlobals (buildAssetsURL: AssetURLFn, publicAssetsURL: AssetURLFn) { | ||
| // @ts-expect-error private property consumed by vite-generated url helpers | ||
| globalThis.__buildAssetsURL = buildAssetsURL | ||
| // @ts-expect-error private property consumed by vite-generated url helpers | ||
| globalThis.__publicAssetsURL = publicAssetsURL | ||
| } |
Contributor
There was a problem hiding this comment.
Add explicit return type annotation to fix build failure.
The pipeline indicates TS9007: Function must have an explicit return type annotation with --isolatedDeclarations. Add : void to the function signature.
Proposed fix
-export function setAssetURLGlobals (buildAssetsURL: AssetURLFn, publicAssetsURL: AssetURLFn) {
+export function setAssetURLGlobals (buildAssetsURL: AssetURLFn, publicAssetsURL: AssetURLFn): void {
// `@ts-expect-error` private property consumed by vite-generated url helpers
globalThis.__buildAssetsURL = buildAssetsURL
// `@ts-expect-error` private property consumed by vite-generated url helpers
globalThis.__publicAssetsURL = publicAssetsURL
}📝 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
| export function setAssetURLGlobals (buildAssetsURL: AssetURLFn, publicAssetsURL: AssetURLFn) { | |
| // @ts-expect-error private property consumed by vite-generated url helpers | |
| globalThis.__buildAssetsURL = buildAssetsURL | |
| // @ts-expect-error private property consumed by vite-generated url helpers | |
| globalThis.__publicAssetsURL = publicAssetsURL | |
| } | |
| export function setAssetURLGlobals (buildAssetsURL: AssetURLFn, publicAssetsURL: AssetURLFn): void { | |
| // `@ts-expect-error` private property consumed by vite-generated url helpers | |
| globalThis.__buildAssetsURL = buildAssetsURL | |
| // `@ts-expect-error` private property consumed by vite-generated url helpers | |
| globalThis.__publicAssetsURL = publicAssetsURL | |
| } |
🧰 Tools
🪛 GitHub Actions: autofix.ci
[error] 3-3: PARSE_ERROR: TS9007: Function must have an explicit return type annotation with --isolatedDeclarations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nitro-server/src/runtime/utils/renderer/asset-url-globals.ts` around
lines 3 - 8, Add an explicit return type to the setAssetURLGlobals function to
satisfy --isolatedDeclarations; update the function signature for
setAssetURLGlobals to include ": void" so TypeScript knows it returns nothing
while leaving the body (assignments to globalThis.__buildAssetsURL and
globalThis.__publicAssetsURL) unchanged.
Comment on lines
+771
to
+777
| const normalizedBuildDir = normalize(nuxt.options.buildDir) | ||
|
|
||
| // Keep internal buildDir entries absolute, even when buildDir lives in | ||
| // node_modules/.cache, so Nitro can still match and transpile them. | ||
| if (normalizedPath.startsWith(normalizedBuildDir)) { | ||
| return normalizedPath | ||
| } |
Contributor
There was a problem hiding this comment.
Guard buildDir matching with a path boundary.
Line 775 currently uses a plain startsWith, so sibling paths sharing the same prefix as buildDir can be treated as internal build entries by mistake.
Suggested fix
- const normalizedBuildDir = normalize(nuxt.options.buildDir)
+ const normalizedBuildDir = normalize(nuxt.options.buildDir)
+ const normalizedBuildDirPrefix = withTrailingSlash(normalizedBuildDir)
@@
- if (normalizedPath.startsWith(normalizedBuildDir)) {
+ if (normalizedPath === normalizedBuildDir || normalizedPath.startsWith(normalizedBuildDirPrefix)) {
return normalizedPath
}📝 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 normalizedBuildDir = normalize(nuxt.options.buildDir) | |
| // Keep internal buildDir entries absolute, even when buildDir lives in | |
| // node_modules/.cache, so Nitro can still match and transpile them. | |
| if (normalizedPath.startsWith(normalizedBuildDir)) { | |
| return normalizedPath | |
| } | |
| const normalizedBuildDir = normalize(nuxt.options.buildDir) | |
| const normalizedBuildDirPrefix = withTrailingSlash(normalizedBuildDir) | |
| // Keep internal buildDir entries absolute, even when buildDir lives in | |
| // node_modules/.cache, so Nitro can still match and transpile them. | |
| if (normalizedPath === normalizedBuildDir || normalizedPath.startsWith(normalizedBuildDirPrefix)) { | |
| return normalizedPath | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nuxt/src/core/nuxt.ts` around lines 771 - 777, The buildDir check
incorrectly uses startsWith(normalizedBuildDir) which will match sibling paths
sharing the same prefix; update the guard around normalizedPath to ensure the
match is a true path boundary by checking equality or that normalizedPath starts
with normalizedBuildDir + path.sep (taking into account trailing separators) or
by using a path-relative test (e.g., path.relative(normalizedBuildDir,
normalizedPath) that does not start with '..'); modify the logic around the
symbols normalizedBuildDir and normalizedPath so only paths inside the buildDir
are treated as internal entries.
Comment on lines
+112
to
+113
| const normalizedRelative = normalize(id) | ||
| return /(^|\/)\.\.\/(?:.*\/)?(app|pages|layouts|middleware|plugins|components|composables)(\/|$)/.test(normalizedRelative) |
Contributor
There was a problem hiding this comment.
Fallback regex is missing utils directory.
The appRoots array on line 104 includes utils, but the fallback regex pattern on line 113 only matches (app|pages|layouts|middleware|plugins|components|composables). This creates an inconsistency where imports traversing into utils/ via relative paths won't be caught when the full path cannot be resolved.
🐛 Proposed fix to add 'utils' to the regex
- return /(^|\/)\.\.\/(?:.*\/)?(app|pages|layouts|middleware|plugins|components|composables)(\/|$)/.test(normalizedRelative)
+ return /(^|\/)\.\.\/(?:.*\/)?(app|pages|layouts|middleware|plugins|components|composables|utils)(\/|$)/.test(normalizedRelative)📝 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 normalizedRelative = normalize(id) | |
| return /(^|\/)\.\.\/(?:.*\/)?(app|pages|layouts|middleware|plugins|components|composables)(\/|$)/.test(normalizedRelative) | |
| const normalizedRelative = normalize(id) | |
| return /(^|\/)\.\.\/(?:.*\/)?(app|pages|layouts|middleware|plugins|components|composables|utils)(\/|$)/.test(normalizedRelative) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nuxt/src/core/plugins/import-protection.ts` around lines 112 - 113,
The fallback regex used to detect upward relative imports (the expression
testing normalizedRelative) is missing the "utils" directory and so doesn't
match imports into utils even though appRoots includes it; update the regex in
the return line that tests normalizedRelative to include "utils" alongside app,
pages, layouts, middleware, plugins, components, composables so relative paths
like ../.../utils/... are caught by the same check.
Comment on lines
+47
to
+49
| interface HotUpdateModuleLike { | ||
| id?: string | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Vite ModuleNode id type signature
💡 Result:
The id property of Vite's ModuleNode class is typed as string | null in TypeScript. This represents the resolved module identifier (absolute path, potentially with hash or query parameters). It starts as null until the module is resolved via the resolveId hook or file system lookup during module graph construction. Key evidence from Vite source analysis: - ModuleNode constructor initializes id: string | null = null - Maps like idToModuleMap: Map<string, ModuleNode> store resolved modules by their id - APIs like getModuleById(id: string) expect a non-null string for lookups - During resolution, mod.id = resolvedId sets the string value Official Vite JavaScript API docs reference ModuleNode in server.moduleGraph and reloadModule(module: ModuleNode), confirming its role in the dev server module graph without contradicting the nullable type.
Citations:
- 1: https://yanceyleo.com/post/a852197f-eacd-4741-8e1c-bdfc4610cf62
- 2: https://v3.vite.dev/guide/api-javascript
- 3: https://vitejs.dev/guide/api-javascript.html
- 4: feat: separate module graphs per environment vitejs/vite#16003
- 5: fix(vite)!: refactor "module cache" to "evaluated modules", pass down module to "runInlinedModule" vitejs/vite#18092
🏁 Script executed:
# Check if the file exists and get its basic info
ls -la packages/nuxt/src/pages/plugins/page-meta.tsRepository: nuxt/nuxt
Length of output: 147
🏁 Script executed:
# Read the specific lines around the interface definition (lines 45-65)
sed -n '45,65p' packages/nuxt/src/pages/plugins/page-meta.tsRepository: nuxt/nuxt
Length of output: 532
🏁 Script executed:
# Read the handleHotUpdate usage (lines 370-380)
sed -n '370,380p' packages/nuxt/src/pages/plugins/page-meta.tsRepository: nuxt/nuxt
Length of output: 546
🏁 Script executed:
# Check imports and see if ModuleNode type is actually used/imported in this file
grep -n "ModuleNode\|vite\|handleHotUpdate" packages/nuxt/src/pages/plugins/page-meta.ts | head -20Repository: nuxt/nuxt
Length of output: 207
🏁 Script executed:
# Get more context around the handleHotUpdate to understand the full signature
sed -n '360,385p' packages/nuxt/src/pages/plugins/page-meta.tsRepository: nuxt/nuxt
Length of output: 865
Fix type incompatibility with Vite's ModuleNode.
The interface HotUpdateModuleLike defines id as string | undefined (via id?: string), but the function is passed ModuleNode instances where id is typed as string | null. These types are incompatible in TypeScript.
🐛 Proposed fix
interface HotUpdateModuleLike {
- id?: string
+ id?: string | 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
| interface HotUpdateModuleLike { | |
| id?: string | |
| } | |
| interface HotUpdateModuleLike { | |
| id?: string | null | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nuxt/src/pages/plugins/page-meta.ts` around lines 47 - 49, The type
mismatch comes from HotUpdateModuleLike declaring "id?: string" while Vite's
ModuleNode uses "id: string | null"; update HotUpdateModuleLike's id to allow
null (e.g., make id optionally string or null) so the interface accepts
ModuleNode instances; change the id declaration in HotUpdateModuleLike (the
interface named HotUpdateModuleLike) to include null (resulting in string | null
| undefined) so TypeScript no longer reports incompatibility when passing
ModuleNode objects.
Comment on lines
+41
to
+48
| const pathname = String(url).split(/[?#]/)[0] | ||
| if (!/^\/?assets\/.*worker.*\.js$/i.test(pathname)) { | ||
| return null | ||
| } | ||
|
|
||
| if (cache?.has(pathname)) { | ||
| return cache.get(pathname) || null | ||
| } |
Contributor
There was a problem hiding this comment.
Fix TypeScript error: pathname may be string | undefined.
The static analysis correctly flags that String(url).split(/[?#]/)[0] returns string | undefined in strict TypeScript mode, causing type errors on lines 42 and 46 where pathname is used with RegExp.test() and Map.has().
🐛 Proposed fix using nullish coalescing
- const pathname = String(url).split(/[?#]/)[0]
+ const pathname = String(url).split(/[?#]/)[0] ?? ''
if (!/^\/?assets\/.*worker.*\.js$/i.test(pathname)) {
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 pathname = String(url).split(/[?#]/)[0] | |
| if (!/^\/?assets\/.*worker.*\.js$/i.test(pathname)) { | |
| return null | |
| } | |
| if (cache?.has(pathname)) { | |
| return cache.get(pathname) || null | |
| } | |
| const pathname = String(url).split(/[?#]/)[0] ?? '' | |
| if (!/^\/?assets\/.*worker.*\.js$/i.test(pathname)) { | |
| return null | |
| } | |
| if (cache?.has(pathname)) { | |
| return cache.get(pathname) || null | |
| } |
🧰 Tools
🪛 GitHub Check: build
[failure] 46-46:
Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
[failure] 42-42:
Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vite/src/plugins/dev-server.ts` around lines 41 - 48, The TypeScript
error comes from pathname possibly being undefined; ensure pathname is always a
string before using RegExp.test and Map.has by coalescing null/undefined values:
when computing pathname from url in dev-server.ts (the variable named pathname
produced from String(url).split(/[?#]/)[0]), replace the expression with a
nullish-safe version (e.g., use url ?? '' and coalesce the split result to an
empty string) so pathname is typed as string, then the subsequent calls to the
RegExp.test on pathname and cache.has(pathname) / cache.get(pathname) will
type-check.
| } | ||
|
|
||
| function formatViteError (errorData: any, id: string) { | ||
| export function formatViteError (errorData: any, id: string) { |
Contributor
There was a problem hiding this comment.
Add return type annotation to fix --isolatedDeclarations error.
The pipeline failure indicates that shorthand properties in the return object cannot be inferred with --isolatedDeclarations. Add an explicit return type to resolve this.
🐛 Proposed fix
-export function formatViteError (errorData: any, id: string) {
+export function formatViteError (errorData: any, id: string): { message: string, stack: string } {📝 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
| export function formatViteError (errorData: any, id: string) { | |
| export function formatViteError (errorData: any, id: string): { message: string, stack: string } { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vite/src/vite-node-runner.ts` at line 48, The function
formatViteError currently lacks an explicit return type which breaks
--isolatedDeclarations; update the formatViteError function signature to include
an explicit return type that exactly matches the shape of the object it returns
(include all properties it returns such as id and any optional fields like
message, stack, frame, loc, etc.), so TypeScript can generate isolated
declaration files without inference errors.
Member
|
I think this is the same PR that has already been closed. |
Member
|
please see #34565 (comment) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
📚 Description