-
Notifications
You must be signed in to change notification settings - Fork 325
fix(build): eliminate Vite 8 treeshake.preset warning #746
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
Merged
james-elicx
merged 4 commits into
cloudflare:main
from
Divkix:fix/vite8-treeshake-preset-warning
Apr 2, 2026
+79
β30
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
4b6c58a
fix(build): eliminate Vite 8 treeshake.preset warning
Divkix 63152c2
refactor(build): address PR review feedback for treeshake config
Divkix 90dd87f
docs(build): clarify Rolldown treeshake divergence in comments
Divkix 146475e
chore: address PR review feedback on treeshake config
Divkix File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -598,6 +598,9 @@ const clientCodeSplittingConfig = { | |
| * 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 getClientTreeshakeConfigForVite(viteMajorVersion) instead | ||
| * for Vite version compatibility. Kept for backward compatibility. | ||
| */ | ||
| const clientTreeshakeConfig = { | ||
| preset: "recommended" as const, | ||
|
|
@@ -634,6 +637,42 @@ function getClientOutputConfigForVite(viteMajorVersion: number) { | |
| : clientOutputConfig; | ||
| } | ||
|
|
||
| /** | ||
| * Returns treeshake configuration appropriate for the Vite version. | ||
| * | ||
| * Rollup (Vite 7) supports presets like "recommended" which set multiple | ||
| * treeshake options at once. Rolldown (Vite 8+) doesn't support presets, | ||
| * so we only return moduleSideEffects for Vite 8+. | ||
| * | ||
| * The Rollup "recommended" preset sets: | ||
| * - annotations: true (Rolldown default is also true) | ||
| * - manualPureFunctions: [] (Rolldown default is also []) | ||
| * - propertyReadSideEffects: true (Rolldown equivalent is 'always', the default) | ||
| * - unknownGlobalSideEffects: false (Rolldown default is true β this is a known acceptable | ||
| * divergence. Slightly less aggressive DCE on unknown globals, acceptable for client bundles) | ||
| * - correctVarValueBeforeDeclaration and tryCatchDeoptimization (Rolldown handles these differently) | ||
| * | ||
| * The key optimization is moduleSideEffects: "no-external", which is supported | ||
| * by both bundlers and provides the DCE benefits for barrel-exporting libraries. | ||
| * It treats node_modules as side-effect-free (enabling aggressive DCE) while | ||
| * preserving side effects in local code. | ||
| */ | ||
| function getClientTreeshakeConfigForVite(viteMajorVersion: number) { | ||
| if (viteMajorVersion >= 8) { | ||
| // Rolldown (Vite 8+) - no preset support, only specific options. | ||
| // Rolldown's built-in defaults already cover what Rollup's 'recommended' | ||
| // preset provides (annotations, correctContext, tryCatchDeoptimization). | ||
| return { | ||
| moduleSideEffects: "no-external" as const, | ||
| }; | ||
| } | ||
| // Rollup (Vite 7) - supports presets for convenient option grouping | ||
| return { | ||
| preset: "recommended" as const, | ||
| moduleSideEffects: "no-external" as const, | ||
| }; | ||
| } | ||
|
|
||
| type BundleBackfillChunk = { | ||
| type: "chunk"; | ||
| fileName: string; | ||
|
|
@@ -1441,14 +1480,16 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |
| }; | ||
| })(), | ||
| // Enable aggressive tree-shaking for client builds. | ||
| // See clientTreeshakeConfig for rationale. | ||
| // See getClientTreeshakeConfigForVite JSDoc 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 } : {}), | ||
| ...(!isSSR && !isMultiEnv | ||
| ? { treeshake: getClientTreeshakeConfigForVite(viteMajorVersion) } | ||
|
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. Not part of this diff, but the comment at line 1483 still references the deprecated |
||
| : {}), | ||
| // 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. | ||
|
|
@@ -1696,7 +1737,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |
| ...withBuildBundlerOptions(viteMajorVersion, { | ||
| input: { index: VIRTUAL_APP_BROWSER_ENTRY }, | ||
| output: getClientOutputConfigForVite(viteMajorVersion), | ||
| treeshake: clientTreeshakeConfig, | ||
| treeshake: getClientTreeshakeConfigForVite(viteMajorVersion), | ||
| }), | ||
| }, | ||
| }, | ||
|
|
@@ -1717,7 +1758,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |
| ...withBuildBundlerOptions(viteMajorVersion, { | ||
| input: { index: VIRTUAL_CLIENT_ENTRY }, | ||
| output: getClientOutputConfigForVite(viteMajorVersion), | ||
| treeshake: clientTreeshakeConfig, | ||
| treeshake: getClientTreeshakeConfigForVite(viteMajorVersion), | ||
| }), | ||
| }, | ||
| }, | ||
|
|
@@ -1743,7 +1784,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { | |
| ...withBuildBundlerOptions(viteMajorVersion, { | ||
| input: { index: VIRTUAL_CLIENT_ENTRY }, | ||
| output: getClientOutputConfigForVite(viteMajorVersion), | ||
| treeshake: clientTreeshakeConfig, | ||
| treeshake: getClientTreeshakeConfigForVite(viteMajorVersion), | ||
| }), | ||
| }, | ||
| }, | ||
|
|
@@ -4084,6 +4125,7 @@ export { | |
| clientTreeshakeConfig, | ||
| computeLazyChunks, | ||
| getClientOutputConfigForVite, | ||
| getClientTreeshakeConfigForVite, | ||
| }; | ||
| export { augmentSsrManifestFromBundle as _augmentSsrManifestFromBundle }; | ||
| export { resolvePostcssStringPlugins as _resolvePostcssStringPlugins }; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 Rollup
recommendedpreset sets more than justmoduleSideEffectsβ it also enablesannotations: true,correctContext: true,tryCatchDeoptimization: true, etc. Dropping these for Vite 8+ is probably fine if Rolldown's defaults already cover them, but worth confirming and documenting here. A one-liner like:would help future readers understand this isn't an accidental loss of optimizations.