Skip to content
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

Add option to deoptimize var declarations for tree-shaking #4139

Merged
merged 10 commits into from Jun 16, 2021
30 changes: 26 additions & 4 deletions docs/999-big-list-of-options.md
Expand Up @@ -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`

Expand All @@ -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:
Expand All @@ -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>
Expand Down
10 changes: 8 additions & 2 deletions src/ast/nodes/Identifier.ts
Expand Up @@ -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';

Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/rollup/types.d.ts
Expand Up @@ -478,6 +478,7 @@ type TreeshakingPreset = 'smallest' | 'safest' | 'recommended';

export interface TreeshakingOptions {
annotations?: boolean;
correctVarValueBeforeDeclaration?: boolean;
moduleSideEffects?: ModuleSideEffectsOption;
preset?: TreeshakingPreset;
propertyReadSideEffects?: boolean | 'always';
Expand All @@ -489,6 +490,7 @@ export interface TreeshakingOptions {

export interface NormalizedTreeshakingOptions {
annotations: boolean;
correctVarValueBeforeDeclaration: boolean;
moduleSideEffects: HasModuleSideEffects;
propertyReadSideEffects: boolean | 'always';
tryCatchDeoptimization: boolean;
Expand Down
61 changes: 29 additions & 32 deletions src/utils/options/normalizeInputOptions.ts
Expand Up @@ -7,6 +7,7 @@ import {
PreserveEntrySignaturesOption,
PureModulesOption,
RollupBuild,
TreeshakingOptions,
WarningHandler
} from '../../rollup/types';
import { ensureArray } from '../ensureArray';
Expand Down Expand Up @@ -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(
Expand All @@ -252,7 +259,7 @@ const getTreeshake = (
strictDeprecations
);
}
let configWithPreset = configTreeshake;
configWithPreset = configTreeshake;
const presetName = configTreeshake.preset;
if (presetName) {
const preset = treeshakePresets[presetName];
Expand All @@ -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 = (
Expand Down
3 changes: 3 additions & 0 deletions src/utils/options/options.ts
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions test/cli/samples/treeshake-preset-override/main.js
Expand Up @@ -13,3 +13,6 @@ try {
} catch {}

unknownGlobal;

if (!foo) console.log('effect');
var foo = true;
Expand Up @@ -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);
Expand Down
Expand Up @@ -13,3 +13,6 @@ try {
} catch {}

unknownGlobal;

if (!foo) console.log('effect');
var foo = true;
1 change: 1 addition & 0 deletions test/form/samples/treeshake-presets/recommended/_config.js
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions test/form/samples/treeshake-presets/recommended/main.js
Expand Up @@ -13,3 +13,6 @@ try {
} catch {}

unknownGlobal;

if (!foo) console.log('effect');
var foo = true;
1 change: 1 addition & 0 deletions test/form/samples/treeshake-presets/safest/_config.js
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions test/form/samples/treeshake-presets/safest/_expected.js
Expand Up @@ -13,3 +13,6 @@ try {
} catch {}

unknownGlobal;

if (!foo) console.log('effect');
var foo = true;
3 changes: 3 additions & 0 deletions test/form/samples/treeshake-presets/safest/main.js
Expand Up @@ -13,3 +13,6 @@ try {
} catch {}

unknownGlobal;

if (!foo) console.log('effect');
var foo = true;
1 change: 1 addition & 0 deletions test/form/samples/treeshake-presets/smallest/_config.js
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions test/form/samples/treeshake-presets/smallest/main.js
Expand Up @@ -13,3 +13,6 @@ try {
} catch {}

unknownGlobal;

if (!foo) console.log('effect');
var foo = true;
1 change: 1 addition & 0 deletions test/form/samples/treeshake-presets/true/_config.js
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions test/form/samples/treeshake-presets/true/main.js
Expand Up @@ -13,3 +13,6 @@ try {
} catch {}

unknownGlobal;

if (!foo) console.log('effect');
var foo = true;