Skip to content

feat(nuxt,vite): add experimental support for rolldown-vite #31812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TheAlexLichter
Copy link
Member

@TheAlexLichter TheAlexLichter commented Apr 16, 2025

Resolves #30654
PR running with the actual rolldown-vite dep for testing

Current issues:

Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@TheAlexLichter TheAlexLichter marked this pull request as draft April 16, 2025 07:03
Copy link

coderabbitai bot commented Apr 16, 2025

Walkthrough

Conditional Rolldown-aware behaviour was added across Vite/Nuxt internals: build output.generatedCode is omitted and manualChunks deletion is replaced with advancedChunks deletion when vite.rolldownVersion is truthy; analyze plugin now casts modules as RenderedModule and adds TS-ignore comments; vite.ts switches to a dynamic, Rolldown-aware replace plugin import, removes esbuild options under Rolldown, and adjusts the watcher shape; two tests add @ts-ignore lines before vuePlugin()/vuePluginJsx(); schema adds watchers.rolldown: {}; build.config externals now include 'rolldown/experimental'.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/rolldown-vite

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)

124-127: Consider using undefined assignment instead of delete operator

The static analysis tool correctly flagged a potential performance issue with the delete operator. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4edb14c and b0182b8.

⛔ Files ignored due to path filters (7)
  • package.json is excluded by !package.json, !**/package.json
  • packages/kit/package.json is excluded by !**/package.json
  • packages/nuxt/package.json is excluded by !**/package.json
  • packages/schema/package.json is excluded by !**/package.json
  • packages/ui-templates/package.json is excluded by !**/package.json
  • packages/vite/package.json is excluded by !**/package.json
  • pnpm-lock.yaml is 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 good

This addition of onlyBuiltDependencies with rolldown as its sole entry aligns with the PR's purpose of testing rolldown-vite integration. It instructs pnpm to exclusively consider rolldown for building, which is appropriate for targeted testing.

packages/vite/src/server.ts (1)

93-100: Conditional application of Rollup options for non-Rolldown builds

The 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 TheAlexLichter removed the request for review from danielroe April 16, 2025 07:25
@TheAlexLichter TheAlexLichter marked this pull request as ready for review April 16, 2025 10:08
@danielroe
Copy link
Member

@TheAlexLichter do you think we could ship under an experimental flag? users would still need to manually set a resolution for vite, I think...

@TheAlexLichter
Copy link
Member Author

image That seems new 👀🤔

Tests are failing at the moment (at least locally).

We should be able to ship it with an experimental flag, yes!

@huang-julien
Copy link
Member

I got an issue with vite:json when using @nuxtjs/i18n.
I haven't debugged it yet. But @BobbieGoede and @kazupon are probably interested about that.

@BobbieGoede
Copy link
Member

I got an issue with vite:json when using @nuxtjs/i18n. I haven't debugged it yet. But @BobbieGoede and @kazupon are probably interested about that.

@huang-julien
I haven't checked yet, but I'm guessing this is probably related to @intlify/unplugin-vue-i18n, perhaps https://github.com/intlify/bundle-tools/blob/main/packages/unplugin-vue-i18n/src/core/resource.ts#L172-L199 (an earlier commit/version though)

@kazupon
Copy link
Member

kazupon commented Apr 17, 2025

related issue

@BobbieGoede
I'v just commented that issue.
We can do the same thing by using the hook's order and handler

@lansi951
Copy link
Contributor

How about using the experimental.enableNativePlugin option?
It helps improve performance.
However, it doesn't work properly in dev mode currently, so should use the resolver in dev mode.

nuxt.config.ts:

$development: {
  vite: {
    experimental: {
      enableNativePlugin: 'resolver',
    },
  },
},
$production: {
  vite: {
    experimental: {
      enableNativePlugin: true,
    },
  },
}

@TheAlexLichter
Copy link
Member Author

How about using the experimental.enableNativePlugin option?

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

@TheAlexLichter
Copy link
Member Author

