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

@@ -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 }`<br>
+Type: `boolean | "smallest" | "safest" | "recommended" | { annotations?: boolean, correctVarValueBeforeDeclaration?: boolean, moduleSideEffects?: ModuleSideEffectsOption, preset?: "smallest" | "safest" | "recommended", propertyReadSideEffects?: boolean | 'always', tryCatchDeoptimization?: boolean, unknownGlobalSideEffects?: boolean }`<br>
 CLI: `--treeshake`/`--no-treeshake`<br>
 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**<br>
+Type: `boolean`<br>
+CLI: `--treeshake.correctVarValueBeforeDeclaration`/`--no-treeshake.correctVarValueBeforeDeclaration`<br>
+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**<br>
 Type: `boolean | "no-external" | string[] | (id: string, external: boolean) => boolean`<br>
 CLI: `--treeshake.moduleSideEffects`/`--no-treeshake.moduleSideEffects`/`--treeshake.moduleSideEffects no-external`<br>

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

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;

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 = (

src/utils/options/options.ts

@@ -38,20 +38,23 @@ export const treeshakePresets: {
 } = {
 	recommended: {
 		annotations: true,
+		correctVarValueBeforeDeclaration: false,
 		moduleSideEffects: () => true,
 		propertyReadSideEffects: true,
 		tryCatchDeoptimization: true,
 		unknownGlobalSideEffects: false
 	},
 	safest: {
 		annotations: true,
+		correctVarValueBeforeDeclaration: true,
 		moduleSideEffects: () => true,
 		propertyReadSideEffects: true,
 		tryCatchDeoptimization: true,
 		unknownGlobalSideEffects: true
 	},
 	smallest: {
 		annotations: true,
+		correctVarValueBeforeDeclaration: false,
 		moduleSideEffects: () => false,
 		propertyReadSideEffects: false,
 		tryCatchDeoptimization: false,

test/cli/samples/treeshake-preset-override/main.js

@@ -13,3 +13,6 @@ try {
 } catch {}
 
 unknownGlobal;
+
+if (!foo) console.log('effect');
+var foo = true;

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);

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;

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);

test/form/samples/treeshake-presets/recommended/main.js

@@ -13,3 +13,6 @@ try {
 } catch {}
 
 unknownGlobal;
+
+if (!foo) console.log('effect');
+var foo = true;

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);

test/form/samples/treeshake-presets/safest/_expected.js

@@ -13,3 +13,6 @@ try {
 } catch {}
 
 unknownGlobal;
+
+if (!foo) console.log('effect');
+var foo = true;

test/form/samples/treeshake-presets/safest/main.js

@@ -13,3 +13,6 @@ try {
 } catch {}
 
 unknownGlobal;
+
+if (!foo) console.log('effect');
+var foo = true;

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);

test/form/samples/treeshake-presets/smallest/main.js

@@ -13,3 +13,6 @@ try {
 } catch {}
 
 unknownGlobal;
+
+if (!foo) console.log('effect');
+var foo = true;

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);

test/form/samples/treeshake-presets/true/main.js

@@ -13,3 +13,6 @@ try {
 } catch {}
 
 unknownGlobal;
+
+if (!foo) console.log('effect');
+var foo = true;