feat(nuxt,vite): add experimental support for rolldown-vite
#31812
Conversation
|
|
WalkthroughThe changes make Vite configuration conditional on the presence of rolldownVersion. In server.ts, Rollup output.generatedCode is only set when not using Rolldown, and deletion of manualChunks is similarly gated; under Rolldown, advancedChunks is deleted instead. In vite.ts, esbuild-related config (viteConfig.esbuild and viteConfig.optimizeDeps.esbuildOptions) is removed when Rolldown is used, and build.watch switches from a chokidar-based setup to a simple exclude list. TypeScript non-public property checks are suppressed with // @ts-expect-error comments. No public APIs or exports are modified. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)
124-127: Consider using undefined assignment instead of delete operatorThe static analysis tool correctly flagged a potential performance issue with the
deleteoperator. While functionally correct, this approach may impact performance.- if(!vite.rolldownVersion) { - // @ts-ignore Rolldown has no support for `output.manualChunks` - delete serverConfig.build.rollupOptions.output.manualChunks - } + if(!vite.rolldownVersion) { + // Rolldown has no support for `output.manualChunks` + serverConfig.build.rollupOptions.output.manualChunks = undefined + }🧰 Tools
🪛 Biome (1.9.4)
[error] 126-126: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
package.jsonis excluded by!package.json,!**/package.jsonpackages/kit/package.jsonis excluded by!**/package.jsonpackages/nuxt/package.jsonis excluded by!**/package.jsonpackages/schema/package.jsonis excluded by!**/package.jsonpackages/ui-templates/package.jsonis excluded by!**/package.jsonpackages/vite/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (2)
packages/vite/src/server.ts(2 hunks)pnpm-workspace.yaml(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/vite/src/server.ts
[error] 126-126: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: codeql (javascript-typescript)
- GitHub Check: code
🔇 Additional comments (2)
pnpm-workspace.yaml (1)
13-14: New configuration for rolldown dependency looks goodThis addition of
onlyBuiltDependencieswithrolldownas its sole entry aligns with the PR's purpose of testingrolldown-viteintegration. It instructs pnpm to exclusively considerrolldownfor building, which is appropriate for targeted testing.packages/vite/src/server.ts (1)
93-100: Conditional application of Rollup options for non-Rolldown buildsThe conditional logic to apply Rollup-specific options only when not using Rolldown is well-implemented. This correctly handles the compatibility differences between Rollup and Rolldown.
|
@TheAlexLichter do you think we could ship under an experimental flag? users would still need to manually set a resolution for |
That seems new 👀🤔
Tests are failing at the moment (at least locally). We should be able to ship it with an experimental flag, yes! |
|
I got an issue with |
@huang-julien |
|
related issue
@BobbieGoede |
|
How about using the nuxt.config.ts: $development: {
vite: {
experimental: {
enableNativePlugin: 'resolver',
},
},
},
$production: {
vite: {
experimental: {
enableNativePlugin: true,
},
},
} |
I would leave that up the user to enable/disable (but we can document it/recommend it). Depending on the modules used it might or might not build. Native plugins are still WIP in terms of behavior parity with their JS equivalent |
@danielroe I wonder if we could even ship it unflagged as we have rolldown detection in place everywhere |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/kit/src/components.ts (1)
50-76: Potential accumulation of identical hooks
addComponents()registers a newcomponents:extendhook every time it is called.
If an extension repeatedly callsaddComponent/addComponentExportsin a loop, dozens of identical hooks will be attached, slightly degrading performance and making stack‑traces noisy.Consider caching the list of
addedComponentsper‑Nuxt‑instance (e.g. with aWeakMap) and registering only a single hook that merges all queued components.packages/kit/test/components.spec.ts (1)
25-38: Great test coverage – one tiny robustness improvementThe test thoroughly exercises the new helper.
You might additionally assert the component count to make the intention explicit and guard against regressions where extra exports are accidentally picked up:- expect(components).toMatchInlineSnapshot(` + expect(components).toHaveLength(2) + expect(components).toMatchInlineSnapshot(`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
package.jsonis excluded by!package.json,!**/package.jsonpackages/kit/package.jsonis excluded by!**/package.jsonpackages/nuxt/package.jsonis excluded by!**/package.jsonpackages/schema/package.jsonis excluded by!**/package.jsonpackages/ui-templates/package.jsonis excluded by!**/package.jsonpackages/vite/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (4)
packages/kit/src/components.ts(3 hunks)packages/kit/src/index.ts(1 hunks)packages/kit/test/components-fixture/Named.ts(1 hunks)packages/kit/test/components.spec.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/kit/test/components-fixture/Named.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/kit/src/components.ts (3)
packages/kit/src/index.ts (6)
addComponentExports(19-19)AddComponentOptions(20-20)useNuxt(21-21)resolvePath(28-28)addComponent(19-19)logger(32-32)test/fixtures/basic/nuxt.config.ts (1)
components(237-243)packages/schema/src/index.ts (1)
Component(3-3)
🔇 Additional comments (1)
packages/kit/src/index.ts (1)
19-20: Public re‑export looks good
addComponentExportsis now surfaced alongside the existing helpers, keeping the public API coherent.
| export function addComponentExports (opts: Omit<AddComponentOptions, 'name'> & { prefix?: string }) { | ||
| const nuxt = useNuxt() | ||
| const components: Component[] = [] | ||
| nuxt.hook('components:dirs', async () => { | ||
| const filePath = await resolvePath(opts.filePath) | ||
|
|
||
| const names = await resolveModuleExportNames(filePath, { extensions: nuxt.options.extensions }) | ||
| components.length = 0 | ||
| for (const name of names) { | ||
| components.push(normalizeComponent({ name: pascalCase([opts.prefix || '', name === 'default' ? '' : name]), export: name, ...opts })) | ||
| } | ||
| }) | ||
| addComponents(components) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid leaking the prefix prop – move it into meta or drop it before normalisation
addComponentExports() forwards the whole opts object (which still contains the custom prefix field) to normalizeComponent().
Since AddComponentOptions does not declare prefix, TypeScript’s excess‑property check will complain unless Component happens to include an index signature. Even if it compiles, persisting ad‑hoc keys on the component root pollutes the public shape and could conflict with future schema additions.
A minimal fix is to strip prefix before calling normalizeComponent, and—if you want to preserve the information—move it into meta.prefix.
- components.push(normalizeComponent({ name: pascalCase([opts.prefix || '', name === 'default' ? '' : name]), export: name, ...opts }))
+ const { prefix, ...rest } = opts
+ const comp = normalizeComponent({
+ name: pascalCase([prefix || '', name === 'default' ? '' : name]),
+ export: name,
+ ...rest,
+ meta: { ...(rest as any).meta, prefix }
+ })
+ components.push(comp)📝 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.
| export function addComponentExports (opts: Omit<AddComponentOptions, 'name'> & { prefix?: string }) { | |
| const nuxt = useNuxt() | |
| const components: Component[] = [] | |
| nuxt.hook('components:dirs', async () => { | |
| const filePath = await resolvePath(opts.filePath) | |
| const names = await resolveModuleExportNames(filePath, { extensions: nuxt.options.extensions }) | |
| components.length = 0 | |
| for (const name of names) { | |
| components.push(normalizeComponent({ name: pascalCase([opts.prefix || '', name === 'default' ? '' : name]), export: name, ...opts })) | |
| } | |
| }) | |
| addComponents(components) | |
| } | |
| export function addComponentExports (opts: Omit<AddComponentOptions, 'name'> & { prefix?: string }) { | |
| const nuxt = useNuxt() | |
| const components: Component[] = [] | |
| nuxt.hook('components:dirs', async () => { | |
| const filePath = await resolvePath(opts.filePath) | |
| const names = await resolveModuleExportNames(filePath, { extensions: nuxt.options.extensions }) | |
| components.length = 0 | |
| for (const name of names) { | |
| const { prefix, ...rest } = opts | |
| const comp = normalizeComponent({ | |
| name: pascalCase([prefix || '', name === 'default' ? '' : name]), | |
| export: name, | |
| ...rest, | |
| meta: { ...(rest as any).meta, prefix } | |
| }) | |
| components.push(comp) | |
| } | |
| }) | |
| addComponents(components) | |
| } |
| nuxt.hook('components:extend', (components: Component[]) => { | ||
| for (const component of addedComponents) { | ||
| const existingComponentIndex = components.findIndex(c => (c.pascalName === component.pascalName || c.kebabName === component.kebabName) && c.mode === component.mode) | ||
| if (existingComponentIndex !== -1) { | ||
| const existingComponent = components[existingComponentIndex]! | ||
| const existingPriority = existingComponent.priority ?? 0 | ||
| const newPriority = component.priority ?? 0 | ||
|
|
||
| if (newPriority < existingPriority) { return } | ||
|
|
There was a problem hiding this comment.
return inside the loop aborts processing of later components
Inside the components:extend hook, the early‑exit on line 62 stops the whole hook, not just the current iteration.
If the first added component has a lower priority than an existing one, every subsequent component in addedComponents will be silently skipped.
Replace return with continue so that only the single low‑priority component is ignored:
- if (newPriority < existingPriority) { return }
+ if (newPriority < existingPriority) { continue }📝 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.
| nuxt.hook('components:extend', (components: Component[]) => { | |
| for (const component of addedComponents) { | |
| const existingComponentIndex = components.findIndex(c => (c.pascalName === component.pascalName || c.kebabName === component.kebabName) && c.mode === component.mode) | |
| if (existingComponentIndex !== -1) { | |
| const existingComponent = components[existingComponentIndex]! | |
| const existingPriority = existingComponent.priority ?? 0 | |
| const newPriority = component.priority ?? 0 | |
| if (newPriority < existingPriority) { return } | |
| nuxt.hook('components:extend', (components: Component[]) => { | |
| for (const component of addedComponents) { | |
| const existingComponentIndex = components.findIndex( | |
| c => | |
| (c.pascalName === component.pascalName || | |
| c.kebabName === component.kebabName) && | |
| c.mode === component.mode | |
| ) | |
| if (existingComponentIndex !== -1) { | |
| const existingComponent = components[existingComponentIndex]! | |
| const existingPriority = existingComponent.priority ?? 0 | |
| const newPriority = component.priority ?? 0 | |
| if (newPriority < existingPriority) { continue } |
TheAlexLichter
force-pushed
the
feat/rolldown-vite
branch
3 times, most recently
from
df010f9 to
bfeca58
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)
124-127: Consider using assignment instead of delete operatorThe conditional check for Rolldown is correctly implemented. However, the static analysis tool suggests avoiding the
deleteoperator for performance reasons.if(!vite.rolldownVersion) { // @ts-ignore Rolldown has no support for `output.manualChunks` - delete serverConfig.build.rollupOptions.output.manualChunks + serverConfig.build.rollupOptions.output.manualChunks = undefined }🧰 Tools
🪛 Biome (1.9.4)
[error] 126-126: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/nuxt/src/components/plugins/islands-transform.ts(1 hunks)packages/vite/src/plugins/analyze.ts(1 hunks)packages/vite/src/plugins/ssr-styles.ts(2 hunks)packages/vite/src/server.ts(2 hunks)packages/vite/src/vite.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/vite/src/plugins/analyze.ts
- packages/nuxt/src/components/plugins/islands-transform.ts
- packages/vite/src/plugins/ssr-styles.ts
- packages/vite/src/vite.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/vite/src/server.ts
[error] 126-126: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (1)
packages/vite/src/server.ts (1)
93-100: Good approach for conditional Rollup optionsThe conditional spread operator is a clean way to handle the Rolldown compatibility. The comment clearly explains why these options are only applied for non-Rolldown-powered Vite.
I see the
generatedCodeoption includes a temporary fix for a Vue core issue. Consider adding a TODO comment with a link to track when this workaround can be removed once the upstream issue is resolved.
TheAlexLichter
force-pushed
the
feat/rolldown-vite
branch
from
bfeca58 to
4c95619
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)
123-126: Improve performance by avoiding the delete operatorUsing the delete operator can impact performance. Instead of deleting the property, consider assigning
undefinedfor better performance while achieving the same effect.if(!vite.rolldownVersion) { // @ts-ignore Rolldown has no support for `output.manualChunks` - delete serverConfig.build.rollupOptions.output.manualChunks + serverConfig.build.rollupOptions.output.manualChunks = undefined }The type safety issue with
rolldownVersionalso exists here and should be addressed.🧰 Tools
🪛 Biome (1.9.4)
[error] 125-125: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🪛 GitHub Check: build
[failure] 123-123:
Property 'rolldownVersion' does not exist on type 'typeof import("/home/runner/work/nuxt/nuxt/node_modules/.pnpm/vite@6.3.0_@types[email protected][email protected][email protected][email protected][email protected][email protected]/node_modules/vite/dist/node/index")'. Did you mean 'rollupVersion'?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/nuxt/src/components/plugins/islands-transform.ts(1 hunks)packages/vite/src/plugins/analyze.ts(1 hunks)packages/vite/src/plugins/ssr-styles.ts(2 hunks)packages/vite/src/server.ts(2 hunks)packages/vite/src/vite.ts(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/vite/src/plugins/analyze.ts (1)
packages/vite/src/vite.ts (1)
bundle(31-248)
🪛 GitHub Check: build
packages/nuxt/src/components/plugins/islands-transform.ts
[failure] 204-204:
Unused '@ts-expect-error' directive.
[failure] 201-201:
Property 'rolldownVersion' does not exist on type 'typeof import("/home/runner/work/nuxt/nuxt/node_modules/.pnpm/vite@6.3.0_@types[email protected][email protected][email protected][email protected][email protected][email protected]/node_modules/vite/dist/node/index")'. Did you mean 'rollupVersion'?
packages/vite/src/plugins/analyze.ts
[failure] 31-31:
Unused '@ts-expect-error' directive.
packages/vite/src/plugins/ssr-styles.ts
[failure] 64-64:
Property 'rolldownVersion' does not exist on type 'PluginContextMeta'. Did you mean 'rollupVersion'?
packages/vite/src/server.ts
[failure] 94-94:
Property 'rolldownVersion' does not exist on type 'typeof import("/home/runner/work/nuxt/nuxt/node_modules/.pnpm/vite@6.3.0_@types[email protected][email protected][email protected][email protected][email protected][email protected]/node_modules/vite/dist/node/index")'. Did you mean 'rollupVersion'?
[failure] 123-123:
Property 'rolldownVersion' does not exist on type 'typeof import("/home/runner/work/nuxt/nuxt/node_modules/.pnpm/vite@6.3.0_@types[email protected][email protected][email protected][email protected][email protected][email protected]/node_modules/vite/dist/node/index")'. Did you mean 'rollupVersion'?
packages/vite/src/vite.ts
[failure] 96-96:
Property 'rolldownVersion' does not exist on type 'typeof import("/home/runner/work/nuxt/nuxt/node_modules/.pnpm/vite@6.3.0_@types[email protected][email protected][email protected][email protected][email protected][email protected]/node_modules/vite/dist/node/index")'. Did you mean 'rollupVersion'?
[failure] 110-110:
Unused '@ts-expect-error' directive.
[failure] 128-128:
Property 'rolldownVersion' does not exist on type 'typeof import("/home/runner/work/nuxt/nuxt/node_modules/.pnpm/vite@6.3.0_@types[email protected][email protected][email protected][email protected][email protected][email protected]/node_modules/vite/dist/node/index")'. Did you mean 'rollupVersion'?
🪛 GitHub Actions: ci
packages/nuxt/src/components/plugins/islands-transform.ts
[error] 201-201: TypeScript error TS2551: Property 'rolldownVersion' does not exist on type 'typeof import(...)'. Did you mean 'rollupVersion'?
🪛 Biome (1.9.4)
packages/vite/src/server.ts
[error] 125-125: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: code
🔇 Additional comments (6)
packages/vite/src/plugins/analyze.ts (1)
24-24: Type assertion improves code clarity.Adding the type assertion
as RenderedModuleclarifies the expected type before spreading the module object, improving type safety and making the code more maintainable.packages/nuxt/src/components/plugins/islands-transform.ts (1)
202-206: Appropriate conditional handling forrolldown-vitecompatibility.Conditionally setting
preserveEntrySignaturesonly when not using Rolldown is the correct approach, as this option is not supported in Rolldown. The comment explaining why the option is needed is also helpful.🧰 Tools
🪛 GitHub Check: build
[failure] 204-204:
Unused '@ts-expect-error' directive.packages/vite/src/plugins/ssr-styles.ts (1)
76-80: Good adaptation for Rolldown compatibility.This conditional logic properly handles the difference in assetFileNames options between Rolldown and standard Rollup, ensuring compatibility with both bundlers. The comment explaining why this is needed is also helpful for maintainability.
packages/vite/src/vite.ts (3)
95-101: Appropriate conditional handling for watch mode limitations.The comment and conditional logic correctly handle the limitation that watch mode is not supported in rolldown-vite yet.
🧰 Tools
🪛 GitHub Check: build
[failure] 96-96:
Property 'rolldownVersion' does not exist on type 'typeof import("/home/runner/work/nuxt/nuxt/node_modules/.pnpm/vite@6.3.0_@types[email protected][email protected][email protected][email protected][email protected][email protected]/node_modules/vite/dist/node/index")'. Did you mean 'rollupVersion'?
129-131: Good conditional handling for build watch mode in production.The conditional check and error suppression correctly handle the fact that the watch option is not supported in rolldown-vite yet.
110-111:❓ Verification inconclusive
Check if the
@ts-expect-errorcomment is still needed.The comment references a known issue with rolldown-vite integration, but static analysis indicates it might be unused. Consider verifying if this error suppression is still necessary.
🏁 Script executed:
#!/bin/bash # Check if the error actually occurs without the @ts-expect-error comment # Create a temporary file with the error comment removed TEMP_FILE=$(mktemp) cat packages/vite/src/vite.ts | grep -n -A 1 -B 1 "@ts-expect-error" | grep -v "Not supported" > $TEMP_LINES line_numbers=$(echo "$TEMP_LINES" | grep -o '^[0-9]*' | paste -sd, -) if [ -n "$line_numbers" ]; then cat packages/vite/src/vite.ts | grep -v -f <(echo "$line_numbers" | tr ',' '\n' | while read line; do echo "^$line:"; done) > $TEMP_FILE # Run TypeScript check on the temporary file echo "Running TypeScript check without @ts-expect-error comments:" npx tsc --noEmit --skipLibCheck $TEMP_FILE 2>&1 | grep -A 3 -B 3 "replace" # Clean up rm $TEMP_FILE fiLength of output: 415
Verify the necessity of the
@ts-expect-errorsuppressionWe attempted to automate removal of the suppression in packages/vite/src/vite.ts (around lines 110–111) and re‐run the TypeScript check, but the script failed to produce conclusive results. Please manually:
- Remove or comment out the line
// @ts-expect-error https://github.com/vitejs/rolldown-vite/issues/117- Rebuild or run
npx tsc --noEmit --skipLibCheckagainst the full projectIf no errors appear, you can safely delete the suppression. If a type error is reported on the
replace({...})call, retain the comment.🧰 Tools
🪛 GitHub Check: build
[failure] 110-110:
Unused '@ts-expect-error' directive.
|
|
||
| // don't use 'strict', this would create another "facade" chunk for the entry file, causing the ssr styles to not detect everything | ||
| rollupOptions.preserveEntrySignatures = 'allow-extension' | ||
| const isRolldown = !!(await import('vite')).rolldownVersion |
There was a problem hiding this comment.
Fix property name: rolldownVersion should be rollupVersion.
The property rolldownVersion doesn't exist on the vite module. According to the TypeScript error in the build log, you likely meant rollupVersion.
-const isRolldown = !!(await import('vite')).rolldownVersion
+const isRolldown = !!(await import('vite')).rollupVersion📝 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.
| const isRolldown = !!(await import('vite')).rolldownVersion | |
| const isRolldown = !!(await import('vite')).rollupVersion |
🧰 Tools
🪛 GitHub Check: build
[failure] 201-201:
Property 'rolldownVersion' does not exist on type 'typeof import("/home/runner/work/nuxt/nuxt/node_modules/.pnpm/vite@6.3.0_@types[email protected][email protected][email protected][email protected][email protected][email protected]/node_modules/vite/dist/node/index")'. Did you mean 'rollupVersion'?
🪛 GitHub Actions: ci
[error] 201-201: TypeScript error TS2551: Property 'rolldownVersion' does not exist on type 'typeof import(...)'. Did you mean 'rollupVersion'?
There was a problem hiding this comment.
nope, it should not. That's on purpose 😋
There was a problem hiding this comment.
@TheAlexLichter I understand now! Thanks for the clarification. You're intentionally checking for rolldownVersion to detect if rolldown-vite is being used, despite the TypeScript error. This aligns with the PR's purpose of testing rolldown-vite compatibility.
✏️ Learnings added
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.734Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
packages/vite/src/vite.ts
Outdated
| exclude: nuxt.options.ignore, | ||
| }, | ||
| // TODO: Watch is not supported in rolldown-vite yet | ||
| ...vite.rolldownVersion ? {} : { |
There was a problem hiding this comment.
Fix property name: rolldownVersion should be rollupVersion.
The property rolldownVersion doesn't exist on the vite module. According to the static analysis, you likely meant rollupVersion.
- ...vite.rolldownVersion ? {} : {
+ ...vite.rollupVersion ? {} : {📝 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.
| ...vite.rolldownVersion ? {} : { | |
| ...vite.rollupVersion ? {} : { |
🧰 Tools
🪛 GitHub Check: build
[failure] 96-96:
Property 'rolldownVersion' does not exist on type 'typeof import("/home/runner/work/nuxt/nuxt/node_modules/.pnpm/vite@6.3.0_@types[email protected][email protected][email protected][email protected][email protected][email protected]/node_modules/vite/dist/node/index")'. Did you mean 'rollupVersion'?
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/vite/src/server.ts (1)
93-101: Good addition of Rolldown compatibility checksThe conditional application of Rollup options when not using Rolldown is well-implemented. This correctly avoids using unsupported options like
preserveModulesandgeneratedCodewith Rolldown.However, there's a type safety issue with
vite.rolldownVersionsince this property isn't defined in Vite's type definitions.Consider adding a proper type definition rather than using
@ts-ignorefor improved type safety and code maintainability. You could create or extend type definitions in a.d.tsfile:declare module 'vite' { interface ViteDevServer { rolldownVersion?: string } }
🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)
124-128: Consider an alternative to the delete operatorThe conditional deletion of
manualChunksis correctly implemented to ensure compatibility with Rolldown. However, using thedeleteoperator can have a minor performance impact.Consider using an assignment to
undefinedinstead, which accomplishes the same goal with slightly better performance:- delete serverConfig.build.rollupOptions.output.manualChunks + serverConfig.build.rollupOptions.output.manualChunks = undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 127-127: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/nuxt/src/components/plugins/islands-transform.ts(1 hunks)packages/vite/src/plugins/analyze.ts(1 hunks)packages/vite/src/plugins/ssr-styles.ts(2 hunks)packages/vite/src/server.ts(2 hunks)packages/vite/src/vite.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/vite/src/plugins/analyze.ts
- packages/vite/src/plugins/ssr-styles.ts
- packages/vite/src/vite.ts
- packages/nuxt/src/components/plugins/islands-transform.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/vite/src/server.ts
[error] 127-127: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: code
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)
124-133: Conditional chunk configuration based on bundler typeThe code handles different chunk configurations based on the bundler:
- For Rollup (non-Rolldown): Delete
manualChunks- For Rolldown: Delete
advancedChunksinsteadThe TypeScript suppression is appropriate since
rolldownVersionandadvancedChunksaren't in standard type definitions.Consider using property assignment to
undefinedinstead of thedeleteoperator for better performance:- delete serverConfig.build.rollupOptions.output.manualChunks + serverConfig.build.rollupOptions.output.manualChunks = undefined- delete serverConfig.build.rollupOptions.output.advancedChunks + serverConfig.build.rollupOptions.output.advancedChunks = undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 127-127: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 131-131: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/nuxt/src/components/module.ts(1 hunks)packages/nuxt/src/components/plugins/islands-transform.ts(3 hunks)packages/schema/src/config/experimental.ts(1 hunks)packages/vite/src/plugins/ssr-styles.ts(2 hunks)packages/vite/src/server.ts(2 hunks)packages/vite/src/vite.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/nuxt/src/components/plugins/islands-transform.ts
- packages/vite/src/plugins/ssr-styles.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.734Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
packages/nuxt/src/components/module.ts (1)
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.734Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
packages/vite/src/server.ts (1)
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.734Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
packages/vite/src/vite.ts (1)
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.734Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
🧬 Code Graph Analysis (1)
packages/vite/src/vite.ts (2)
packages/kit/src/index.ts (1)
isIgnored(22-22)packages/kit/src/ignore.ts (1)
isIgnored(13-31)
🪛 Biome (1.9.4)
packages/vite/src/server.ts
[error] 127-127: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 131-131: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/vite/src/vite.ts
[error] 114-114: Expected an identifier, an array pattern, or an object pattern but instead found 'true'.
Expected an identifier, an array pattern, or an object pattern here.
(parse)
[error] 114-114: expected : but instead found ,
Remove ,
(parse)
[error] 114-114: Expected a function body but instead found '}'.
Expected a function body here.
(parse)
🪛 GitHub Check: build
packages/vite/src/vite.ts
[failure] 114-114:
':' expected.
[failure] 114-114:
Identifier expected. 'true' is a reserved word that cannot be used here.
🪛 GitHub Actions: ci
packages/vite/src/vite.ts
[error] 114-114: TypeScript error TS1359: Identifier expected. 'true' is a reserved word that cannot be used here.
🔇 Additional comments (6)
packages/schema/src/config/experimental.ts (1)
633-638: New experimental flag for Rolldown compatibilityThis flag enables compatibility features when using Rolldown or
rolldown-vite, ensuring correct handling for both bundlers by conditionally applying or omitting certain options.packages/nuxt/src/components/module.ts (1)
256-256: Pass Rolldown compatibility flag to ComponentsChunkPluginThe flag is correctly passed to the Vite plugin, which will use it to conditionally adjust plugin behaviour based on the bundler in use.
packages/vite/src/server.ts (1)
93-101: Conditional Rollup options for non-Rolldown buildsThe code correctly implements conditional application of
preserveModulesandgeneratedCodeoptions that are unsupported in Rolldown. The TypeScript suppression comment is necessary due to therolldownVersionproperty missing from Vite's type definitions.As noted in a previous review,
rolldownVersionis intentionally used instead ofrollupVersionto detect rolldown-vite despite TypeScript complaints.packages/vite/src/vite.ts (3)
80-82: Use define for global replacement in RolldownCorrectly uses the
defineoption to replaceglobalwithglobalThiswhen using Rolldown, maintaining same functionality through different mechanisms based on bundler capability.
97-105: Conditionally apply watch configuration for non-Rolldown buildsWatch configuration is correctly omitted for Rolldown as it's not supported, and applied only for standard Vite/Rollup builds.
131-136: Skip watch configuration for Rolldown in non-dev modeCorrectly handles disabling the watch configuration only for non-Rolldown builds in production, as Rolldown does not support this feature yet.
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)
127-137: Good handling of chunk configuration between Rollup and RolldownThe conditional logic correctly handles the different chunking capabilities between Rollup and Rolldown, removing the appropriate properties based on which bundler is in use.
However, the
deleteoperator usage flagged by static analysis could be replaced with undefined assignments for better performance:- delete serverConfig.build.rollupOptions.output.manualChunks + serverConfig.build.rollupOptions.output.manualChunks = undefinedand
- delete serverConfig.build.rollupOptions.output.advancedChunks + serverConfig.build.rollupOptions.output.advancedChunks = undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 131-131: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 136-136: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/nuxt/src/components/module.ts(1 hunks)packages/nuxt/src/components/plugins/islands-transform.ts(3 hunks)packages/schema/src/config/experimental.ts(1 hunks)packages/vite/src/plugins/ssr-styles.ts(1 hunks)packages/vite/src/server.ts(2 hunks)packages/vite/src/vite.ts(2 hunks)test/bundle.test.ts(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/nuxt/src/components/module.ts
- test/bundle.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/schema/src/config/experimental.ts
- packages/vite/src/vite.ts
- packages/vite/src/plugins/ssr-styles.ts
- packages/nuxt/src/components/plugins/islands-transform.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.734Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
packages/vite/src/server.ts (1)
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.734Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
🪛 Biome (1.9.4)
packages/vite/src/server.ts
[error] 131-131: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 136-136: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: code
🔇 Additional comments (1)
packages/vite/src/server.ts (1)
93-104: Correct approach for conditionally applying Rollup-specific optionsThe implementation properly detects Rolldown using the
rolldownVersionproperty and only applies the unsupported options when using standard Vite with Rollup. This aligns with the PR's goal of testingrolldown-vitecompatibility.I see you've included a helpful comment explaining the temporary fix for Vue core issue #8351 through the
generatedCodeoption.
CodSpeed Performance ReportMerging #31812 will not alter performanceComparing Summary
|
|
@TheAlexLichter do you think we can get this in the next minor? |
|
Yes! |
TheAlexLichter
force-pushed
the
feat/rolldown-vite
branch
from
fdbcbe7 to
fbc982a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/nuxt/test/component-loader.test.ts (1)
215-220: Same here: cast Vue plugins instead of using @ts-ignoreReuse the casting approach to avoid suppressions and keep typings intact.
Diff within the selected lines:
- // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore Rolldown-vite type error - vuePlugin(), - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore Rolldown-vite type error - vuePluginJsx(), + vuePlugin() as unknown as RollupPlugin, + vuePluginJsx() as unknown as RollupPlugin,Ensure this file also imports:
import type { Plugin as RollupPlugin } from 'rollup'packages/vite/src/plugins/analyze.ts (1)
30-32: Remove suppression and cast the visualizer to a Vite PluginThe TS ignore is brittle. Cast the Rollup plugin to Vite’s
Plugin(structurally compatible) and drop the suppression.Apply this diff:
- // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore Rolldown-vite type error - visualizer({ + visualizer({ ...analyzeOptions, filename: 'filename' in analyzeOptions ? analyzeOptions.filename!.replace('{name}', 'client') : undefined, title: 'Client bundle stats', gzipSize: true, brotliSize: true, - }), + }) as unknown as Plugin,Alternatively (cleaner), inject it via the plugin’s
confighook underbuild.rollupOptions.pluginsto avoid any casts:{ name: 'nuxt:analyze-visualizer', config() { return { build: { rollupOptions: { plugins: [visualizer({ ...analyzeOptions, filename: 'filename' in analyzeOptions ? analyzeOptions.filename!.replace('{name}', 'client') : undefined, title: 'Client bundle stats', gzipSize: true, brotliSize: true })] } } } } }
🧹 Nitpick comments (1)
packages/nuxt/test/lazy-hydration-macro-transform.test.ts (1)
167-172: Prefer typed interop over blanket @ts-ignore for Vue pluginsSuppressing types is fine short-term, but you can keep tests type-safe without ignores by casting the Vue plugins to Rollup’s Plugin interface.
Apply this diff locally within the selected lines:
- // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore Rolldown-vite type error - vuePlugin(), - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore Rolldown-vite type error - vuePluginJsx(), + vuePlugin() as unknown as RollupPlugin, + vuePluginJsx() as unknown as RollupPlugin,And add this import near the top (outside the selected lines):
import type { Plugin as RollupPlugin } from 'rollup'This removes the need for TS/ESLint suppressions while maintaining correct runtime behaviour.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/nuxt/test/component-loader.test.ts(1 hunks)packages/nuxt/test/lazy-hydration-macro-transform.test.ts(1 hunks)packages/schema/src/config/common.ts(1 hunks)packages/vite/src/plugins/analyze.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/test/component-loader.test.tspackages/schema/src/config/common.tspackages/vite/src/plugins/analyze.tspackages/nuxt/test/lazy-hydration-macro-transform.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Write unit tests for core functionality using
vitest
Files:
packages/nuxt/test/component-loader.test.tspackages/nuxt/test/lazy-hydration-macro-transform.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.753Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
PR: nuxt/nuxt#26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
packages/nuxt/test/component-loader.test.ts
📚 Learning: 2025-04-18T18:33:41.753Z
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.753Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
Applied to files:
packages/schema/src/config/common.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: code
🔇 Additional comments (2)
packages/vite/src/plugins/analyze.ts (1)
23-24: Good fix: ensure RenderedModule typing when minifying per-moduleSpreading
module as RenderedModuleand overridingcodealigns the promise type to[string, RenderedModule]and avoids structural typing pitfalls. No further action needed.packages/schema/src/config/common.ts (1)
264-266: Annotate experimental placeholder and confirm downstream usageIt appears the
watchers.rolldownslot isn’t consumed anywhere in the codebase today. Mark it as experimental and manually verify whether any consumers (e.g. Vite/Rollup integrations, test suites or docs) rely on it—otherwise it can remain a deferred placeholder.
- packages/schema/src/config/common.ts (around line 264)
rolldown: { + // Experimental: placeholder for rolldown watch options (currently ignored by rolldown-vite build) },
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/vite/src/vite.ts (2)
56-66: Use @ts-expect-error (not @ts-ignore) for intentional rolldown detectionThe use of vite.rolldownVersion is intentional (per team learnings), but prefer @ts-expect-error so the suppression is revisited when types catch up. Keep the one-line ESLint disable if needed.
- // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore Rolldown-check + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-expect-error - rolldownVersion exists at runtime when using rolldown-vite but is not in types yet if (vite.rolldownVersion) { // Delete unused esbuild settings if (viteConfig.esbuild) { delete viteConfig.esbuild } if (viteConfig.optimizeDeps?.esbuildOptions) { delete viteConfig.optimizeDeps.esbuildOptions } }
132-144: Consistent TS suppression and minor watch note
- Align with the earlier comment: use @ts-expect-error instead of @ts-ignore.
- Note: rolldown’s build watch is not fully implemented yet; this conditional shape is fine for now.
- // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore Rolldown-check + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-expect-error - rolldownVersion exists at runtime when using rolldown-vite but is not in types yet watch: (vite.rolldownVersion ? { // TODO: https://github.com/rolldown/rolldown/issues/5799 for ignored fn exclude: [...nuxt.options.ignore, /[\\/]node_modules[\\/]/], } : { chokidar: { ...nuxt.options.watchers.chokidar, ignored: [isIgnored, /[\\/]node_modules[\\/]/] }, exclude: nuxt.options.ignore, } ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/vite/src/vite.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/vite/src/vite.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.753Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
📚 Learning: 2025-04-18T18:33:41.753Z
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.753Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
Applied to files:
packages/vite/src/vite.ts
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
PR: nuxt/nuxt#26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
packages/vite/src/vite.ts
🧬 Code Graph Analysis (1)
packages/vite/src/vite.ts (1)
packages/kit/src/ignore.ts (1)
isIgnored(13-31)
🪛 GitHub Actions: ci
packages/vite/src/vite.ts
[warning] 1-1: Implicitly bundling 'rolldown/experimental' detected during prepack.
[error] 1-1: Prepack step failed for '@nuxt/[email protected] prepack' (unbuild) with exit code 1. Warnings were escalated to errors; consider setting 'failOnWarn: false' to allow warnings.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: code
packages/vite/src/vite.ts
Outdated
| nuxt.hook('vite:extendConfig', async (config) => { | ||
| const replaceValues = Object.create(null) | ||
| for (const key in config.define!) { | ||
| if (key.startsWith('import.meta.')) { | ||
| replaceOptions[key] = config.define![key] | ||
| replaceValues[key] = config.define![key] | ||
| } | ||
| } | ||
|
|
||
| config.plugins!.push(replace(replaceOptions)) | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore Rolldown-check | ||
| if(vite.rolldownVersion) { | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore Rolldown-specific | ||
| const { replacePlugin } = await import('rolldown/experimental') | ||
| config.plugins!.push(replacePlugin(replaceValues, { preventAssignment: true })) | ||
| } else { | ||
| const { default: replacePlugin } = (await import('@rollup/plugin-replace')) | ||
| config.plugins!.push(replacePlugin({preventAssignment: true, ...replaceValues})) | ||
| } | ||
| }) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix prepack failure: avoid implicitly bundling 'rolldown/experimental'
CI fails because unbuild detects implicit bundling of rolldown/experimental from the literal dynamic import. Defer resolution to runtime with a non-static import to keep it external/optional. Also switch to @ts-expect-error for the intentional rolldownVersion usage. The fallback tries both common call signatures to tolerate API differences.
nuxt.hook('vite:extendConfig', async (config) => {
const replaceValues = Object.create(null)
@@
-
- // eslint-disable-next-line @typescript-eslint/ban-ts-comment
- // @ts-ignore Rolldown-check
- if(vite.rolldownVersion) {
- // eslint-disable-next-line @typescript-eslint/ban-ts-comment
- // @ts-ignore Rolldown-specific
- const { replacePlugin } = await import('rolldown/experimental')
- config.plugins!.push(replacePlugin(replaceValues, { preventAssignment: true }))
- } else {
- const { default: replacePlugin } = (await import('@rollup/plugin-replace'))
- config.plugins!.push(replacePlugin({preventAssignment: true, ...replaceValues}))
- }
+ const dynamicImport = (m: string) => Function('m','return import(m)')(m)
+ // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+ // @ts-expect-error - rolldownVersion exists at runtime when using rolldown-vite but is not in types yet
+ if (vite.rolldownVersion) {
+ let replacePlugin: any
+ try {
+ // Defer resolve to runtime to avoid implicitly bundling "rolldown/experimental" during prepack
+ const mod: any = await dynamicImport('rolldown' + '/experimental')
+ replacePlugin = mod.replacePlugin
+ } catch {
+ logger.warn('Rolldown replace plugin is unavailable; skipping import.meta.* replacements under rolldown.')
+ }
+ if (replacePlugin) {
+ // Try both calling conventions for compatibility with differing plugin APIs
+ try {
+ config.plugins!.push(replacePlugin(replaceValues, { preventAssignment: true }))
+ } catch {
+ config.plugins!.push(replacePlugin({ values: replaceValues, preventAssignment: true }))
+ }
+ }
+ } else {
+ const { default: replace } = await dynamicImport('@rollup/plugin-replace')
+ config.plugins!.push(replace({ preventAssignment: true, ...replaceValues }))
+ }
})Run this to ensure no remaining literal imports of rolldown/experimental are present (which would still trigger bundling):
🏁 Script executed:
#!/bin/bash
# Should return no results
rg -nP -C1 "(from|import\\()\\s*['\"]rolldown/experimental['\"]" -g '!**/dist/**'Length of output: 369
Avoid bundling rolldown/experimental via a static import
CI is still detecting the literal import of rolldown/experimental, so it’s being implicitly bundled during prepack. Please replace that static dynamic import with a runtime-deferred one as below.
• packages/vite/src/vite.ts
– Line 231:
const { replacePlugin } = await import('rolldown/experimental')
Suggested patch:
- // eslint-disable-next-line @typescript-eslint/ban-ts-comment
- // @ts-ignore Rolldown-specific
- const { replacePlugin } = await import('rolldown/experimental')
+ // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+ // @ts-expect-error - this exists at runtime under rolldown-vite
+ let replacePlugin: any
+ try {
+ // Defer resolution to runtime to avoid bundling during prepack
+ const mod: any = await dynamicImport('rolldown' + '/experimental')
+ replacePlugin = mod.replacePlugin
+ } catch {
+ logger.warn('Rolldown replace plugin unavailable; skipping import.meta.* replacements.')
+ }Don’t forget to add the dynamicImport helper and wrap this block in the if (vite.rolldownVersion) check as outlined in the earlier diff.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/vite/build.config.ts (1)
20-21: Optional: also externalise 'rolldown' root to future‑proof accidental importsIf any code (now or later) imports from 'rolldown' (not just the experimental subpath), unbuild could attempt to resolve it during build. Marking the root as external too is a low-risk guardrail.
Apply this diff:
externals: [ + 'rolldown', 'rolldown/experimental', '@nuxt/schema', ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/vite/build.config.ts(1 hunks)packages/vite/src/vite.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vite/src/vite.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/vite/build.config.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.753Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
📚 Learning: 2025-04-18T18:33:41.753Z
Learnt from: TheAlexLichter
PR: nuxt/nuxt#31812
File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202
Timestamp: 2025-04-18T18:33:41.753Z
Learning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.
Applied to files:
packages/vite/build.config.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: test-fixtures (ubuntu-latest, built, webpack, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, webpack, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, rspack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-benchmark
- GitHub Check: release-pkg-pr-new
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: typecheck (windows-latest, bundler)
🔇 Additional comments (2)
packages/vite/build.config.ts (2)
20-21: Externalising rolldown/experimental is the right call for optional runtime importThis prevents bundling an optional dependency and aligns with the dynamic import usage elsewhere, avoiding install-time resolution issues for users not opting into Rolldown.
20-21: No static imports of ‘rolldown/experimental’ detected
- Verified absence of static ESM and CommonJS imports of ‘rolldown/experimental’
- Confirmed dynamic import in packages/vite/src/vite.ts at line 231:
const { replacePlugin } = await import('rolldown/experimental')
TheAlexLichter
force-pushed
the
feat/rolldown-vite
branch
from
e59c0cf to
fbc982a
Compare
|
@danielroe Did you do that intentionally? 🙃 
|
|
🤯 |
Resolves #30654
PR running with the actual rolldown-vite dep for testing
Current issues:
Watch mode forbuildis not implemented inrolldown-viteat the moment. We ignore the watch options in this PR for now.Partially available (but of course no chokidar watching). Should be worked around in the future if possibleNeeds to be implemented in the PR when available.Used Rollup plugins throw a type error (rolldown-viteplugin types throw when using rollup plugins vitejs/rolldown-vite#117)Can't easly detect Rolldown in an unplugin viteconfighook yetworkaround exists but would lead to implicitly bundling vite (meh!)Experimental option is the way to go now I'd sayesbuild options should be moved/merged/ignored (as only rollupOptions are used)This is just a TODO and can be implemented in this PR alreadypreserveModules- [Feature Request]: add support forpreserveModulesoutput option rolldown/rolldown#2622generatedCode- Supportoutput.generatedCodeoption rolldown/rolldown#206