TheAlexLichter commented Apr 18, 2025

@TheAlexLichter do you think we could ship under an experimental flag? users would still need to manually set a resolution for vite, I think...

@danielroe I wonder if we could even ship it unflagged as we have rolldown detection in place everywhere ☺️

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
packages/kit/src/components.ts (1)

50-76: Potential accumulation of identical hooks

addComponents() registers a new components:extend hook every time it is called.
If an extension repeatedly calls addComponent/addComponentExports in a loop, dozens of identical hooks will be attached, slightly degrading performance and making stack‑traces noisy.

Consider caching the list of addedComponents per‑Nuxt‑instance (e.g. with a WeakMap) 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 improvement

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between 2632aa5 and 2025873.

⛔ Files ignored due to path filters (7)
  • package.json is excluded by !package.json, !**/package.json
  • packages/kit/package.json is excluded by !**/package.json
  • packages/nuxt/package.json is excluded by !**/package.json
  • packages/schema/package.json is excluded by !**/package.json
  • packages/ui-templates/package.json is excluded by !**/package.json
  • packages/vite/package.json is excluded by !**/package.json
  • pnpm-lock.yaml is 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

addComponentExports is now surfaced alongside the existing helpers, keeping the public API coherent.

Comment on lines 27 to 40
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)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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)
}

Comment on lines 54 to 63
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 }

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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 TheAlexLichter force-pushed the feat/rolldown-vite branch 3 times, most recently from df010f9 to bfeca58 Compare April 18, 2025 13:13
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)

124-127: Consider using assignment instead of delete operator

The conditional check for Rolldown is correctly implemented. However, the static analysis tool suggests avoiding the delete operator 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

📥 Commits

Reviewing files that changed from the base of the PR and between 2025873 and bfeca58.

📒 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 options

The 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 generatedCode option 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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)

123-126: Improve performance by avoiding the delete operator

Using the delete operator can impact performance. Instead of deleting the property, consider assigning undefined for 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 rolldownVersion also 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+node@22.14.1_jiti@2.4.2_sass@1.78.0_terser@5.32.0_tsx@4.19.2_yaml@2.7.1/node_modules/vite/dist/node/index")'. Did you mean 'rollupVersion'?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bfeca58 and 4c95619.

📒 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+node@22.14.1_jiti@2.4.2_sass@1.78.0_terser@5.32.0_tsx@4.19.2_yaml@2.7.1/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+node@22.14.1_jiti@2.4.2_sass@1.78.0_terser@5.32.0_tsx@4.19.2_yaml@2.7.1/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+node@22.14.1_jiti@2.4.2_sass@1.78.0_terser@5.32.0_tsx@4.19.2_yaml@2.7.1/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+node@22.14.1_jiti@2.4.2_sass@1.78.0_terser@5.32.0_tsx@4.19.2_yaml@2.7.1/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+node@22.14.1_jiti@2.4.2_sass@1.78.0_terser@5.32.0_tsx@4.19.2_yaml@2.7.1/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 RenderedModule clarifies 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 for rolldown-vite compatibility.

Conditionally setting preserveEntrySignatures only 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+node@22.14.1_jiti@2.4.2_sass@1.78.0_terser@5.32.0_tsx@4.19.2_yaml@2.7.1/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-error comment 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
fi

Length of output: 415


Verify the necessity of the @ts-expect-error suppression

We 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 --skipLibCheck against the full project

If 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
Copy link

@coderabbitai coderabbitai bot Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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+node@22.14.1_jiti@2.4.2_sass@1.78.0_terser@5.32.0_tsx@4.19.2_yaml@2.7.1/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'?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, it should not. That's on purpose 😋

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

