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

Do not tree-shake children of unknown nodes #2510

Merged
merged 5 commits into from Oct 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 3 additions & 21 deletions src/Module.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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 = (<GenericEsTreeNode>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,
Expand Down Expand Up @@ -618,7 +600,7 @@ export default class Module {
}

includeAllInBundle() {
includeFully(this.ast);
this.ast.include(true);
}

isIncluded() {
Expand All @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/ast/nodes/AwaitExpression.ts
Expand Up @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions src/ast/nodes/BlockStatement.ts
Expand Up @@ -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);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/ast/nodes/CallExpression.ts
Expand Up @@ -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);
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/ast/nodes/ConditionalExpression.ts
Expand Up @@ -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);
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/ast/nodes/ForInStatement.ts
Expand Up @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions src/ast/nodes/ForOfStatement.ts
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/ast/nodes/Identifier.ts
Expand Up @@ -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) {
Expand Down
26 changes: 16 additions & 10 deletions src/ast/nodes/IfStatement.ts
Expand Up @@ -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);
}
}

Expand Down
14 changes: 9 additions & 5 deletions src/ast/nodes/LogicalExpression.ts
Expand Up @@ -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);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/ast/nodes/MemberExpression.ts
Expand Up @@ -176,16 +176,16 @@ 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) {
this.variable.include();
this.context.requestTreeshakingPass();
}
}
this.object.include();
this.property.include();
this.object.include(includeAllChildrenRecursively);
this.property.include(includeAllChildrenRecursively);
}

initialise() {
Expand Down
5 changes: 3 additions & 2 deletions src/ast/nodes/Program.ts
Expand Up @@ -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);
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/ast/nodes/SequenceExpression.ts
Expand Up @@ -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) {
Expand Down
7 changes: 4 additions & 3 deletions src/ast/nodes/SwitchCase.ts
Expand Up @@ -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);
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/ast/nodes/UnknownNode.ts
Expand Up @@ -5,4 +5,8 @@ export default class UnknownNode extends NodeBase {
hasEffects(_options: ExecutionPathOptions) {
return true;
}

include() {
super.include(true);
}
}
9 changes: 5 additions & 4 deletions src/ast/nodes/VariableDeclaration.ts
Expand Up @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/ast/nodes/shared/Expression.ts
Expand Up @@ -29,5 +29,5 @@ export interface ExpressionEntity extends WritableEntity {
callOptions: CallOptions,
options: ExecutionPathOptions
): boolean;
include(): void;
include(includeAllChildrenRecursively: boolean): void;
}
4 changes: 2 additions & 2 deletions src/ast/nodes/shared/FunctionNode.ts
Expand Up @@ -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() {
Expand Down
21 changes: 10 additions & 11 deletions src/ast/nodes/shared/Node.ts
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -177,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 = (<GenericEsTreeNode>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);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/ast/variables/LocalVariable.ts
Expand Up @@ -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 = <Node>declaration.parent;
while (!node.included) {
// We do not want to properly include parents in case they are part of a dead branch
Expand Down