From 40c1c6cfa4fbebdff188a996fd506b09d7233e53 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Wed, 16 Jun 2021 07:22:25 +0200 Subject: [PATCH] Add option to deoptimize var declarations for tree-shaking (#4139) * Reference types when normalizing options * Add string presets for the treeshake option * Add "recommended" option and warn for unknown options * Add ability to use presets with overrides * Add CLI support for presets * Add documentation * Fix docs * Reference types when normalizing options * Add treeshake.correctVarValueBeforeDeclaration option * Add documentation --- docs/999-big-list-of-options.md | 30 +++++++-- src/ast/nodes/Identifier.ts | 10 ++- src/rollup/types.d.ts | 2 + src/utils/options/normalizeInputOptions.ts | 61 +++++++++---------- src/utils/options/options.ts | 3 + .../samples/treeshake-preset-override/main.js | 3 + .../preset-with-override/_config.js | 1 + .../preset-with-override/main.js | 3 + .../treeshake-presets/recommended/_config.js | 1 + .../treeshake-presets/recommended/main.js | 3 + .../treeshake-presets/safest/_config.js | 1 + .../treeshake-presets/safest/_expected.js | 3 + .../samples/treeshake-presets/safest/main.js | 3 + .../treeshake-presets/smallest/_config.js | 1 + .../treeshake-presets/smallest/main.js | 3 + .../samples/treeshake-presets/true/_config.js | 1 + .../samples/treeshake-presets/true/main.js | 3 + 17 files changed, 94 insertions(+), 38 deletions(-) diff --git a/docs/999-big-list-of-options.md b/docs/999-big-list-of-options.md index a568fdd6559..9db70067512 100755 --- a/docs/999-big-list-of-options.md +++ b/docs/999-big-list-of-options.md @@ -1379,7 +1379,7 @@ Default: `false` If this option is provided, bundling will not fail if bindings are imported from a file that does not define these bindings. Instead, new variables will be created for these bindings with the value `undefined`. #### treeshake -Type: `boolean | "smallest" | "safest" | "recommended" | { annotations?: boolean, moduleSideEffects?: ModuleSideEffectsOption, preset?: "smallest" | "safest" | "recommended", propertyReadSideEffects?: boolean | 'always', tryCatchDeoptimization?: boolean, unknownGlobalSideEffects?: boolean }`
+Type: `boolean | "smallest" | "safest" | "recommended" | { annotations?: boolean, correctVarValueBeforeDeclaration?: boolean, moduleSideEffects?: ModuleSideEffectsOption, preset?: "smallest" | "safest" | "recommended", propertyReadSideEffects?: boolean | 'always', tryCatchDeoptimization?: boolean, unknownGlobalSideEffects?: boolean }`
CLI: `--treeshake`/`--no-treeshake`
Default: `true` @@ -1389,9 +1389,10 @@ Whether to apply tree-shaking and to fine-tune the tree-shaking process. Setting * getters with side effects will only be retained if the return value is used (`treeshake.propertyReadSideEffects: false`) * code from imported modules will only be retained if at least one exported value is used (`treeshake.moduleSideEffects: false`) * you should not bundle polyfills that rely on detecting broken builtins (`treeshake.tryCatchDeoptimization: false`) - * some semantic errors may be swallowed (`treeshake.unknownGlobalSideEffects: false`) -* `"recommended"` should work well for most usage patterns. Some semantic errors may be swallowed, though (`treeshake.unknownGlobalSideEffects: false`) -* `"safest"` tries to be as spec compliant as possible while still providing some basic tree-shaking capabilities. This is currently equivalent to `true` but this may change in the next major version. + * some semantic issues may be swallowed (`treeshake.unknownGlobalSideEffects: false`, `treeshake.correctVarValueBeforeDeclaration: false`) +* `"recommended"` should work well for most usage patterns. Some semantic issues may be swallowed, though (`treeshake.unknownGlobalSideEffects: false`, `treeshake.correctVarValueBeforeDeclaration: false`) +* `"safest"` tries to be as spec compliant as possible while still providing some basic tree-shaking capabilities. +* `true` is equivalent to not specifying the option and will always choose the default value (see below). If you discover a bug caused by the tree-shaking algorithm, please file an issue! Setting this option to an object implies tree-shaking is enabled and grants the following additional options: @@ -1415,6 +1416,27 @@ class Impure { /*@__PURE__*/new Impure(); ``` +**treeshake.correctVarValueBeforeDeclaration**
+Type: `boolean`
+CLI: `--treeshake.correctVarValueBeforeDeclaration`/`--no-treeshake.correctVarValueBeforeDeclaration`
+Default: `false` + +If a variable is assigned a value in its declaration and is never reassigned, Rollup assumes the value to be constant. This is not true if the variable is declared with `var`, however, as those variables can be accessed before their declaration where they will evaluate to `undefined`. +Choosing `true` will make sure Rollup does not make (wrong) assumptions about the value of such variables. Note though that this can have a noticeable negative impact on tree-shaking results. + +```js +// input +if (x) console.log('not executed'); +var x = true; + +// output with treeshake.correctVarValueBeforeDeclaration === false +console.log('not executed'); + +// output with treeshake.correctVarValueBeforeDeclaration === true +if (x) console.log('not executed'); +var x = true; +``` + **treeshake.moduleSideEffects**
Type: `boolean | "no-external" | string[] | (id: string, external: boolean) => boolean`
CLI: `--treeshake.moduleSideEffects`/`--no-treeshake.moduleSideEffects`/`--treeshake.moduleSideEffects no-external`
diff --git a/src/ast/nodes/Identifier.ts b/src/ast/nodes/Identifier.ts index 97a4f560092..d21db0dd645 100644 --- a/src/ast/nodes/Identifier.ts +++ b/src/ast/nodes/Identifier.ts @@ -14,7 +14,7 @@ import LocalVariable from '../variables/LocalVariable'; import Variable from '../variables/Variable'; import * as NodeType from './NodeType'; import SpreadElement from './SpreadElement'; -import { ExpressionEntity, LiteralValueOrUnknown } from './shared/Expression'; +import { ExpressionEntity, LiteralValueOrUnknown, UNKNOWN_EXPRESSION } from './shared/Expression'; import { ExpressionNode, NodeBase } from './shared/Node'; import { PatternNode } from './shared/Pattern'; @@ -45,9 +45,15 @@ export default class Identifier extends NodeBase implements PatternNode { declare(kind: string, init: ExpressionEntity): LocalVariable[] { let variable: LocalVariable; + const { treeshake } = this.context.options; switch (kind) { case 'var': - variable = this.scope.addDeclaration(this, this.context, init, true); + variable = this.scope.addDeclaration( + this, + this.context, + treeshake && treeshake.correctVarValueBeforeDeclaration ? UNKNOWN_EXPRESSION : init, + true + ); break; case 'function': // in strict mode, functions are only hoisted within a scope but not across block scopes diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 8c3a17c3e05..a9a146a791a 100644 --- a/src/rollup/types.d.ts +++ b/src/rollup/types.d.ts @@ -478,6 +478,7 @@ type TreeshakingPreset = 'smallest' | 'safest' | 'recommended'; export interface TreeshakingOptions { annotations?: boolean; + correctVarValueBeforeDeclaration?: boolean; moduleSideEffects?: ModuleSideEffectsOption; preset?: TreeshakingPreset; propertyReadSideEffects?: boolean | 'always'; @@ -489,6 +490,7 @@ export interface TreeshakingOptions { export interface NormalizedTreeshakingOptions { annotations: boolean; + correctVarValueBeforeDeclaration: boolean; moduleSideEffects: HasModuleSideEffects; propertyReadSideEffects: boolean | 'always'; tryCatchDeoptimization: boolean; diff --git a/src/utils/options/normalizeInputOptions.ts b/src/utils/options/normalizeInputOptions.ts index 7fdc96de610..a1aa3ff9e64 100644 --- a/src/utils/options/normalizeInputOptions.ts +++ b/src/utils/options/normalizeInputOptions.ts @@ -7,6 +7,7 @@ import { PreserveEntrySignaturesOption, PureModulesOption, RollupBuild, + TreeshakingOptions, WarningHandler } from '../../rollup/types'; import { ensureArray } from '../ensureArray'; @@ -234,15 +235,21 @@ const getTreeshake = ( if (configTreeshake === false) { return false; } - if (!configTreeshake || configTreeshake === true) { - return { - annotations: true, - moduleSideEffects: () => true, - propertyReadSideEffects: true, - tryCatchDeoptimization: true, - unknownGlobalSideEffects: true - }; + if (typeof configTreeshake === 'string') { + const preset = treeshakePresets[configTreeshake]; + if (preset) { + return preset; + } + error( + errInvalidOption( + 'treeshake', + `valid values are false, true, ${printQuotedStringList( + Object.keys(treeshakePresets) + )}. You can also supply an object for more fine-grained control` + ) + ); } + let configWithPreset: TreeshakingOptions = {}; if (typeof configTreeshake === 'object') { if (typeof configTreeshake.pureExternalModules !== 'undefined') { warnDeprecationWithOptions( @@ -252,7 +259,7 @@ const getTreeshake = ( strictDeprecations ); } - let configWithPreset = configTreeshake; + configWithPreset = configTreeshake; const presetName = configTreeshake.preset; if (presetName) { const preset = treeshakePresets[presetName]; @@ -267,34 +274,24 @@ const getTreeshake = ( ); } } - return { - annotations: configWithPreset.annotations !== false, - moduleSideEffects: configTreeshake.pureExternalModules + } + return { + annotations: configWithPreset.annotations !== false, + correctVarValueBeforeDeclaration: configWithPreset.correctVarValueBeforeDeclaration === true, + moduleSideEffects: + typeof configTreeshake === 'object' && configTreeshake.pureExternalModules ? getHasModuleSideEffects( configTreeshake.moduleSideEffects, configTreeshake.pureExternalModules ) : getHasModuleSideEffects(configWithPreset.moduleSideEffects, undefined), - propertyReadSideEffects: - configWithPreset.propertyReadSideEffects === 'always' - ? 'always' - : configWithPreset.propertyReadSideEffects !== false, - tryCatchDeoptimization: configWithPreset.tryCatchDeoptimization !== false, - unknownGlobalSideEffects: configWithPreset.unknownGlobalSideEffects !== false - }; - } - const preset = treeshakePresets[configTreeshake]; - if (preset) { - return preset; - } - error( - errInvalidOption( - 'treeshake', - `valid values are false, true, ${printQuotedStringList( - Object.keys(treeshakePresets) - )}. You can also supply an object for more fine-grained control` - ) - ); + propertyReadSideEffects: + configWithPreset.propertyReadSideEffects === 'always' + ? 'always' + : configWithPreset.propertyReadSideEffects !== false, + tryCatchDeoptimization: configWithPreset.tryCatchDeoptimization !== false, + unknownGlobalSideEffects: configWithPreset.unknownGlobalSideEffects !== false + }; }; const getHasModuleSideEffects = ( diff --git a/src/utils/options/options.ts b/src/utils/options/options.ts index 22cf89d2923..8899b29476f 100644 --- a/src/utils/options/options.ts +++ b/src/utils/options/options.ts @@ -38,6 +38,7 @@ export const treeshakePresets: { } = { recommended: { annotations: true, + correctVarValueBeforeDeclaration: false, moduleSideEffects: () => true, propertyReadSideEffects: true, tryCatchDeoptimization: true, @@ -45,6 +46,7 @@ export const treeshakePresets: { }, safest: { annotations: true, + correctVarValueBeforeDeclaration: true, moduleSideEffects: () => true, propertyReadSideEffects: true, tryCatchDeoptimization: true, @@ -52,6 +54,7 @@ export const treeshakePresets: { }, smallest: { annotations: true, + correctVarValueBeforeDeclaration: false, moduleSideEffects: () => false, propertyReadSideEffects: false, tryCatchDeoptimization: false, diff --git a/test/cli/samples/treeshake-preset-override/main.js b/test/cli/samples/treeshake-preset-override/main.js index 2ef8f761fe7..46a542d6190 100644 --- a/test/cli/samples/treeshake-preset-override/main.js +++ b/test/cli/samples/treeshake-preset-override/main.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; diff --git a/test/form/samples/treeshake-presets/preset-with-override/_config.js b/test/form/samples/treeshake-presets/preset-with-override/_config.js index 73bd2448ad1..cf5764aed70 100644 --- a/test/form/samples/treeshake-presets/preset-with-override/_config.js +++ b/test/form/samples/treeshake-presets/preset-with-override/_config.js @@ -11,6 +11,7 @@ module.exports = { plugins: [ { buildStart(options) { + assert.strictEqual(options.treeshake.correctVarValueBeforeDeclaration, false); assert.strictEqual(options.treeshake.propertyReadSideEffects, false); assert.strictEqual(options.treeshake.tryCatchDeoptimization, false); assert.strictEqual(options.treeshake.unknownGlobalSideEffects, true); diff --git a/test/form/samples/treeshake-presets/preset-with-override/main.js b/test/form/samples/treeshake-presets/preset-with-override/main.js index 2ef8f761fe7..46a542d6190 100644 --- a/test/form/samples/treeshake-presets/preset-with-override/main.js +++ b/test/form/samples/treeshake-presets/preset-with-override/main.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; diff --git a/test/form/samples/treeshake-presets/recommended/_config.js b/test/form/samples/treeshake-presets/recommended/_config.js index e943af3e2aa..ada92c53ef7 100644 --- a/test/form/samples/treeshake-presets/recommended/_config.js +++ b/test/form/samples/treeshake-presets/recommended/_config.js @@ -8,6 +8,7 @@ module.exports = { plugins: [ { buildStart(options) { + assert.strictEqual(options.treeshake.correctVarValueBeforeDeclaration, false); assert.strictEqual(options.treeshake.propertyReadSideEffects, true); assert.strictEqual(options.treeshake.tryCatchDeoptimization, true); assert.strictEqual(options.treeshake.unknownGlobalSideEffects, false); diff --git a/test/form/samples/treeshake-presets/recommended/main.js b/test/form/samples/treeshake-presets/recommended/main.js index 2ef8f761fe7..46a542d6190 100644 --- a/test/form/samples/treeshake-presets/recommended/main.js +++ b/test/form/samples/treeshake-presets/recommended/main.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; diff --git a/test/form/samples/treeshake-presets/safest/_config.js b/test/form/samples/treeshake-presets/safest/_config.js index 35b71dfd8a1..3e5353f5c7e 100644 --- a/test/form/samples/treeshake-presets/safest/_config.js +++ b/test/form/samples/treeshake-presets/safest/_config.js @@ -8,6 +8,7 @@ module.exports = { plugins: [ { buildStart(options) { + assert.strictEqual(options.treeshake.correctVarValueBeforeDeclaration, true); assert.strictEqual(options.treeshake.propertyReadSideEffects, true); assert.strictEqual(options.treeshake.tryCatchDeoptimization, true); assert.strictEqual(options.treeshake.unknownGlobalSideEffects, true); diff --git a/test/form/samples/treeshake-presets/safest/_expected.js b/test/form/samples/treeshake-presets/safest/_expected.js index 3df383b997e..a859fbd4964 100644 --- a/test/form/samples/treeshake-presets/safest/_expected.js +++ b/test/form/samples/treeshake-presets/safest/_expected.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; diff --git a/test/form/samples/treeshake-presets/safest/main.js b/test/form/samples/treeshake-presets/safest/main.js index 2ef8f761fe7..46a542d6190 100644 --- a/test/form/samples/treeshake-presets/safest/main.js +++ b/test/form/samples/treeshake-presets/safest/main.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; diff --git a/test/form/samples/treeshake-presets/smallest/_config.js b/test/form/samples/treeshake-presets/smallest/_config.js index ac49dae5a46..39e1162c861 100644 --- a/test/form/samples/treeshake-presets/smallest/_config.js +++ b/test/form/samples/treeshake-presets/smallest/_config.js @@ -8,6 +8,7 @@ module.exports = { plugins: [ { buildStart(options) { + assert.strictEqual(options.treeshake.correctVarValueBeforeDeclaration, false); assert.strictEqual(options.treeshake.propertyReadSideEffects, false); assert.strictEqual(options.treeshake.tryCatchDeoptimization, false); assert.strictEqual(options.treeshake.unknownGlobalSideEffects, false); diff --git a/test/form/samples/treeshake-presets/smallest/main.js b/test/form/samples/treeshake-presets/smallest/main.js index 2ef8f761fe7..46a542d6190 100644 --- a/test/form/samples/treeshake-presets/smallest/main.js +++ b/test/form/samples/treeshake-presets/smallest/main.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true; diff --git a/test/form/samples/treeshake-presets/true/_config.js b/test/form/samples/treeshake-presets/true/_config.js index 9a024352632..0445050db42 100644 --- a/test/form/samples/treeshake-presets/true/_config.js +++ b/test/form/samples/treeshake-presets/true/_config.js @@ -8,6 +8,7 @@ module.exports = { plugins: [ { buildStart(options) { + assert.strictEqual(options.treeshake.correctVarValueBeforeDeclaration, false); assert.strictEqual(options.treeshake.propertyReadSideEffects, true); assert.strictEqual(options.treeshake.tryCatchDeoptimization, true); assert.strictEqual(options.treeshake.unknownGlobalSideEffects, true); diff --git a/test/form/samples/treeshake-presets/true/main.js b/test/form/samples/treeshake-presets/true/main.js index 2ef8f761fe7..46a542d6190 100644 --- a/test/form/samples/treeshake-presets/true/main.js +++ b/test/form/samples/treeshake-presets/true/main.js @@ -13,3 +13,6 @@ try { } catch {} unknownGlobal; + +if (!foo) console.log('effect'); +var foo = true;