exclude: nuxt.options.ignore,
},
// TODO: Watch is not supported in rolldown-vite yet
...vite.rolldownVersion ? {} : {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
...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+node@22.14.1_jiti@2.4.2_sass@1.78.0_terser@5.32.0_tsx@4.19.2_yaml@2.7.1/node_modules/vite/dist/node/index")'. Did you mean 'rollupVersion'?

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
packages/vite/src/server.ts (1)

93-101: Good addition of Rolldown compatibility checks

The conditional application of Rollup options when not using Rolldown is well-implemented. This correctly avoids using unsupported options like preserveModules and generatedCode with Rolldown.

However, there's a type safety issue with vite.rolldownVersion since this property isn't defined in Vite's type definitions.

Consider adding a proper type definition rather than using @ts-ignore for improved type safety and code maintainability. You could create or extend type definitions in a .d.ts file:

declare module 'vite' {
  interface ViteDevServer {
    rolldownVersion?: string
  }
}
🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)

124-128: Consider an alternative to the delete operator

The conditional deletion of manualChunks is correctly implemented to ensure compatibility with Rolldown. However, using the delete operator can have a minor performance impact.

Consider using an assignment to undefined instead, 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4c95619 and 16e1911.

📒 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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)

124-133: Conditional chunk configuration based on bundler type

The code handles different chunk configurations based on the bundler:

  • For Rollup (non-Rolldown): Delete manualChunks
  • For Rolldown: Delete advancedChunks instead

The TypeScript suppression is appropriate since rolldownVersion and advancedChunks aren't in standard type definitions.

Consider using property assignment to undefined instead of the delete operator 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

📥 Commits

Reviewing files that changed from the base of the PR and between 16e1911 and 891ac75.

📒 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 compatibility

This 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 ComponentsChunkPlugin

The 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 builds

The code correctly implements conditional application of preserveModules and generatedCode options that are unsupported in Rolldown. The TypeScript suppression comment is necessary due to the rolldownVersion property missing from Vite's type definitions.

As noted in a previous review, rolldownVersion is intentionally used instead of rollupVersion to detect rolldown-vite despite TypeScript complaints.

packages/vite/src/vite.ts (3)

80-82: Use define for global replacement in Rolldown

Correctly uses the define option to replace global with globalThis when using Rolldown, maintaining same functionality through different mechanisms based on bundler capability.


97-105: Conditionally apply watch configuration for non-Rolldown builds

Watch 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 mode

Correctly handles disabling the watch configuration only for non-Rolldown builds in production, as Rolldown does not support this feature yet.

Copy link

pkg-pr-new bot commented Apr 23, 2025

Open in StackBlitz

@nuxt/kit

npm i https://pkg.pr.new/@nuxt/kit@31812

nuxt

npm i https://pkg.pr.new/nuxt@31812

@nuxt/rspack-builder

npm i https://pkg.pr.new/@nuxt/rspack-builder@31812

@nuxt/schema

npm i https://pkg.pr.new/@nuxt/schema@31812

@nuxt/vite-builder

npm i https://pkg.pr.new/@nuxt/vite-builder@31812

@nuxt/webpack-builder

npm i https://pkg.pr.new/@nuxt/webpack-builder@31812

commit: fbc982a

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)

127-137: Good handling of chunk configuration between Rollup and Rolldown

The 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 delete operator 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 = undefined

and

- 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

📥 Commits

Reviewing files that changed from the base of the PR and between 6e7dffe and 37d0b95.

📒 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 options

The implementation properly detects Rolldown using the rolldownVersion property and only applies the unsupported options when using standard Vite with Rollup. This aligns with the PR's goal of testing rolldown-vite compatibility.

I see you've included a helpful comment explaining the temporary fix for Vue core issue #8351 through the generatedCode option.

Copy link

codspeed-hq bot commented Apr 23, 2025

CodSpeed Performance Report

Merging #31812 will not alter performance

Comparing feat/rolldown-vite (fbc982a) with main (b86a1c0)

Summary

✅ 10 untouched benchmarks

Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks great - though I wonder if it might be easier to ship as a standalone module which would make it easier to iterate upon things.

@TheAlexLichter
Copy link
Member Author

this looks great - though I wonder if it might be easier to ship as a standalone module which would make it easier to iterate upon things.

