diff --git a/build-plugins/conditional-fsevents-import.js b/build-plugins/conditional-fsevents-import.js index 38d08dcf99d..8960e7e792b 100644 --- a/build-plugins/conditional-fsevents-import.js +++ b/build-plugins/conditional-fsevents-import.js @@ -23,8 +23,8 @@ export default function conditionalFsEventsImport() { return { code: magicString.toString(), map: magicString.generateMap({ hires: true }) }; } }, - buildEnd() { - if (!transformed) { + buildEnd(error) { + if (!(error || transformed)) { throw new Error('Could not find "fsevents-handler.js", was the file renamed?'); } } diff --git a/src/ast/nodes/DoWhileStatement.ts b/src/ast/nodes/DoWhileStatement.ts index d7892889581..baa5f0dc3b5 100644 --- a/src/ast/nodes/DoWhileStatement.ts +++ b/src/ast/nodes/DoWhileStatement.ts @@ -26,7 +26,7 @@ export default class DoWhileStatement extends StatementBase { this.included = true; this.test.include(context, includeChildrenRecursively); const { brokenFlow } = context; - this.body.include(context, includeChildrenRecursively); + this.body.includeAsSingleStatement(context, includeChildrenRecursively); context.brokenFlow = brokenFlow; } } diff --git a/src/ast/nodes/ForInStatement.ts b/src/ast/nodes/ForInStatement.ts index cc96fc0b590..609f0d022c2 100644 --- a/src/ast/nodes/ForInStatement.ts +++ b/src/ast/nodes/ForInStatement.ts @@ -49,11 +49,11 @@ export default class ForInStatement extends StatementBase { include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) { this.included = true; - this.left.includeAllDeclaredVariables(context, includeChildrenRecursively); + this.left.include(context, includeChildrenRecursively || true); this.left.deoptimizePath(EMPTY_PATH); this.right.include(context, includeChildrenRecursively); const { brokenFlow } = context; - this.body.include(context, includeChildrenRecursively); + this.body.includeAsSingleStatement(context, includeChildrenRecursively); context.brokenFlow = brokenFlow; } diff --git a/src/ast/nodes/ForOfStatement.ts b/src/ast/nodes/ForOfStatement.ts index 78bddb6eb27..65527662fc0 100644 --- a/src/ast/nodes/ForOfStatement.ts +++ b/src/ast/nodes/ForOfStatement.ts @@ -34,11 +34,11 @@ export default class ForOfStatement extends StatementBase { include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) { this.included = true; - this.left.includeAllDeclaredVariables(context, includeChildrenRecursively); + this.left.include(context, includeChildrenRecursively || true); this.left.deoptimizePath(EMPTY_PATH); this.right.include(context, includeChildrenRecursively); const { brokenFlow } = context; - this.body.include(context, includeChildrenRecursively); + this.body.includeAsSingleStatement(context, includeChildrenRecursively); context.brokenFlow = brokenFlow; } diff --git a/src/ast/nodes/ForStatement.ts b/src/ast/nodes/ForStatement.ts index 15385dd214b..0ef166ee424 100644 --- a/src/ast/nodes/ForStatement.ts +++ b/src/ast/nodes/ForStatement.ts @@ -40,11 +40,11 @@ export default class ForStatement extends StatementBase { include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) { this.included = true; - if (this.init) this.init.includeAllDeclaredVariables(context, includeChildrenRecursively); + if (this.init) this.init.includeAsSingleStatement(context, includeChildrenRecursively); if (this.test) this.test.include(context, includeChildrenRecursively); const { brokenFlow } = context; if (this.update) this.update.include(context, includeChildrenRecursively); - this.body.include(context, includeChildrenRecursively); + this.body.includeAsSingleStatement(context, includeChildrenRecursively); context.brokenFlow = brokenFlow; } diff --git a/src/ast/nodes/IfStatement.ts b/src/ast/nodes/IfStatement.ts index 651cb9a25d8..23667804428 100644 --- a/src/ast/nodes/IfStatement.ts +++ b/src/ast/nodes/IfStatement.ts @@ -136,10 +136,10 @@ export default class IfStatement extends StatementBase implements DeoptimizableE this.test.include(context, false); } if (testValue && this.consequent.shouldBeIncluded(context)) { - this.consequent.include(context, false); + this.consequent.includeAsSingleStatement(context, false); } if (this.alternate !== null && !testValue && this.alternate.shouldBeIncluded(context)) { - this.alternate.include(context, false); + this.alternate.includeAsSingleStatement(context, false); } } @@ -159,12 +159,12 @@ export default class IfStatement extends StatementBase implements DeoptimizableE const { brokenFlow } = context; let consequentBrokenFlow = BROKEN_FLOW_NONE; if (this.consequent.shouldBeIncluded(context)) { - this.consequent.include(context, false); + this.consequent.includeAsSingleStatement(context, false); consequentBrokenFlow = context.brokenFlow; context.brokenFlow = brokenFlow; } if (this.alternate !== null && this.alternate.shouldBeIncluded(context)) { - this.alternate.include(context, false); + this.alternate.includeAsSingleStatement(context, false); context.brokenFlow = context.brokenFlow < consequentBrokenFlow ? context.brokenFlow : consequentBrokenFlow; } diff --git a/src/ast/nodes/VariableDeclaration.ts b/src/ast/nodes/VariableDeclaration.ts index 6da9123206f..04016cfebb3 100644 --- a/src/ast/nodes/VariableDeclaration.ts +++ b/src/ast/nodes/VariableDeclaration.ts @@ -68,14 +68,15 @@ export default class VariableDeclaration extends NodeBase { } } - includeAllDeclaredVariables( - context: InclusionContext, - includeChildrenRecursively: IncludeChildren - ): void { + includeAsSingleStatement(context: InclusionContext, includeChildrenRecursively: IncludeChildren) { this.included = true; for (const declarator of this.declarations) { - declarator.id.included = true; - declarator.includeAllDeclaredVariables(context, includeChildrenRecursively); + if (includeChildrenRecursively || declarator.shouldBeIncluded(context)) { + declarator.include(context, includeChildrenRecursively); + if (declarator.included) { + declarator.id.include(context, includeChildrenRecursively); + } + } } } @@ -87,7 +88,6 @@ export default class VariableDeclaration extends NodeBase { render(code: MagicString, options: RenderOptions, nodeRenderOptions: NodeRenderOptions = BLANK) { if ( - nodeRenderOptions.isNoStatement || areAllDeclarationsIncludedAndNotExported(this.declarations, options.exportNamesByVariable) ) { for (const declarator of this.declarations) { @@ -111,12 +111,15 @@ export default class VariableDeclaration extends NodeBase { actualContentEnd: number, renderedContentEnd: number, systemPatternExports: Variable[], - options: RenderOptions + options: RenderOptions, + isNoStatement: boolean | undefined ): void { if (code.original.charCodeAt(this.end - 1) === 59 /*";"*/) { code.remove(this.end - 1, this.end); } - separatorString += ';'; + if (!isNoStatement) { + separatorString += ';'; + } if (lastSeparatorPos !== null) { if ( code.original.charCodeAt(actualContentEnd - 1) === 10 /*"\n"*/ && @@ -148,7 +151,7 @@ export default class VariableDeclaration extends NodeBase { private renderReplacedDeclarations( code: MagicString, options: RenderOptions, - { start = this.start, end = this.end }: NodeRenderOptions + { start = this.start, end = this.end, isNoStatement }: NodeRenderOptions ): void { const separatedNodes = getCommaSeparatedNodesWithBoundaries( this.declarations, @@ -247,7 +250,8 @@ export default class VariableDeclaration extends NodeBase { actualContentEnd!, renderedContentEnd, systemPatternExports, - options + options, + isNoStatement ); } else { code.remove(start, end); diff --git a/src/ast/nodes/VariableDeclarator.ts b/src/ast/nodes/VariableDeclarator.ts index fb1bb6efc20..4ffca9274b7 100644 --- a/src/ast/nodes/VariableDeclarator.ts +++ b/src/ast/nodes/VariableDeclarator.ts @@ -39,17 +39,6 @@ export default class VariableDeclarator extends NodeBase { } } - includeAllDeclaredVariables( - context: InclusionContext, - includeChildrenRecursively: IncludeChildren - ): void { - this.included = true; - this.id.include(context, includeChildrenRecursively); - if (this.init) { - this.init.include(context, includeChildrenRecursively); - } - } - render(code: MagicString, options: RenderOptions) { const renderId = this.id.included; if (renderId) { diff --git a/src/ast/nodes/WhileStatement.ts b/src/ast/nodes/WhileStatement.ts index 5deb69d1479..6cc8120640c 100644 --- a/src/ast/nodes/WhileStatement.ts +++ b/src/ast/nodes/WhileStatement.ts @@ -26,7 +26,7 @@ export default class WhileStatement extends StatementBase { this.included = true; this.test.include(context, includeChildrenRecursively); const { brokenFlow } = context; - this.body.include(context, includeChildrenRecursively); + this.body.includeAsSingleStatement(context, includeChildrenRecursively); context.brokenFlow = brokenFlow; } } diff --git a/src/ast/nodes/shared/Node.ts b/src/ast/nodes/shared/Node.ts index a7a1656aebd..ed7c1702836 100644 --- a/src/ast/nodes/shared/Node.ts +++ b/src/ast/nodes/shared/Node.ts @@ -65,10 +65,9 @@ export interface Node extends Entity { /** * 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. + * declarations to not include the id by default if the declarator has an effect. */ - includeAllDeclaredVariables( + includeAsSingleStatement( context: InclusionContext, includeChildrenRecursively: IncludeChildren ): void; @@ -207,10 +206,7 @@ export class NodeBase implements ExpressionNode { } } - includeAllDeclaredVariables( - context: InclusionContext, - includeChildrenRecursively: IncludeChildren - ): void { + includeAsSingleStatement(context: InclusionContext, includeChildrenRecursively: IncludeChildren) { this.include(context, includeChildrenRecursively); } diff --git a/test/function/samples/unused-do-while-loop-declaration/_config.js b/test/function/samples/unused-do-while-loop-declaration/_config.js new file mode 100644 index 00000000000..9b51c20745a --- /dev/null +++ b/test/function/samples/unused-do-while-loop-declaration/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: + 'does not partially tree-shake unused declarations with side-effects in do-while loop bodies' +}; diff --git a/test/function/samples/unused-do-while-loop-declaration/main.js b/test/function/samples/unused-do-while-loop-declaration/main.js new file mode 100644 index 00000000000..be8e73283b5 --- /dev/null +++ b/test/function/samples/unused-do-while-loop-declaration/main.js @@ -0,0 +1,8 @@ +let result = 3; +var b = 3; + +do var b = b - 1, unused = result--, unused2 = 0; +while (b > 0); + +assert.strictEqual(b, 0); +assert.strictEqual(result, 0); diff --git a/test/function/samples/unused-for-in-loop-declaration/_config.js b/test/function/samples/unused-for-in-loop-declaration/_config.js new file mode 100644 index 00000000000..9da0091ef68 --- /dev/null +++ b/test/function/samples/unused-for-in-loop-declaration/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: + 'does not partially tree-shake unused declarations with side-effects in for-in loop bodies' +}; diff --git a/test/function/samples/unused-for-in-loop-declaration/main.js b/test/function/samples/unused-for-in-loop-declaration/main.js new file mode 100644 index 00000000000..983818af93d --- /dev/null +++ b/test/function/samples/unused-for-in-loop-declaration/main.js @@ -0,0 +1,7 @@ +let result = 3; +var b = 3; + +for (var x in {foo: 1, bar: 2}) var b = b - 1, unused = result--, unused2 = 0; + +assert.strictEqual(b, 1); +assert.strictEqual(result, 1); diff --git a/test/function/samples/unused-for-loop-body-declaration/_config.js b/test/function/samples/unused-for-loop-body-declaration/_config.js new file mode 100644 index 00000000000..5912da84ed0 --- /dev/null +++ b/test/function/samples/unused-for-loop-body-declaration/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: + 'does not partially tree-shake unused declarations with side-effects in for loop bodies' +}; diff --git a/test/function/samples/unused-for-loop-body-declaration/main.js b/test/function/samples/unused-for-loop-body-declaration/main.js new file mode 100644 index 00000000000..c2074aa2fb6 --- /dev/null +++ b/test/function/samples/unused-for-loop-body-declaration/main.js @@ -0,0 +1,6 @@ +let result = 3; + +for (var a = 0; a < 3; a++) var b = a, unused = result--; + +assert.strictEqual(b, 2); +assert.strictEqual(result, 0); diff --git a/test/function/samples/unused-for-loop-declaration/_config.js b/test/function/samples/unused-for-loop-init-declaration/_config.js similarity index 100% rename from test/function/samples/unused-for-loop-declaration/_config.js rename to test/function/samples/unused-for-loop-init-declaration/_config.js diff --git a/test/function/samples/unused-for-loop-declaration/main.js b/test/function/samples/unused-for-loop-init-declaration/main.js similarity index 61% rename from test/function/samples/unused-for-loop-declaration/main.js rename to test/function/samples/unused-for-loop-init-declaration/main.js index 4f775356dca..0ccfefa3757 100644 --- a/test/function/samples/unused-for-loop-declaration/main.js +++ b/test/function/samples/unused-for-loop-init-declaration/main.js @@ -1,7 +1,7 @@ let result; let reassigned; -for (var a = (reassigned = 'reassigned'), b = 0; b < 2; b++) { +for (var a = (reassigned = 'reassigned'), b = 0, unused = 3; b < 2; b++) { result = b; } diff --git a/test/function/samples/unused-for-of-loop-declaration/_config.js b/test/function/samples/unused-for-of-loop-declaration/_config.js new file mode 100644 index 00000000000..4741e2c4ceb --- /dev/null +++ b/test/function/samples/unused-for-of-loop-declaration/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: + 'does not partially tree-shake unused declarations with side-effects in for-of loop bodies' +}; diff --git a/test/function/samples/unused-for-of-loop-declaration/main.js b/test/function/samples/unused-for-of-loop-declaration/main.js new file mode 100644 index 00000000000..070a797ad88 --- /dev/null +++ b/test/function/samples/unused-for-of-loop-declaration/main.js @@ -0,0 +1,7 @@ +let result = 3; +var b = 3; + +for (var x of [1, 2]) var b = b - 1, unused = result--, unused2 = 0; + +assert.strictEqual(b, 1); +assert.strictEqual(result, 1); diff --git a/test/function/samples/unused-if-statement-declaration/_config.js b/test/function/samples/unused-if-statement-declaration/_config.js new file mode 100644 index 00000000000..ed47aec6333 --- /dev/null +++ b/test/function/samples/unused-if-statement-declaration/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'does not partially tree-shake unused declarations with side-effects in for loops' +}; diff --git a/test/function/samples/unused-if-statement-declaration/main.js b/test/function/samples/unused-if-statement-declaration/main.js new file mode 100644 index 00000000000..7790b7d0fe1 --- /dev/null +++ b/test/function/samples/unused-if-statement-declaration/main.js @@ -0,0 +1,12 @@ +let result; + +export function testStatement(condition) { + if (condition) var a = 1, b = result = 1, unused1 = 3; + else var a = 2, c = result = 2, unused2 = 3; + return a; +} + +assert.strictEqual(testStatement(true), 1); +assert.strictEqual(result, 1); +assert.strictEqual(testStatement(false), 2); +assert.strictEqual(result, 2); diff --git a/test/function/samples/unused-while-loop-declaration/_config.js b/test/function/samples/unused-while-loop-declaration/_config.js new file mode 100644 index 00000000000..4f236593d35 --- /dev/null +++ b/test/function/samples/unused-while-loop-declaration/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: + 'does not partially tree-shake unused declarations with side-effects in while loop bodies' +}; diff --git a/test/function/samples/unused-while-loop-declaration/main.js b/test/function/samples/unused-while-loop-declaration/main.js new file mode 100644 index 00000000000..baa6528228d --- /dev/null +++ b/test/function/samples/unused-while-loop-declaration/main.js @@ -0,0 +1,8 @@ +let result = 3; +var b = 3; + +while (b > 0) +var b = b - 1, unused = result--, unused2 = 0; + +assert.strictEqual(b, 0); +assert.strictEqual(result, 0);