-
Notifications
You must be signed in to change notification settings - Fork 293
fix(vite-8): remove deprecated Rollup config options for Vite 8/Rolldown #548
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
3fb524c
109ac93
6beaa47
1e6744e
029d279
6b9d5ef
45cd355
719778e
bec552d
89c24ea
f0e3905
701a42d
10bd03c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -241,7 +241,7 @@ function extractStaticValue(node: any): unknown { | |||||
| * by a project that has Vite 8 — so we resolve from cwd, not from | ||||||
| * the plugin's own location. | ||||||
| */ | ||||||
| function getViteMajorVersion(): number { | ||||||
| export function getViteMajorVersion(): number { | ||||||
| try { | ||||||
| const require = createRequire(path.join(process.cwd(), "package.json")); | ||||||
| const vitePkg = require("vite/package.json"); | ||||||
|
|
@@ -489,6 +489,8 @@ function clientManualChunks(id: string): string | undefined { | |||||
| * their importers. This reduces HTTP request count and improves gzip | ||||||
| * compression efficiency — small files restart the compression dictionary, | ||||||
| * adding ~5-15% wire overhead vs fewer larger chunks. | ||||||
| * | ||||||
| * @deprecated Use `getClientOutputConfig()` instead — applies version-gated config. | ||||||
| */ | ||||||
| const clientOutputConfig = { | ||||||
| manualChunks: clientManualChunks, | ||||||
|
|
@@ -519,12 +521,124 @@ const clientOutputConfig = { | |||||
| * tryCatchDeoptimization: false, which can break specific libraries | ||||||
| * that rely on property access side effects or try/catch for feature detection | ||||||
| * - 'recommended' + 'no-external' gives most of the benefit with less risk | ||||||
| * | ||||||
| * @deprecated Use `getClientTreeshakeConfig()` instead — applies version-gated config. | ||||||
| */ | ||||||
| const clientTreeshakeConfig = { | ||||||
| preset: "recommended" as const, | ||||||
| moduleSideEffects: "no-external" as const, | ||||||
| }; | ||||||
|
|
||||||
| /** | ||||||
| * Get Rollup-compatible output config for client builds. | ||||||
| * Returns config without Vite 8/Rolldown-incompatible options. | ||||||
| */ | ||||||
| function getClientOutputConfig(viteVersion: number): { | ||||||
| manualChunks: typeof clientManualChunks; | ||||||
| experimentalMinChunkSize?: number; | ||||||
| } { | ||||||
| if (viteVersion >= 8) { | ||||||
| // Vite 8+ uses Rolldown which doesn't support experimentalMinChunkSize | ||||||
| return { | ||||||
| manualChunks: clientManualChunks, | ||||||
| }; | ||||||
| } | ||||||
| // Vite 7 uses Rollup with experimentalMinChunkSize support | ||||||
| return clientOutputConfig; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Get Rollup-compatible treeshake config for client builds. | ||||||
| * Returns config without Vite 8/Rolldown-incompatible options. | ||||||
| */ | ||||||
| function getClientTreeshakeConfig(viteVersion: number): { | ||||||
| preset?: "recommended"; | ||||||
| moduleSideEffects: "no-external"; | ||||||
| } { | ||||||
| if (viteVersion >= 8) { | ||||||
| // Vite 8+ uses Rolldown which doesn't support `preset` option | ||||||
| // moduleSideEffects is still supported in Rolldown | ||||||
| return { | ||||||
| moduleSideEffects: "no-external" as const, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the critical question for the whole PR: does Vite 8 / Rolldown actually respect The issue (#540) and the Vite 8 migration docs both indicate that Rolldown treeshake config should go under Could you verify this with an actual Vite 8 build? If
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment says "moduleSideEffects is still supported in Rolldown" — has this been verified? The previous bot reviews raised the same concern. If Rolldown ignores The remaining question is whether Rolldown actually supports the |
||||||
| }; | ||||||
| } | ||||||
| // Vite 7 uses Rollup with preset support | ||||||
| return clientTreeshakeConfig; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Get build options config for client builds, version-gated for Vite 8/Rolldown. | ||||||
| * Vite 7 uses build.rollupOptions, Vite 8+ uses build.rolldownOptions. | ||||||
| */ | ||||||
| function getClientBuildOptions(viteVersion: number): { | ||||||
| rollupOptions?: { | ||||||
| input?: Record<string, string>; | ||||||
| output: { manualChunks: typeof clientManualChunks; experimentalMinChunkSize?: number }; | ||||||
| treeshake: { preset?: "recommended"; moduleSideEffects: "no-external" }; | ||||||
| }; | ||||||
| rolldownOptions?: { | ||||||
| input?: Record<string, string>; | ||||||
| output: { manualChunks: typeof clientManualChunks }; | ||||||
| treeshake: { moduleSideEffects: "no-external" }; | ||||||
| }; | ||||||
| } { | ||||||
| if (viteVersion >= 8) { | ||||||
| // Vite 8+ uses Rolldown - config goes under rolldownOptions | ||||||
| return { | ||||||
| rolldownOptions: { | ||||||
| output: getClientOutputConfig(viteVersion), | ||||||
| treeshake: getClientTreeshakeConfig(viteVersion), | ||||||
| }, | ||||||
| }; | ||||||
| } | ||||||
| // Vite 7 uses Rollup - config goes under rollupOptions | ||||||
| return { | ||||||
| rollupOptions: { | ||||||
| output: getClientOutputConfig(viteVersion), | ||||||
| treeshake: getClientTreeshakeConfig(viteVersion), | ||||||
| }, | ||||||
| }; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Get build options config for client builds with custom input, version-gated for Vite 8/Rolldown. | ||||||
| * Vite 7 uses build.rollupOptions, Vite 8+ uses build.rolldownOptions. | ||||||
| */ | ||||||
| function getClientBuildOptionsWithInput( | ||||||
| viteVersion: number, | ||||||
| input: Record<string, string>, | ||||||
| ): { | ||||||
| rollupOptions?: { | ||||||
| input: Record<string, string>; | ||||||
| output: { manualChunks: typeof clientManualChunks; experimentalMinChunkSize?: number }; | ||||||
| treeshake: { preset?: "recommended"; moduleSideEffects: "no-external" }; | ||||||
| }; | ||||||
| rolldownOptions?: { | ||||||
| input: Record<string, string>; | ||||||
| output: { manualChunks: typeof clientManualChunks }; | ||||||
| treeshake: { moduleSideEffects: "no-external" }; | ||||||
| }; | ||||||
| } { | ||||||
| if (viteVersion >= 8) { | ||||||
| // Vite 8+ uses Rolldown - config goes under rolldownOptions | ||||||
| return { | ||||||
| rolldownOptions: { | ||||||
| input, | ||||||
| output: getClientOutputConfig(viteVersion), | ||||||
| treeshake: getClientTreeshakeConfig(viteVersion), | ||||||
| }, | ||||||
| }; | ||||||
| } | ||||||
| // Vite 7 uses Rollup - config goes under rollupOptions | ||||||
| return { | ||||||
| rollupOptions: { | ||||||
| input, | ||||||
| output: getClientOutputConfig(viteVersion), | ||||||
| treeshake: getClientTreeshakeConfig(viteVersion), | ||||||
| }, | ||||||
| }; | ||||||
| } | ||||||
|
Comment on lines
+648
to
+680
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
function getClientBuildOptions(
viteVersion: number,
input?: Record<string, string>,
) { ... }This halves the type signature and branching duplication, and eliminates a whole exported function. |
||||||
|
|
||||||
| type BuildManifestChunk = { | ||||||
| file: string; | ||||||
| isEntry?: boolean; | ||||||
|
|
@@ -1199,50 +1313,75 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |||||
| const viteConfig: UserConfig = { | ||||||
| // Disable Vite's default HTML serving - we handle all routing | ||||||
| appType: "custom", | ||||||
| build: { | ||||||
| rollupOptions: { | ||||||
| // Suppress "Module level directives cause errors when bundled" | ||||||
| // warnings for "use client" / "use server" directives. Our shims | ||||||
| // and third-party libraries legitimately use these directives; | ||||||
| // they are handled by the RSC plugin and are harmless in the | ||||||
| // final bundle. We preserve any user-supplied onwarn so custom | ||||||
| // warning handling is not lost. | ||||||
| onwarn: (() => { | ||||||
| const userOnwarn = config.build?.rollupOptions?.onwarn; | ||||||
| return (warning, defaultHandler) => { | ||||||
| if ( | ||||||
| warning.code === "MODULE_LEVEL_DIRECTIVE" && | ||||||
| (warning.message?.includes('"use client"') || | ||||||
| warning.message?.includes('"use server"')) | ||||||
| ) { | ||||||
| return; | ||||||
| } | ||||||
| if (userOnwarn) { | ||||||
| userOnwarn(warning, defaultHandler); | ||||||
| } else { | ||||||
| defaultHandler(warning); | ||||||
| } | ||||||
| }; | ||||||
| })(), | ||||||
| // Enable aggressive tree-shaking for client builds. | ||||||
| // See clientTreeshakeConfig for rationale. | ||||||
| // Only apply globally for standalone client builds (Pages Router | ||||||
| // CLI). For multi-environment builds (App Router, Cloudflare), | ||||||
| // treeshake is set per-environment on the client env below to | ||||||
| // avoid leaking into RSC/SSR environments where | ||||||
| // moduleSideEffects: 'no-external' could drop server packages | ||||||
| // that rely on module-level side effects. | ||||||
| ...(!isSSR && !isMultiEnv ? { treeshake: clientTreeshakeConfig } : {}), | ||||||
| // Code-split client bundles: separate framework (React/ReactDOM), | ||||||
| // vinext runtime (shims), and vendor packages into their own | ||||||
| // chunks so pages only load the JS they need. | ||||||
| // Only apply globally for standalone client builds (CLI Pages | ||||||
| // Router). For multi-environment builds (App Router, Cloudflare), | ||||||
| // manualChunks is set per-environment on the client env below | ||||||
| // to avoid leaking into RSC/SSR environments. | ||||||
| ...(!isSSR && !isMultiEnv ? { output: clientOutputConfig } : {}), | ||||||
| }, | ||||||
| }, | ||||||
| // For standalone client builds (Pages Router CLI), apply version-gated | ||||||
| // rollup/rolldown options. Vite 7 uses rollupOptions, Vite 8+ uses rolldownOptions. | ||||||
| // Multi-environment builds (App Router, Cloudflare) set these per-environment | ||||||
| // on the client env below to avoid leaking into RSC/SSR environments. | ||||||
| ...(isSSR || isMultiEnv | ||||||
| ? { | ||||||
| build: {}, | ||||||
| } | ||||||
| : viteMajorVersion >= 8 | ||||||
| ? { | ||||||
| build: { | ||||||
| rolldownOptions: { | ||||||
| ...getClientBuildOptions(viteMajorVersion).rolldownOptions, | ||||||
| // Suppress "Module level directives cause errors when bundled" | ||||||
| // warnings for "use client" / "use server" directives. Our shims | ||||||
| // and third-party libraries legitimately use these directives; | ||||||
| // they are handled by the RSC plugin and are harmless in the | ||||||
| // final bundle. We preserve any user-supplied onwarn so custom | ||||||
| // warning handling is not lost. | ||||||
| onwarn: (() => { | ||||||
| const userOnwarn = config.build?.rollupOptions?.onwarn; | ||||||
|
||||||
| const userOnwarn = config.build?.rollupOptions?.onwarn; | |
| const userOnwarn = config.build?.rolldownOptions?.onwarn ?? config.build?.rollupOptions?.onwarn; |
(Checking both covers the transition period where users might still have rollupOptions config.)
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The as any cast here disables all type checking for the entire Vite 8 build config object. This means if you misspell a property name or pass the wrong shape, the compiler won't catch it.
Consider a narrower cast, e.g.:
build: {
["rolldownOptions" as string]: { ... },
}Or define a minimal type for the Vite 8 config shape.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The onwarn factory is duplicated verbatim across the Vite 8 and Vite 7 branches (15 lines each). Extract it into a helper:
function createDirectiveOnwarn(userOnwarn?: (warning: any, handler: (w: any) => void) => void) {
return (warning: any, defaultHandler: (warning: any) => void) => {
if (
warning.code === "MODULE_LEVEL_DIRECTIVE" &&
(warning.message?.includes('"use client"') ||
warning.message?.includes('"use server"'))
) {
return;
}
if (userOnwarn) {
userOnwarn(warning, defaultHandler);
} else {
defaultHandler(warning);
}
};
}Then both branches become a single line: onwarn: createDirectiveOnwarn(config.build?.rollupOptions?.onwarn)
This also makes the onwarn source (rollupOptions vs rolldownOptions) easier to vary per branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good that both the old constants and the new getters are exported — preserves backwards compatibility for any external consumers importing clientOutputConfig / clientTreeshakeConfig directly. Worth adding a brief @deprecated JSDoc on the old constants so consumers know to migrate to the getters:
/** @deprecated Use `getClientOutputConfig()` instead — applies version-gated config. */
const clientOutputConfig = { ... };
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same feedback as
getClientOutputConfig— the Vite 8 branch stripspresetand returns{ moduleSideEffects: "no-external" }underrollupOptions.treeshake. However, the issue (#540) and Vite 8 migration docs suggest that Rolldown's treeshake config should be underbuild.rolldownOptions.treeshake, notbuild.rollupOptions.treeshake. If Vite 8/Rolldown ignoresrollupOptions.treeshakeentirely, thenmoduleSideEffects: "no-external"has no effect and the optimization is silently lost.Please verify that
rollupOptions.treeshake.moduleSideEffectsis actually respected by Vite 8/Rolldown. If not, the Vite 8 path may need to return the config under a different key, or the caller sites need to conditionally place it underrolldownOptionsinstead.