From 4e4cfb0721b6583c3fcc00723b643683c687159c Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Sun, 14 Oct 2018 12:43:06 -0500 Subject: [PATCH 1/3] fix: Ensure all effects checks return true for unknown nodes Unknown nodes may have any behaviour, as they are unknown. It is therefore unsafe to assume the unknown node, or any child of the unknown node, is able to be removed from the render. Fixes https://github.com/rollup/rollup/issues/2506 --- src/ast/nodes/shared/Node.ts | 4 ++- .../samples/unknown-token-effects/_config.js | 28 +++++++++++++++++++ .../unknown-token-effects/_expected.js | 22 +++++++++++++++ .../samples/unknown-token-effects/main.js | 22 +++++++++++++++ 4 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 test/form/samples/unknown-token-effects/_config.js create mode 100644 test/form/samples/unknown-token-effects/_expected.js create mode 100644 test/form/samples/unknown-token-effects/main.js diff --git a/src/ast/nodes/shared/Node.ts b/src/ast/nodes/shared/Node.ts index ee7fb7e401e..1e40cec34da 100644 --- a/src/ast/nodes/shared/Node.ts +++ b/src/ast/nodes/shared/Node.ts @@ -95,9 +95,11 @@ export class NodeBase implements ExpressionNode { parent: Node | { type: string; context: AstContext }, parentScope: Scope ) { - this.keys = keys[esTreeNode.type] || getAndCreateKeys(esTreeNode); this.parent = parent; this.context = parent.context; + this.keys = this.context.nodeConstructors[esTreeNode.type] + ? keys[esTreeNode.type] || getAndCreateKeys(esTreeNode) + : []; this.createScope(parentScope); this.parseNode(esTreeNode); this.initialise(); diff --git a/test/form/samples/unknown-token-effects/_config.js b/test/form/samples/unknown-token-effects/_config.js new file mode 100644 index 00000000000..b3be078cdee --- /dev/null +++ b/test/form/samples/unknown-token-effects/_config.js @@ -0,0 +1,28 @@ +module.exports = { + description: 'does not tree-shake unknown tokens', + options: { + acorn: { + plugins: { doTestExpressions: true } + }, + acornInjectPlugins: [ + acorn => { + acorn.plugins.doTestExpressions = function doTestExpressions(instance) { + instance.extend( + 'parseExprAtom', + superF => + function parseExprAtom(...args) { + if (this.type === acorn.tokTypes._do) { + this.eat(acorn.tokTypes._do); + const node = this.startNode(); + node.body = this.parseBlock(); + return this.finishNode(node, 'DoExpression'); + } + return Reflect.apply(superF, this, args); + } + ); + }; + return acorn; + } + ] + } +}; diff --git a/test/form/samples/unknown-token-effects/_expected.js b/test/form/samples/unknown-token-effects/_expected.js new file mode 100644 index 00000000000..bed7a45f3a7 --- /dev/null +++ b/test/form/samples/unknown-token-effects/_expected.js @@ -0,0 +1,22 @@ +(do {})(); +(do {}).y; +(do {}).y(); +(do {}).y = 'retained'; + +const a = do { + if (c1) { + x(); + } else if (c2) { + x.a; + } else { + x; + } +}; +a(); +a.x(); +a.y = 'retained'; + +const b = (do {})(); +const c = (do {}).c; +const d = (do {}).d(); +const e = (do {}).e = 'retained'; diff --git a/test/form/samples/unknown-token-effects/main.js b/test/form/samples/unknown-token-effects/main.js new file mode 100644 index 00000000000..bed7a45f3a7 --- /dev/null +++ b/test/form/samples/unknown-token-effects/main.js @@ -0,0 +1,22 @@ +(do {})(); +(do {}).y; +(do {}).y(); +(do {}).y = 'retained'; + +const a = do { + if (c1) { + x(); + } else if (c2) { + x.a; + } else { + x; + } +}; +a(); +a.x(); +a.y = 'retained'; + +const b = (do {})(); +const c = (do {}).c; +const d = (do {}).d(); +const e = (do {}).e = 'retained'; From d71a0756f9b118c52b6a46cfc65ee98475fd7251 Mon Sep 17 00:00:00 2001 From: Lukas Taegert Date: Mon, 29 Oct 2018 09:00:48 +0100 Subject: [PATCH 2/3] Modify test to check if necessary dependencies are included as well --- .../unknown-token-effects/_expected.js | 48 +++++++++++-------- .../samples/unknown-token-effects/main.js | 46 ++++++++++-------- 2 files changed, 56 insertions(+), 38 deletions(-) diff --git a/test/form/samples/unknown-token-effects/_expected.js b/test/form/samples/unknown-token-effects/_expected.js index bed7a45f3a7..f894704e87b 100644 --- a/test/form/samples/unknown-token-effects/_expected.js +++ b/test/form/samples/unknown-token-effects/_expected.js @@ -1,22 +1,32 @@ -(do {})(); -(do {}).y; -(do {}).y(); -(do {}).y = 'retained'; +const functionUsedInExpr = () => 1; +const objectUsedInExpr = {value: 2}; +const valueUsedInExpr = 3; -const a = do { - if (c1) { - x(); - } else if (c2) { - x.a; - } else { - x; - } +const exprValue = do { + if (unknownCondition1) { + functionUsedInExpr(); + } else if (unknownCondition2) { + objectUsedInExpr.value; + } else { + valueUsedInExpr; + } }; -a(); -a.x(); -a.y = 'retained'; -const b = (do {})(); -const c = (do {}).c; -const d = (do {}).d(); -const e = (do {}).e = 'retained'; +(do { + () => console.log('retained'); +}()); +(do { + null; +}.y); +(do { + ({ y: () => console.log('retained') }); +}.y()); +(do { + ({ + set y(value) { + console.log(value); + } + }); +}.y = 'retained'); + +export { exprValue }; diff --git a/test/form/samples/unknown-token-effects/main.js b/test/form/samples/unknown-token-effects/main.js index bed7a45f3a7..57e898d37fd 100644 --- a/test/form/samples/unknown-token-effects/main.js +++ b/test/form/samples/unknown-token-effects/main.js @@ -1,22 +1,30 @@ -(do {})(); -(do {}).y; -(do {}).y(); -(do {}).y = 'retained'; +const functionUsedInExpr = () => 1; +const objectUsedInExpr = { value: 2 }; +const valueUsedInExpr = 3; -const a = do { - if (c1) { - x(); - } else if (c2) { - x.a; - } else { - x; - } +export const exprValue = do { + if (unknownCondition1) { + functionUsedInExpr(); + } else if (unknownCondition2) { + objectUsedInExpr.value; + } else { + valueUsedInExpr; + } }; -a(); -a.x(); -a.y = 'retained'; -const b = (do {})(); -const c = (do {}).c; -const d = (do {}).d(); -const e = (do {}).e = 'retained'; +(do { + () => console.log('retained'); +}()); +(do { + null; +}.y); +(do { + ({ y: () => console.log('retained') }); +}.y()); +(do { + ({ + set y(value) { + console.log(value); + } + }); +}.y = 'retained'); From 142ebb7311929c7a6d2d1313a158bdafa1683974 Mon Sep 17 00:00:00 2001 From: Lukas Taegert Date: Mon, 29 Oct 2018 10:54:56 +0100 Subject: [PATCH 3/3] Deactivate tree-shaking via a flag to the inclusion logic --- src/Module.ts | 24 ++----------- src/ast/nodes/AwaitExpression.ts | 4 +-- src/ast/nodes/BlockStatement.ts | 5 +-- src/ast/nodes/CallExpression.ts | 6 ++-- src/ast/nodes/ConditionalExpression.ts | 12 +++---- src/ast/nodes/ForInStatement.ts | 8 ++--- src/ast/nodes/ForOfStatement.ts | 8 ++--- src/ast/nodes/Identifier.ts | 2 +- src/ast/nodes/IfStatement.ts | 26 ++++++++------ src/ast/nodes/LogicalExpression.ts | 14 +++++--- src/ast/nodes/MemberExpression.ts | 6 ++-- src/ast/nodes/Program.ts | 5 +-- src/ast/nodes/SequenceExpression.ts | 7 ++-- src/ast/nodes/SwitchCase.ts | 7 ++-- src/ast/nodes/UnknownNode.ts | 4 +++ src/ast/nodes/VariableDeclaration.ts | 9 ++--- src/ast/nodes/shared/Expression.ts | 2 +- src/ast/nodes/shared/FunctionNode.ts | 4 +-- src/ast/nodes/shared/Node.ts | 25 ++++++------- src/ast/variables/LocalVariable.ts | 2 +- .../samples/no-treeshake/_expected/amd.js | 32 +++++++++++------ .../samples/no-treeshake/_expected/cjs.js | 32 +++++++++++------ .../form/samples/no-treeshake/_expected/es.js | 35 ++++++++++++------ .../samples/no-treeshake/_expected/iife.js | 32 +++++++++++------ .../samples/no-treeshake/_expected/system.js | 36 ++++++++++++------- .../samples/no-treeshake/_expected/umd.js | 32 +++++++++++------ test/form/samples/no-treeshake/foo.js | 2 +- test/form/samples/no-treeshake/main.js | 31 +++++++++++----- .../unknown-token-effects/_expected.js | 30 ++++++++-------- .../samples/unknown-token-effects/main.js | 30 ++++++++-------- 30 files changed, 279 insertions(+), 193 deletions(-) diff --git a/src/Module.ts b/src/Module.ts index c0f80c09594..ebe7bf743dc 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -15,7 +15,7 @@ import { isLiteral } from './ast/nodes/Literal'; import MetaProperty from './ast/nodes/MetaProperty'; import * as NodeType from './ast/nodes/NodeType'; import Program from './ast/nodes/Program'; -import { GenericEsTreeNode, Node, NodeBase } from './ast/nodes/shared/Node'; +import { Node, NodeBase } from './ast/nodes/shared/Node'; import { isTemplateLiteral } from './ast/nodes/TemplateLiteral'; import ModuleScope from './ast/scopes/ModuleScope'; import { EntityPathTracker } from './ast/utils/EntityPathTracker'; @@ -129,24 +129,6 @@ function tryParse(module: Module, parse: IParse, acornOptions: AcornOptions) { } } -function includeFully(node: Node) { - node.included = true; - if (node.variable && !node.variable.included) { - node.variable.include(); - } - for (const key of node.keys) { - const value = (node)[key]; - if (value === null) continue; - if (Array.isArray(value)) { - for (const child of value) { - if (child !== null) includeFully(child); - } - } else { - includeFully(value); - } - } -} - function handleMissingExport( exportName: string, importingModule: Module, @@ -618,7 +600,7 @@ export default class Module { } includeAllInBundle() { - includeFully(this.ast); + this.ast.include(true); } isIncluded() { @@ -627,7 +609,7 @@ export default class Module { include(): boolean { this.needsTreeshakingPass = false; - if (this.ast.shouldBeIncluded()) this.ast.include(); + if (this.ast.shouldBeIncluded()) this.ast.include(false); return this.needsTreeshakingPass; } diff --git a/src/ast/nodes/AwaitExpression.ts b/src/ast/nodes/AwaitExpression.ts index a147f5c2c0c..ebd63ac99f0 100644 --- a/src/ast/nodes/AwaitExpression.ts +++ b/src/ast/nodes/AwaitExpression.ts @@ -11,8 +11,8 @@ export default class AwaitExpression extends NodeBase { type: NodeType.tAwaitExpression; argument: ExpressionNode; - include() { - super.include(); + include(includeAllChildrenRecursively: boolean) { + super.include(includeAllChildrenRecursively); if (!this.context.usesTopLevelAwait) { let parent = this.parent; do { diff --git a/src/ast/nodes/BlockStatement.ts b/src/ast/nodes/BlockStatement.ts index e539105a03d..c4fd9d0a2d7 100644 --- a/src/ast/nodes/BlockStatement.ts +++ b/src/ast/nodes/BlockStatement.ts @@ -34,10 +34,11 @@ export default class BlockStatement extends StatementBase { } } - include() { + include(includeAllChildrenRecursively: boolean) { this.included = true; for (const node of this.body) { - if (node.shouldBeIncluded()) node.include(); + if (includeAllChildrenRecursively || node.shouldBeIncluded()) + node.include(includeAllChildrenRecursively); } } diff --git a/src/ast/nodes/CallExpression.ts b/src/ast/nodes/CallExpression.ts index a6351ac8116..5a8dab171c4 100644 --- a/src/ast/nodes/CallExpression.ts +++ b/src/ast/nodes/CallExpression.ts @@ -180,10 +180,10 @@ export default class CallExpression extends NodeBase implements DeoptimizableEnt ); } - include() { - super.include(); + include(includeAllChildrenRecursively: boolean) { + super.include(includeAllChildrenRecursively); if (!this.returnExpression.included) { - this.returnExpression.include(); + this.returnExpression.include(false); } } diff --git a/src/ast/nodes/ConditionalExpression.ts b/src/ast/nodes/ConditionalExpression.ts index 10b85108779..be693bcfc67 100644 --- a/src/ast/nodes/ConditionalExpression.ts +++ b/src/ast/nodes/ConditionalExpression.ts @@ -129,14 +129,14 @@ export default class ConditionalExpression extends NodeBase implements Deoptimiz this.expressionsToBeDeoptimized = []; } - include() { + include(includeAllChildrenRecursively: boolean) { this.included = true; - if (this.usedBranch === null || this.test.shouldBeIncluded()) { - this.test.include(); - this.consequent.include(); - this.alternate.include(); + if (includeAllChildrenRecursively || this.usedBranch === null || this.test.shouldBeIncluded()) { + this.test.include(includeAllChildrenRecursively); + this.consequent.include(includeAllChildrenRecursively); + this.alternate.include(includeAllChildrenRecursively); } else { - this.usedBranch.include(); + this.usedBranch.include(includeAllChildrenRecursively); } } diff --git a/src/ast/nodes/ForInStatement.ts b/src/ast/nodes/ForInStatement.ts index 790755c25e4..824f83d265e 100644 --- a/src/ast/nodes/ForInStatement.ts +++ b/src/ast/nodes/ForInStatement.ts @@ -40,12 +40,12 @@ export default class ForInStatement extends StatementBase { ); } - include() { + include(includeAllChildrenRecursively: boolean) { this.included = true; - this.left.includeWithAllDeclaredVariables(); + this.left.includeWithAllDeclaredVariables(includeAllChildrenRecursively); this.left.deoptimizePath(EMPTY_PATH); - this.right.include(); - this.body.include(); + this.right.include(includeAllChildrenRecursively); + this.body.include(includeAllChildrenRecursively); } render(code: MagicString, options: RenderOptions) { diff --git a/src/ast/nodes/ForOfStatement.ts b/src/ast/nodes/ForOfStatement.ts index 7014e76eee3..d03861dcfba 100644 --- a/src/ast/nodes/ForOfStatement.ts +++ b/src/ast/nodes/ForOfStatement.ts @@ -41,12 +41,12 @@ export default class ForOfStatement extends StatementBase { ); } - include() { + include(includeAllChildrenRecursively: boolean) { this.included = true; - this.left.includeWithAllDeclaredVariables(); + this.left.includeWithAllDeclaredVariables(includeAllChildrenRecursively); this.left.deoptimizePath(EMPTY_PATH); - this.right.include(); - this.body.include(); + this.right.include(includeAllChildrenRecursively); + this.body.include(includeAllChildrenRecursively); } render(code: MagicString, options: RenderOptions) { diff --git a/src/ast/nodes/Identifier.ts b/src/ast/nodes/Identifier.ts index 45d892fea7f..b512dd83b09 100644 --- a/src/ast/nodes/Identifier.ts +++ b/src/ast/nodes/Identifier.ts @@ -108,7 +108,7 @@ export default class Identifier extends NodeBase { return !this.variable || this.variable.hasEffectsWhenCalledAtPath(path, callOptions, options); } - include() { + include(_includeAllChildrenRecursively: boolean) { if (!this.included) { this.included = true; if (this.variable !== null && !this.variable.included) { diff --git a/src/ast/nodes/IfStatement.ts b/src/ast/nodes/IfStatement.ts index 68665d56f58..c2eda61bffb 100644 --- a/src/ast/nodes/IfStatement.ts +++ b/src/ast/nodes/IfStatement.ts @@ -42,22 +42,28 @@ export default class IfStatement extends StatementBase implements DeoptimizableE : this.alternate !== null && this.alternate.hasEffects(options); } - include() { + include(includeAllChildrenRecursively: boolean) { this.included = true; - if (this.testValue === UNKNOWN_VALUE || this.test.shouldBeIncluded()) { - this.test.include(); + if (includeAllChildrenRecursively) { + this.test.include(true); + this.consequent.include(true); + if (this.alternate !== null) { + this.alternate.include(true); + } + return; } - if ( - (this.testValue === UNKNOWN_VALUE || this.testValue) && - this.consequent.shouldBeIncluded() - ) { - this.consequent.include(); + const hasUnknownTest = this.testValue === UNKNOWN_VALUE; + if (hasUnknownTest || this.test.shouldBeIncluded()) { + this.test.include(false); + } + if ((hasUnknownTest || this.testValue) && this.consequent.shouldBeIncluded()) { + this.consequent.include(false); } if ( this.alternate !== null && - ((this.testValue === UNKNOWN_VALUE || !this.testValue) && this.alternate.shouldBeIncluded()) + ((hasUnknownTest || !this.testValue) && this.alternate.shouldBeIncluded()) ) { - this.alternate.include(); + this.alternate.include(false); } } diff --git a/src/ast/nodes/LogicalExpression.ts b/src/ast/nodes/LogicalExpression.ts index 8c5129c70b3..91caf81426e 100644 --- a/src/ast/nodes/LogicalExpression.ts +++ b/src/ast/nodes/LogicalExpression.ts @@ -122,13 +122,17 @@ export default class LogicalExpression extends NodeBase implements Deoptimizable return this.usedBranch.hasEffectsWhenCalledAtPath(path, callOptions, options); } - include() { + include(includeAllChildrenRecursively: boolean) { this.included = true; - if (this.usedBranch === null || this.unusedBranch.shouldBeIncluded()) { - this.left.include(); - this.right.include(); + if ( + includeAllChildrenRecursively || + this.usedBranch === null || + this.unusedBranch.shouldBeIncluded() + ) { + this.left.include(includeAllChildrenRecursively); + this.right.include(includeAllChildrenRecursively); } else { - this.usedBranch.include(); + this.usedBranch.include(includeAllChildrenRecursively); } } diff --git a/src/ast/nodes/MemberExpression.ts b/src/ast/nodes/MemberExpression.ts index e981cf5fbf2..b4cfc8a685a 100644 --- a/src/ast/nodes/MemberExpression.ts +++ b/src/ast/nodes/MemberExpression.ts @@ -176,7 +176,7 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE ); } - include() { + include(includeAllChildrenRecursively: boolean) { if (!this.included) { this.included = true; if (this.variable !== null && !this.variable.included) { @@ -184,8 +184,8 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE this.context.requestTreeshakingPass(); } } - this.object.include(); - this.property.include(); + this.object.include(includeAllChildrenRecursively); + this.property.include(includeAllChildrenRecursively); } initialise() { diff --git a/src/ast/nodes/Program.ts b/src/ast/nodes/Program.ts index 0ad7243e9c5..06952c8912b 100644 --- a/src/ast/nodes/Program.ts +++ b/src/ast/nodes/Program.ts @@ -15,10 +15,11 @@ export default class Program extends NodeBase { } } - include() { + include(includeAllChildrenRecursively: boolean) { this.included = true; for (const node of this.body) { - if (node.shouldBeIncluded()) node.include(); + if (includeAllChildrenRecursively || node.shouldBeIncluded()) + node.include(includeAllChildrenRecursively); } } diff --git a/src/ast/nodes/SequenceExpression.ts b/src/ast/nodes/SequenceExpression.ts index 47a6585f281..d06aae78965 100644 --- a/src/ast/nodes/SequenceExpression.ts +++ b/src/ast/nodes/SequenceExpression.ts @@ -63,13 +63,14 @@ export default class SequenceExpression extends NodeBase { ); } - include() { + include(includeAllChildrenRecursively: boolean) { this.included = true; for (let i = 0; i < this.expressions.length - 1; i++) { const node = this.expressions[i]; - if (node.shouldBeIncluded()) node.include(); + if (includeAllChildrenRecursively || node.shouldBeIncluded()) + node.include(includeAllChildrenRecursively); } - this.expressions[this.expressions.length - 1].include(); + this.expressions[this.expressions.length - 1].include(includeAllChildrenRecursively); } deoptimizePath(path: ObjectPath) { diff --git a/src/ast/nodes/SwitchCase.ts b/src/ast/nodes/SwitchCase.ts index a4a91d4ebf7..30068eaf487 100644 --- a/src/ast/nodes/SwitchCase.ts +++ b/src/ast/nodes/SwitchCase.ts @@ -12,11 +12,12 @@ export default class SwitchCase extends NodeBase { test: ExpressionNode | null; consequent: StatementNode[]; - include() { + include(includeAllChildrenRecursively: boolean) { this.included = true; - if (this.test) this.test.include(); + if (this.test) this.test.include(includeAllChildrenRecursively); for (const node of this.consequent) { - if (node.shouldBeIncluded()) node.include(); + if (includeAllChildrenRecursively || node.shouldBeIncluded()) + node.include(includeAllChildrenRecursively); } } diff --git a/src/ast/nodes/UnknownNode.ts b/src/ast/nodes/UnknownNode.ts index 06ee462c461..7dca98101eb 100644 --- a/src/ast/nodes/UnknownNode.ts +++ b/src/ast/nodes/UnknownNode.ts @@ -5,4 +5,8 @@ export default class UnknownNode extends NodeBase { hasEffects(_options: ExecutionPathOptions) { return true; } + + include() { + super.include(true); + } } diff --git a/src/ast/nodes/VariableDeclaration.ts b/src/ast/nodes/VariableDeclaration.ts index e146bfe726d..36216bb4cda 100644 --- a/src/ast/nodes/VariableDeclaration.ts +++ b/src/ast/nodes/VariableDeclaration.ts @@ -35,17 +35,18 @@ export default class VariableDeclaration extends NodeBase { return false; } - includeWithAllDeclaredVariables() { + includeWithAllDeclaredVariables(includeAllChildrenRecursively: boolean) { this.included = true; for (const declarator of this.declarations) { - declarator.include(); + declarator.include(includeAllChildrenRecursively); } } - include() { + include(includeAllChildrenRecursively: boolean) { this.included = true; for (const declarator of this.declarations) { - if (declarator.shouldBeIncluded()) declarator.include(); + if (includeAllChildrenRecursively || declarator.shouldBeIncluded()) + declarator.include(includeAllChildrenRecursively); } } diff --git a/src/ast/nodes/shared/Expression.ts b/src/ast/nodes/shared/Expression.ts index 7626a6da816..7c4eb07559e 100644 --- a/src/ast/nodes/shared/Expression.ts +++ b/src/ast/nodes/shared/Expression.ts @@ -29,5 +29,5 @@ export interface ExpressionEntity extends WritableEntity { callOptions: CallOptions, options: ExecutionPathOptions ): boolean; - include(): void; + include(includeAllChildrenRecursively: boolean): void; } diff --git a/src/ast/nodes/shared/FunctionNode.ts b/src/ast/nodes/shared/FunctionNode.ts index ec54155f7e1..3ae5c3ce695 100644 --- a/src/ast/nodes/shared/FunctionNode.ts +++ b/src/ast/nodes/shared/FunctionNode.ts @@ -60,9 +60,9 @@ export default class FunctionNode extends NodeBase { return this.body.hasEffects(innerOptions); } - include() { + include(includeAllChildrenRecursively: boolean) { this.scope.variables.arguments.include(); - super.include(); + super.include(includeAllChildrenRecursively); } initialise() { diff --git a/src/ast/nodes/shared/Node.ts b/src/ast/nodes/shared/Node.ts index 1e40cec34da..b79725d3339 100644 --- a/src/ast/nodes/shared/Node.ts +++ b/src/ast/nodes/shared/Node.ts @@ -49,19 +49,18 @@ export interface Node extends Entity { hasEffects(options: ExecutionPathOptions): boolean; /** - * Includes the node in the bundle. Children are usually included if they are - * necessary for this node (e.g. a function body) or if they have effects. - * Necessary variables need to be included as well. Should return true if any - * nodes or variables have been added that were missing before. + * Includes the node in the bundle. If the flag is not set, children are usually included + * if they are necessary for this node (e.g. a function body) or if they have effects. + * Necessary variables need to be included as well. */ - include(): void; + include(includeAllChildrenRecursively: boolean): void; /** * Alternative version of include to override the default behaviour of * declarations to only include nodes for declarators that have an effect. Necessary * for for-loops that do not use a declared loop variable. */ - includeWithAllDeclaredVariables(): void; + includeWithAllDeclaredVariables(includeAllChildrenRecursively: boolean): void; render(code: MagicString, options: RenderOptions, nodeRenderOptions?: NodeRenderOptions): void; /** @@ -95,11 +94,9 @@ export class NodeBase implements ExpressionNode { parent: Node | { type: string; context: AstContext }, parentScope: Scope ) { + this.keys = keys[esTreeNode.type] || getAndCreateKeys(esTreeNode); this.parent = parent; this.context = parent.context; - this.keys = this.context.nodeConstructors[esTreeNode.type] - ? keys[esTreeNode.type] || getAndCreateKeys(esTreeNode) - : []; this.createScope(parentScope); this.parseNode(esTreeNode); this.initialise(); @@ -179,23 +176,23 @@ export class NodeBase implements ExpressionNode { return true; } - include() { + include(includeAllChildrenRecursively: boolean) { this.included = true; for (const key of this.keys) { const value = (this)[key]; if (value === null) continue; if (Array.isArray(value)) { for (const child of value) { - if (child !== null) child.include(); + if (child !== null) child.include(includeAllChildrenRecursively); } } else { - value.include(); + value.include(includeAllChildrenRecursively); } } } - includeWithAllDeclaredVariables() { - this.include(); + includeWithAllDeclaredVariables(includeAllChildrenRecursively: boolean) { + this.include(includeAllChildrenRecursively); } /** diff --git a/src/ast/variables/LocalVariable.ts b/src/ast/variables/LocalVariable.ts index 7896842687f..efd9fa95ff2 100644 --- a/src/ast/variables/LocalVariable.ts +++ b/src/ast/variables/LocalVariable.ts @@ -153,7 +153,7 @@ export default class LocalVariable extends Variable { this.included = true; for (const declaration of this.declarations) { // If node is a default export, it can save a tree-shaking run to include the full declaration now - if (!declaration.included) declaration.include(); + if (!declaration.included) declaration.include(false); let node = declaration.parent; while (!node.included) { // We do not want to properly include parents in case they are part of a dead branch diff --git a/test/form/samples/no-treeshake/_expected/amd.js b/test/form/samples/no-treeshake/_expected/amd.js index 5a10b8c64a2..ecb358c7923 100644 --- a/test/form/samples/no-treeshake/_expected/amd.js +++ b/test/form/samples/no-treeshake/_expected/amd.js @@ -1,24 +1,36 @@ define(['exports', 'external'], function (exports, external) { 'use strict'; - var foo = 'unused'; + var foo = 13; const quux = 1; const other = () => quux; - function bar () { - return foo; + function baz() { + return foo + external.value; } - function baz () { - return 13 + external.value; + var create = Object.create, + getPrototypeOf = Object.getPrototypeOf; + + function unusedButIncluded() { + const unusedConst = 'unused'; + if (true) { + true ? 'first' : 'second'; + } else { + (true && 'first') || 'second'; + } + 'sequence', 'expression'; + switch ('test') { + case 'test': + (() => {})(); + case 'other': + 'no effect'; + default: + const ignored = 2; + } } - const moreExternal = external.more; - - var create = Object.create, getPrototypeOf = Object.getPrototypeOf; - - exports.baz = baz; exports.create = create; exports.getPrototypeOf = getPrototypeOf; exports.strange = quux; diff --git a/test/form/samples/no-treeshake/_expected/cjs.js b/test/form/samples/no-treeshake/_expected/cjs.js index 53fce965345..c8f47529115 100644 --- a/test/form/samples/no-treeshake/_expected/cjs.js +++ b/test/form/samples/no-treeshake/_expected/cjs.js @@ -4,25 +4,37 @@ Object.defineProperty(exports, '__esModule', { value: true }); var external = require('external'); -var foo = 'unused'; +var foo = 13; const quux = 1; const other = () => quux; -function bar () { - return foo; +function baz() { + return foo + external.value; } -function baz () { - return 13 + external.value; +var create = Object.create, + getPrototypeOf = Object.getPrototypeOf; + +function unusedButIncluded() { + const unusedConst = 'unused'; + if (true) { + true ? 'first' : 'second'; + } else { + (true && 'first') || 'second'; + } + 'sequence', 'expression'; + switch ('test') { + case 'test': + (() => {})(); + case 'other': + 'no effect'; + default: + const ignored = 2; + } } -const moreExternal = external.more; - -var create = Object.create, getPrototypeOf = Object.getPrototypeOf; - -exports.baz = baz; exports.create = create; exports.getPrototypeOf = getPrototypeOf; exports.strange = quux; diff --git a/test/form/samples/no-treeshake/_expected/es.js b/test/form/samples/no-treeshake/_expected/es.js index 48e3035d91b..28b4f47617f 100644 --- a/test/form/samples/no-treeshake/_expected/es.js +++ b/test/form/samples/no-treeshake/_expected/es.js @@ -1,21 +1,34 @@ -import { value, more } from 'external'; +import { value } from 'external'; -var foo = 'unused'; +var foo = 13; const quux = 1; const other = () => quux; -function bar () { - return foo; +function baz() { + return foo + value; } -function baz () { - return 13 + value; +var create = Object.create, + getPrototypeOf = Object.getPrototypeOf; + +function unusedButIncluded() { + const unusedConst = 'unused'; + if (true) { + true ? 'first' : 'second'; + } else { + (true && 'first') || 'second'; + } + 'sequence', 'expression'; + switch ('test') { + case 'test': + (() => {})(); + case 'other': + 'no effect'; + default: + const ignored = 2; + } } -const moreExternal = more; - -var create = Object.create, getPrototypeOf = Object.getPrototypeOf; - -export { baz, create, getPrototypeOf, quux as strange }; +export { create, getPrototypeOf, quux as strange }; diff --git a/test/form/samples/no-treeshake/_expected/iife.js b/test/form/samples/no-treeshake/_expected/iife.js index 060a647e7d8..a8294f43882 100644 --- a/test/form/samples/no-treeshake/_expected/iife.js +++ b/test/form/samples/no-treeshake/_expected/iife.js @@ -1,25 +1,37 @@ var stirred = (function (exports,external) { 'use strict'; - var foo = 'unused'; + var foo = 13; const quux = 1; const other = () => quux; - function bar () { - return foo; + function baz() { + return foo + external.value; } - function baz () { - return 13 + external.value; + var create = Object.create, + getPrototypeOf = Object.getPrototypeOf; + + function unusedButIncluded() { + const unusedConst = 'unused'; + if (true) { + true ? 'first' : 'second'; + } else { + (true && 'first') || 'second'; + } + 'sequence', 'expression'; + switch ('test') { + case 'test': + (() => {})(); + case 'other': + 'no effect'; + default: + const ignored = 2; + } } - const moreExternal = external.more; - - var create = Object.create, getPrototypeOf = Object.getPrototypeOf; - - exports.baz = baz; exports.create = create; exports.getPrototypeOf = getPrototypeOf; exports.strange = quux; diff --git a/test/form/samples/no-treeshake/_expected/system.js b/test/form/samples/no-treeshake/_expected/system.js index 1bbfb5fb64c..426db5b89f8 100644 --- a/test/form/samples/no-treeshake/_expected/system.js +++ b/test/form/samples/no-treeshake/_expected/system.js @@ -1,33 +1,43 @@ System.register('stirred', ['external'], function (exports, module) { 'use strict'; - var value, more; + var value; return { setters: [function (module) { value = module.value; - more = module.more; }], execute: function () { - exports('baz', baz); - - var foo = 'unused'; + var foo = 13; const quux = exports('strange', 1); const other = () => quux; - function bar () { - return foo; + function baz() { + return foo + value; } - function baz () { - return 13 + value; + var create = exports('create', Object.create), + getPrototypeOf = exports('getPrototypeOf', Object.getPrototypeOf); + + function unusedButIncluded() { + const unusedConst = 'unused'; + if (true) { + true ? 'first' : 'second'; + } else { + (true && 'first') || 'second'; + } + 'sequence', 'expression'; + switch ('test') { + case 'test': + (() => {})(); + case 'other': + 'no effect'; + default: + const ignored = 2; + } } - const moreExternal = more; - - var create = exports('create', Object.create), getPrototypeOf = exports('getPrototypeOf', Object.getPrototypeOf); - } }; }); diff --git a/test/form/samples/no-treeshake/_expected/umd.js b/test/form/samples/no-treeshake/_expected/umd.js index 05f355ea6c2..92caa73a32f 100644 --- a/test/form/samples/no-treeshake/_expected/umd.js +++ b/test/form/samples/no-treeshake/_expected/umd.js @@ -4,25 +4,37 @@ (factory((global.stirred = {}),global.external)); }(this, (function (exports,external) { 'use strict'; - var foo = 'unused'; + var foo = 13; const quux = 1; const other = () => quux; - function bar () { - return foo; + function baz() { + return foo + external.value; } - function baz () { - return 13 + external.value; + var create = Object.create, + getPrototypeOf = Object.getPrototypeOf; + + function unusedButIncluded() { + const unusedConst = 'unused'; + if (true) { + true ? 'first' : 'second'; + } else { + (true && 'first') || 'second'; + } + 'sequence', 'expression'; + switch ('test') { + case 'test': + (() => {})(); + case 'other': + 'no effect'; + default: + const ignored = 2; + } } - const moreExternal = external.more; - - var create = Object.create, getPrototypeOf = Object.getPrototypeOf; - - exports.baz = baz; exports.create = create; exports.getPrototypeOf = getPrototypeOf; exports.strange = quux; diff --git a/test/form/samples/no-treeshake/foo.js b/test/form/samples/no-treeshake/foo.js index e4c7f512867..e9ac6f7b062 100644 --- a/test/form/samples/no-treeshake/foo.js +++ b/test/form/samples/no-treeshake/foo.js @@ -1 +1 @@ -export default 'unused'; +export default 13; diff --git a/test/form/samples/no-treeshake/main.js b/test/form/samples/no-treeshake/main.js index 154a571059f..2a6361ba2b7 100644 --- a/test/form/samples/no-treeshake/main.js +++ b/test/form/samples/no-treeshake/main.js @@ -1,15 +1,28 @@ import * as external from 'external'; -import foo from './foo.js' +import foo from './foo.js'; export { quux as strange } from './quux.js'; -function bar () { - return foo; +function baz() { + return foo + external.value; } -export function baz () { - return 13 + external.value; -} - -const moreExternal = external.more; +export var create = Object.create, + getPrototypeOf = Object.getPrototypeOf; -export var create = Object.create, getPrototypeOf = Object.getPrototypeOf; +function unusedButIncluded() { + const unusedConst = 'unused'; + if (true) { + true ? 'first' : 'second'; + } else { + (true && 'first') || 'second'; + } + 'sequence', 'expression'; + switch ('test') { + case 'test': + (() => {})(); + case 'other': + 'no effect'; + default: + const ignored = 2; + } +} diff --git a/test/form/samples/unknown-token-effects/_expected.js b/test/form/samples/unknown-token-effects/_expected.js index f894704e87b..ecb1863985c 100644 --- a/test/form/samples/unknown-token-effects/_expected.js +++ b/test/form/samples/unknown-token-effects/_expected.js @@ -1,17 +1,3 @@ -const functionUsedInExpr = () => 1; -const objectUsedInExpr = {value: 2}; -const valueUsedInExpr = 3; - -const exprValue = do { - if (unknownCondition1) { - functionUsedInExpr(); - } else if (unknownCondition2) { - objectUsedInExpr.value; - } else { - valueUsedInExpr; - } -}; - (do { () => console.log('retained'); }()); @@ -29,4 +15,20 @@ const exprValue = do { }); }.y = 'retained'); +const functionUsedInExpr = () => 1; +const objectUsedInExpr = { value: 2 }; +const valueUsedInExpr = 3; + +const exprValue = do { + if (unknownCondition1) { + functionUsedInExpr(); + } else if (unknownCondition2) { + objectUsedInExpr.value; + } else if (unknownCondition3) { + valueUsedInExpr; + } else { + 'direct value'; + } +}; + export { exprValue }; diff --git a/test/form/samples/unknown-token-effects/main.js b/test/form/samples/unknown-token-effects/main.js index 57e898d37fd..6242104883a 100644 --- a/test/form/samples/unknown-token-effects/main.js +++ b/test/form/samples/unknown-token-effects/main.js @@ -1,17 +1,3 @@ -const functionUsedInExpr = () => 1; -const objectUsedInExpr = { value: 2 }; -const valueUsedInExpr = 3; - -export const exprValue = do { - if (unknownCondition1) { - functionUsedInExpr(); - } else if (unknownCondition2) { - objectUsedInExpr.value; - } else { - valueUsedInExpr; - } -}; - (do { () => console.log('retained'); }()); @@ -28,3 +14,19 @@ export const exprValue = do { } }); }.y = 'retained'); + +const functionUsedInExpr = () => 1; +const objectUsedInExpr = { value: 2 }; +const valueUsedInExpr = 3; + +export const exprValue = do { + if (unknownCondition1) { + functionUsedInExpr(); + } else if (unknownCondition2) { + objectUsedInExpr.value; + } else if (unknownCondition3) { + valueUsedInExpr; + } else { + 'direct value' + } +};