Fair point. That might be an option - though ideally not many changes should be necessary. The changes are even optional at the moment and mainly relevant when not using ROLLDOWN_OPTION_VALIDATION=loose.

If a module is the way, we should then make sure it is documented & the module runs for the rolldown-vite ecosystem CI as well.

@danielroe danielroe changed the title chore: test rolldown-vite feat(nuxt,vite): add experimental support for rolldown-vite Apr 26, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/vite/src/server.ts (1)

126-136: Proper handling of chunking options for Rollup/Rolldown compatibility

Good implementation of the conditional logic to handle different chunking behaviors between Rollup and Rolldown. One minor improvement would be to replace the delete operator with undefined assignments for better performance, as flagged by static analysis.

-      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] 130-130: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 135-135: 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

📥 Commits

Reviewing files that changed from the base of the PR and between 96deb43 and f6e7000.

📒 Files selected for processing (4)
  • packages/nuxt/build.config.ts (1 hunks)
  • packages/nuxt/src/components/plugins/islands-transform.ts (1 hunks)
  • packages/vite/src/server.ts (2 hunks)
  • packages/vite/src/vite.ts (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/nuxt/build.config.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/nuxt/src/components/plugins/islands-transform.ts
  • packages/vite/src/vite.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.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.
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.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.
🪛 Biome (1.9.4)
packages/vite/src/server.ts

[error] 130-130: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 135-135: 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-103: Correct conditional handling for Rolldown support

The conditional application of generatedCode options only when not using Rolldown is a sensible approach, given that these options aren't supported by Rolldown. The comment about the temporary fix for the Vue.js issue is helpful for future maintainers.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/vite/src/vite.ts (1)

101-102: Document rolldown-vite watch mode limitations

The TODO comment is correct but could be more informative about the current limitations.

Consider expanding the TODO comment to provide more context:

-                  // TODO: Find equivalent for rolldown-vite here
+                  // TODO: Watch mode for the `build` command is not implemented in rolldown-vite yet
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ed12cd and d620dc5.

📒 Files selected for processing (1)
  • packages/vite/src/vite.ts (1 hunks)
🧰 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.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.
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.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.
🪛 GitHub Check: build
packages/vite/src/vite.ts

[failure] 99-99:
Property 'rolldownVersion' does not exist on type 'typeof import("/home/runner/work/nuxt/nuxt/node_modules/.pnpm/vite@6.3.4_@types+node@22.15.3_jiti@2.4.2_terser@5.39.0_yaml@2.7.1/node_modules/vite/dist/node/index")'. Did you mean 'rollupVersion'?

🪛 GitHub Actions: ci
packages/vite/src/vite.ts

[error] 99-99: TypeScript error TS2551: Property 'rolldownVersion' does not exist on type 'typeof import(...)'. Did you mean 'rollupVersion'?

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: code

@lansi951
Copy link
Contributor

preserveModules is supported now.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🔭 Outside diff range comments (1)
packages/vite/src/server.ts (1)

93-136: 🛠️ Refactor suggestion

Document the Rolldown compatibility status and limitations

The code adds conditional handling for Rolldown compatibility, but lacks comprehensive documentation about the current state of Rolldown support and its limitations.

Add a descriptive comment at the top of this section to clarify:

  1. That this is experimental support for Rolldown
  2. What features are working vs. not working
  3. Any known issues or compatibility concerns

For example:

// Experimental Rolldown support
// Current limitations:
// - No support for generatedCode options
// - Different chunking mechanism (advancedChunks vs manualChunks)
// - See https://github.com/rolldown/rolldown/issues/206 for more details

This will help future maintainers understand the reasoning behind these conditional checks.

🧰 Tools
🪛 Biome (1.9.4)

[error] 130-130: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 135-135: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

🧹 Nitpick comments (3)
packages/vite/src/server.ts (3)

93-103: Improve type safety for Rolldown detection

The conditional handling for Rolldown vs. Rollup is implemented correctly, but the use of @ts-ignore creates technical debt. Since this is an experimental feature that will likely remain, it would be better to properly type the rolldownVersion property.

Consider creating proper type declarations for Rolldown-specific properties as suggested in previous reviews. This will provide better type safety and improve maintainability:

- // eslint-disable-next-line @typescript-eslint/ban-ts-comment
- // @ts-ignore Rolldown-Vite specific - https://github.com/rolldown/rolldown/issues/206
- ...vite.rolldownVersion
+ ...(vite as any).rolldownVersion

Or better yet, create a type declaration file to properly extend Vite's types.


128-136: Refactor delete operations to use undefined assignment

The use of the delete operator can impact runtime performance as flagged by static analysis. While the performance impact is likely minimal in this build-time code, it's still good practice to follow recommended patterns.

Consider using undefined assignment instead of delete operations:

- // If Rolldown is used, remove all chunk settings (mirror behavior for rollup for now)
- // eslint-disable-next-line @typescript-eslint/ban-ts-comment
- // @ts-ignore Rolldown-only option
- delete serverConfig.build.rollupOptions.output.advancedChunks
+ // If Rolldown is used, remove all chunk settings (mirror behavior for rollup for now)
+ // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+ // @ts-ignore Rolldown-only option
+ serverConfig.build.rollupOptions.output.advancedChunks = undefined

And similarly for the manualChunks deletion.

🧰 Tools
🪛 Biome (1.9.4)

[error] 130-130: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 135-135: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


93-97: Consider using a constant or utility function for Rolldown detection

The check for vite.rolldownVersion is used multiple times and might be needed in other files as well. This could lead to inconsistent detection or repetition.

Extract the Rolldown detection logic into a utility function:

+ /**
+  * Determines if the current Vite instance is using Rolldown
+  */
+ function isUsingRolldown(vite: any): boolean {
+   return !!vite.rolldownVersion
+ }

// Then use it in your conditional logic:
- ...vite.rolldownVersion
+ ...isUsingRolldown(vite)
-    ? {}
+    ? {}

This would centralise the detection logic, making it easier to maintain and adapt if the detection method changes in future.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7bcd410 and 50602a1.

📒 Files selected for processing (6)
  • packages/nuxt/build.config.ts (1 hunks)
  • packages/nuxt/src/components/plugins/islands-transform.ts (2 hunks)
  • packages/vite/src/plugins/analyze.ts (1 hunks)
  • packages/vite/src/plugins/ssr-styles.ts (1 hunks)
  • packages/vite/src/server.ts (2 hunks)
  • packages/vite/src/vite.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • packages/nuxt/build.config.ts
  • 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
🧠 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.
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.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.
🪛 Biome (1.9.4)
packages/vite/src/server.ts

[error] 130-130: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 135-135: 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 (4)
  • GitHub Check: build
  • GitHub Check: code
  • GitHub Check: codeql (javascript-typescript)
  • GitHub Check: lint-docs

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/vite/src/vite.ts (1)

58-65: Handle esbuild configuration for rolldown-vite

The code correctly removes esbuild configuration when rolldown-vite is detected, as esbuild options aren't needed in rolldown-vite environments. However, there's a performance concern with using the delete operator.

Consider using nullish assignment instead of delete for better performance:

if (vite.rolldownVersion) {
-  if (viteConfig.esbuild) {
-    delete viteConfig.esbuild
-  }
-  if (viteConfig.optimizeDeps?.esbuildOptions) {
-    delete viteConfig.optimizeDeps.esbuildOptions
-  }
+  if (viteConfig.esbuild) {
+    viteConfig.esbuild = undefined
+  }
+  if (viteConfig.optimizeDeps?.esbuildOptions) {
+    viteConfig.optimizeDeps.esbuildOptions = undefined
+  }
}
🧰 Tools
🪛 Biome (1.9.4)

[error] 60-60: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 63-63: 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

📥 Commits

Reviewing files that changed from the base of the PR and between 17786f1 and 255f967.

📒 Files selected for processing (1)
  • packages/vite/src/vite.ts (2 hunks)
🧰 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.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.
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.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.
🧬 Code Graph Analysis (1)
packages/vite/src/vite.ts (1)
packages/kit/src/ignore.ts (1)
  • isIgnored (13-31)
🪛 Biome (1.9.4)
packages/vite/src/vite.ts

[error] 60-60: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


[error] 63-63: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

🔇 Additional comments (1)
packages/vite/src/vite.ts (1)

108-117:

✅ Verification successful

Add conditional watch configuration for rolldown-vite

The code correctly adds a placeholder for rolldown-vite watch configuration while maintaining the original for standard Vite. The TypeScript error suppression is necessary as noted in the retrieved learning.

The TODO comment indicates that an equivalent for the watch configuration in rolldown-vite needs to be implemented. This aligns with the PR objectives which mention that "Watch mode for the build command is not implemented in rolldown-vite yet".


🌐 Web query:

What is the current status of watch mode support in rolldown-vite?

💡 Result:

Current Status of Watch Mode Support in rolldown-vite

Summary:
As of May 2025, watch mode support in rolldown-vite is not yet fully implemented. While the underlying Rolldown bundler has partial support for watch mode, this functionality has not been integrated into the rolldown-vite package itself.

Details:

  • Partial Implementation in Rolldown:
    Pull Request #84 on the rolldown-vite GitHub repository clearly states that watch mode is “partially implemented in rolldown, but not yet in rolldown-vite” as of April 21, 2025. Development towards watch mode is ongoing, but users of rolldown-vite do not have access to this feature yet[1].

  • Changelog and Releases:
    The changelog for recent beta releases of rolldown-vite (up to 6.3.0-beta.6, released April 10, 2025) does not mention completed watch mode support, nor new features related to file watching or rebuilds on change[3].

  • Official Vite Documentation:
    The Vite documentation regarding the Rolldown integration indicates that rolldown-vite is a temporary package for feedback and stabilization, with several features (such as Full Bundle Mode) planned for the future, but it does not mention that watch mode is currently available with rolldown-vite[2].

  • Contrast with Existing Vite Features:
    In stable Vite (without Rolldown), watch mode is supported with the vite build --watch CLI flag, leveraging underlying Rollup options[5][6][7]. However, this established watch mode is not currently available in the Rolldown-based experimental integration.

In Summary:
If you require watch mode (automatic rebuilds on file changes) with Vite, you must use the standard Vite setup with Rollup for now. rolldown-vite users will need to wait for further development before watch mode becomes available as part of the Rolldown integration[1][2][3].

Citations:


Approve placeholder for rolldown-vite watch configuration

Leaving the // TODO: Find equivalent for rolldown-vite here is correct: as of May 2025, rolldown-vite does not yet implement watch mode (PR #84). No further changes are needed until that feature is integrated.

• File: packages/vite/src/vite.ts
• Lines: 108–117

@danielroe
Copy link
Member

@TheAlexLichter do you think we can get this in the next minor?

@TheAlexLichter
Copy link
Member Author

Yes!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-ignore

Reuse 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 Plugin

The 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 config hook under build.rollupOptions.plugins to 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 plugins

Suppressing 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 255f967 and fbc982a.

📒 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.ts
  • packages/schema/src/config/common.ts
  • packages/vite/src/plugins/analyze.ts
  • packages/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.ts
  • packages/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-module

Spreading module as RenderedModule and overriding code aligns 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 usage

It appears the watchers.rolldown slot 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)
     },

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 detection

The 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.

📥 Commits

Reviewing files that changed from the base of the PR and between fbc982a and 1c38dc6.

📒 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/vite-builder@4.0.3 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

Comment on lines 218 to 231
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}))
}
})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/vite/build.config.ts (1)

20-21: Optional: also externalise 'rolldown' root to future‑proof accidental imports

If 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 1c38dc6 and e59c0cf.

📒 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 import

This 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')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

experimental rolldown support
7 participants