Conversation
📝 WalkthroughWalkthroughThis PR adds runtime detection for Vite version compatibility, supporting both Vite 7 (rollupOptions) and Vite 8 (rolldownOptions) through utility functions that abstract bundler option access based on the detected bundler. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit a65903e
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview1 package(s) bumped directly, 3 bumped as dependents. 🟨 Minor bumps
🟩 Patch bumps
|
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
![coderabbitai[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/start-plugin-core/src/utils.ts`:
- Around line 15-16: Replace the loose any usage in getBundlerOptions by adding
precise types for the build parameter and return (e.g., RollupOptions |
RolldownOptions) and explicitly pick the correct option based on a detected
bundler flag instead of relying on the nullish coalescing; inside
getBundlerOptions detect whether to use rolldown by checking a bundler indicator
(isRolldown) and return build.rolldownOptions when isRolldown is true otherwise
return build.rollupOptions, updating the function signature and imports/types
accordingly to eliminate all any occurrences (reference: getBundlerOptions,
build.rolldownOptions, build.rollupOptions, isRolldown).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ddaf4d44-b4a4-422a-9920-a7bc915b8ab7
📒 Files selected for processing (4)
.changeset/rolldownoptions-compat.mdpackages/start-plugin-core/src/plugin.tspackages/start-plugin-core/src/preview-server-plugin/plugin.tspackages/start-plugin-core/src/utils.ts
| export function getBundlerOptions(build: any): any { | ||
| return build?.rolldownOptions ?? build?.rollupOptions |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Vite 7 and Vite 8, when both build.rollupOptionsandbuild.rolldownOptionsare provided, which one is actually used by the active bundler, and isrolldownVersionon thevite module an official runtime discriminator?
💡 Result:
-
Vite 7 (regular
vite): the active bundler is Rollup, so onlybuild.rollupOptionsis used. (There is nobuild.rolldownOptionsconfig API in Vite 7 docs; Rolldown support lives in the separaterolldown-vitepackage.)[3] -
Vite 8: the active bundler is Rolldown.[1] The config field
build.rollupOptionsis deprecated and is just an alias ofbuild.rolldownOptions.[2] So the active bundler effectively usesbuild.rolldownOptions;build.rollupOptionsdoes not represent a separate, independently-applied set of options in Vite 8.[2] -
Is
vite.rolldownVersionan official runtime discriminator? Yes—Vite’s official Rolldown integration guide explicitly documents checking the existence of therolldownVersionexport (and/orthis.meta.rolldownVersion) to detect when running underrolldown-vite.[3]
Sources: [1] [2] [3]
🏁 Script executed:
# First, let's see the full context of the file around lines 15-16
cat -n packages/start-plugin-core/src/utils.ts | head -30Repository: TanStack/router
Length of output: 1309
🏁 Script executed:
# Also check what imports are available
head -20 packages/start-plugin-core/src/utils.tsRepository: TanStack/router
Length of output: 716
🏁 Script executed:
# Search for isRolldown definition and usage
rg "isRolldown" packages/start-plugin-core/src/Repository: TanStack/router
Length of output: 234
🏁 Script executed:
# Check if vite types are available in the project
rg "from.*vite" packages/start-plugin-core/src/ | head -10Repository: TanStack/router
Length of output: 1180
Remove any types and bind getBundlerOptions to detected bundler.
The function currently uses loose any types, violating the TypeScript strict mode requirement. Additionally, while the ?? logic happens to work (Vite 7 lacks rolldownOptions, Vite 8 treats rollupOptions as an alias), explicitly binding to isRolldown improves clarity and maintainability.
Proposed fix
+type BuildWithBundlerOptions = {
+ rollupOptions?: vite.RollupOptions
+ rolldownOptions?: vite.RollupOptions
+}
+
/** Read `build.rollupOptions` or `build.rolldownOptions` from a build config. */
-export function getBundlerOptions(build: any): any {
- return build?.rolldownOptions ?? build?.rollupOptions
+export function getBundlerOptions(
+ build?: BuildWithBundlerOptions,
+): vite.RollupOptions | undefined {
+ return isRolldown ? build?.rolldownOptions : build?.rollupOptions
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/start-plugin-core/src/utils.ts` around lines 15 - 16, Replace the
loose any usage in getBundlerOptions by adding precise types for the build
parameter and return (e.g., RollupOptions | RolldownOptions) and explicitly pick
the correct option based on a detected bundler flag instead of relying on the
nullish coalescing; inside getBundlerOptions detect whether to use rolldown by
checking a bundler indicator (isRolldown) and return build.rolldownOptions when
isRolldown is true otherwise return build.rollupOptions, updating the function
signature and imports/types accordingly to eliminate all any occurrences
(reference: getBundlerOptions, build.rolldownOptions, build.rollupOptions,
isRolldown).
Merging this PR will not alter performance
Comparing 
Footnotes
|
birkskyum
changed the title
birkskyum
changed the title
birkskyum
changed the title
birkskyum
changed the title
rollupOptions) and Vite 8 (rolldownOptions)
|
I think this PR might have broken the SSR benchmarks. They are marked as skipped here
Looking at the commit history, this PR's merge commit seems to be the 1st with skipped benchmarks. They seem to fail during the build Maybe an issue with the rolldown detection? GPT tracked it down to this #6984. BTW: if it fails on the benchmarks, there is a chance it fails in real apps too, right? |
2 participants
By runtime checking if rollup or rolldown is used, the plugin can now read/write to rollupOptions or rolldownOptions, thus support both vite 7 and 8
Summary by CodeRabbit