Skip to content

Conversation

@huang-julien
Copy link
Member

@huang-julien huang-julien commented Oct 30, 2025

🔗 Linked issue

#33602

resolved by rolldown/rolldown#6782

📚 Description

This probably isn't wanted as the final fix should be done in rolldown-vite. It's temporary

@bolt-new-by-stackblitz
Copy link

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

@coderabbitai
Copy link

coderabbitai bot commented Oct 30, 2025

Walkthrough

The change introduces a production environment guard to the rolldown plugin selection logic in the replace plugin module. Previously, rolldown would be conditionally used based on its availability check. Now, the plugin additionally verifies that config.isProduction is true before selecting the rolldown executable plugin. In non-production configurations, the code falls back to the standard replacePlugin implementation. The modification does not alter error handling or other existing logic paths.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single-file change with straightforward conditional logic addition
  • Simple production environment guard requirement
  • Verify the conditional logic correctly distinguishes between production and non-production environments
  • Confirm the fallback to replacePlugin operates as intended when the production guard is false

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ❓ Inconclusive The pull request description provided by the author is minimal and lacks clarity about the actual changes being made. While the description references the linked issue (#33602) and acknowledges this is a temporary workaround intended for rolldown-vite, it does not explicitly describe what problem is being solved or what the changeset accomplishes. The statement "This probably isn't wanted as the final fix should be done in rolldown-vite. It's temporary" is vague and uses non-descriptive language that does not clearly convey the nature or purpose of the modifications to the codebase. The description is related to the pull request's context but lacks the specificity needed to understand the changes independently. The author should enhance the pull request description to explicitly outline the problem being addressed and explain what the production guard changes accomplish. A clearer description might state: "Restricts the use of the rolldown replace plugin to production builds only, preventing it from being used during development. This is a temporary workaround pending the permanent fix in rolldown-vite." This would provide readers with a better understanding of the changes without requiring them to examine the code or linked issue.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "fix(vite): use rolldown replace only in build" directly and accurately describes the main change documented in the raw summary. The change adds a production guard to restrict rolldown usage to build time only (when config.isProduction is true), and the title captures this core intent concisely. The title follows conventional commits format and is specific enough that a teammate scanning the history would understand the primary purpose of the changeset without ambiguity.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/rollown_replace

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 30, 2025

Open in StackBlitz

@nuxt/kit

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

nuxt

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

@nuxt/rspack-builder

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

@nuxt/schema

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

@nuxt/vite-builder

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

@nuxt/webpack-builder

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

commit: bd81bcd

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 30, 2025

CodSpeed Performance Report

Merging #33615 will improve performances by 11.36%

Comparing fix/rollown_replace (bd81bcd) with main (2a08781)1

Summary

⚡ 1 improvement
✅ 9 untouched

Benchmarks breakdown

Benchmark BASE HEAD Change
writeTypes in the basic-types fixture 89.1 ms 80 ms +11.36%

Footnotes

  1. No successful run was found on main (96c6605) during the generation of this report, so 2a08781 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@huang-julien
Copy link
Member Author

@TheAlexLichter This PR can be closed now since rolldown/rolldown#6782 has been merged ?

@TheAlexLichter
Copy link
Member

@huang-julien I don't think so!

  1. It still needs a release
  2. In dev, the rolldown plugin is still not recommended (see fix(plugins): wrap replacePlugin with makeBuiltinPluginCallable rolldown/rolldown#6782 (comment))

@huang-julien
Copy link
Member Author

The plugins listed in https://rolldown.rs/builtin-plugins/ are the only ones expected to be used by external projects

From my understanding, builtin:replace is allowed to be used by external projects since it is listed. It's a bit confusing.

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.

4 participants