fix(vite-8): remove deprecated Rollup config options for Vite 8/Rolldown #548
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. | ||||||
| */ | ||||||
| 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. 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. 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
+569
to
+640
Contributor
There was a problem hiding this comment.
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", | ||||||
| // 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; | ||||||
|
Contributor
There was a problem hiding this comment. Bug: In the Vite 8 branch, this reads This should read from
Suggested change
(Checking both covers the transition period where users might still have |
||||||
| 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); | ||||||
| } | ||||||
| }; | ||||||
| })(), | ||||||
| }, | ||||||
| } as any, | ||||||
|
Contributor
There was a problem hiding this comment. The Consider a narrower cast, e.g.: build: {
["rolldownOptions" as string]: { ... },
}Or define a minimal type for the Vite 8 config shape. |
||||||
| } | ||||||
| : { | ||||||
| build: { | ||||||
| rollupOptions: { | ||||||
| ...getClientBuildOptions(viteMajorVersion).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: 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); | ||||||
| } | ||||||
| }; | ||||||
| })(), | ||||||
| }, | ||||||
| }, | ||||||
| }), | ||||||
|
Comment on lines
+1324
to
+1384
Contributor
There was a problem hiding this comment. The 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: This also makes the |
||||||
| // Let OPTIONS requests pass through Vite's CORS middleware to our | ||||||
| // route handlers so they can set the Allow header and run user-defined | ||||||
| // OPTIONS handlers. Without this, Vite's CORS middleware responds to | ||||||
|
|
@@ -1442,11 +1581,9 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |||||
| // on every page — defeating code-splitting for React.lazy() and | ||||||
| // next/dynamic boundaries. | ||||||
| ...(hasCloudflarePlugin ? { manifest: true } : {}), | ||||||
| ...getClientBuildOptionsWithInput(viteMajorVersion, { | ||||||
| index: VIRTUAL_APP_BROWSER_ENTRY, | ||||||
| }), | ||||||
| }, | ||||||
| }, | ||||||
| }; | ||||||
|
|
@@ -1461,11 +1598,9 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |||||
| build: { | ||||||
| manifest: true, | ||||||
| ssrManifest: true, | ||||||
| ...getClientBuildOptionsWithInput(viteMajorVersion, { | ||||||
| index: VIRTUAL_CLIENT_ENTRY, | ||||||
| }), | ||||||
| }, | ||||||
| }, | ||||||
| }; | ||||||
|
|
@@ -3752,7 +3887,16 @@ export type { | |||||
| export type { NextConfig } from "./config/next-config.js"; | ||||||
|
|
||||||
| // Exported for CLI and testing | ||||||
| export { | ||||||
| clientManualChunks, | ||||||
| clientOutputConfig, | ||||||
| clientTreeshakeConfig, | ||||||
| computeLazyChunks, | ||||||
| getClientBuildOptions, | ||||||
| getClientBuildOptionsWithInput, | ||||||
| getClientOutputConfig, | ||||||
| getClientTreeshakeConfig, | ||||||
|
Contributor
There was a problem hiding this comment. Good that both the old constants and the new getters are exported — preserves backwards compatibility for any external consumers importing /** @deprecated Use `getClientOutputConfig()` instead — applies version-gated config. */
const clientOutputConfig = { ... }; |
||||||
| }; | ||||||
| export { augmentSsrManifestFromBundle as _augmentSsrManifestFromBundle }; | ||||||
| export { resolvePostcssStringPlugins as _resolvePostcssStringPlugins }; | ||||||
| export { _postcssCache }; | ||||||
|
|
||||||
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.