Skip to content

Commit

Permalink
Prevent ASI errors for logical and sequence expressions, also check
Browse files Browse the repository at this point in the history
for throw statements
  • Loading branch information
lukastaegert committed Aug 7, 2019
1 parent 1059e99 commit 808d2da
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 25 deletions.
14 changes: 2 additions & 12 deletions src/ast/nodes/ConditionalExpression.ts
@@ -1,9 +1,9 @@
import MagicString from 'magic-string';
import { BLANK } from '../../utils/blank';
import {
findFirstLineBreakOutsideComment,
findFirstOccurrenceOutsideComment,
NodeRenderOptions,
removeLineBreaks,
RenderOptions
} from '../../utils/renderHelpers';
import { removeAnnotations } from '../../utils/treeshakeNode';
Expand Down Expand Up @@ -161,17 +161,7 @@ export default class ConditionalExpression extends NodeBase implements Deoptimiz
? findFirstOccurrenceOutsideComment(code.original, '?', this.test.end)
: colonPos) + 1;
if (preventASI) {
const branchStart = (this.usedBranch as ExpressionNode).start;
let lineBreakPos = inclusionStart;
do {
lineBreakPos = findFirstLineBreakOutsideComment(code.original, lineBreakPos);
if (lineBreakPos >= 0 && lineBreakPos < branchStart) {
code.remove(lineBreakPos, lineBreakPos + 1);
lineBreakPos++;
} else {
lineBreakPos = -1;
}
} while (lineBreakPos >= 0);
removeLineBreaks(code, inclusionStart, (this.usedBranch as ExpressionNode).start);
}
code.remove(this.start, inclusionStart);
if (this.consequent.included) {
Expand Down
6 changes: 5 additions & 1 deletion src/ast/nodes/LogicalExpression.ts
Expand Up @@ -3,6 +3,7 @@ import { BLANK } from '../../utils/blank';
import {
findFirstOccurrenceOutsideComment,
NodeRenderOptions,
removeLineBreaks,
RenderOptions
} from '../../utils/renderHelpers';
import { removeAnnotations } from '../../utils/treeshakeNode';
Expand Down Expand Up @@ -155,7 +156,7 @@ export default class LogicalExpression extends NodeBase implements Deoptimizable
render(
code: MagicString,
options: RenderOptions,
{ renderedParentType, isCalleeOfRenderedParent }: NodeRenderOptions = BLANK
{ renderedParentType, isCalleeOfRenderedParent, preventASI }: NodeRenderOptions = BLANK
) {
if (!this.left.included || !this.right.included) {
const operatorPos = findFirstOccurrenceOutsideComment(
Expand All @@ -165,6 +166,9 @@ export default class LogicalExpression extends NodeBase implements Deoptimizable
);
if (this.right.included) {
code.remove(this.start, operatorPos + 2);
if (preventASI) {
removeLineBreaks(code, operatorPos + 2, this.right.start);
}
} else {
code.remove(operatorPos, this.end);
}
Expand Down
10 changes: 6 additions & 4 deletions src/ast/nodes/SequenceExpression.ts
Expand Up @@ -3,6 +3,7 @@ import { BLANK } from '../../utils/blank';
import {
getCommaSeparatedNodesWithBoundaries,
NodeRenderOptions,
removeLineBreaks,
RenderOptions
} from '../../utils/renderHelpers';
import { treeshakeNode } from '../../utils/treeshakeNode';
Expand Down Expand Up @@ -81,10 +82,9 @@ export default class SequenceExpression extends NodeBase {
render(
code: MagicString,
options: RenderOptions,
{ renderedParentType, isCalleeOfRenderedParent }: NodeRenderOptions = BLANK
{ renderedParentType, isCalleeOfRenderedParent, preventASI }: NodeRenderOptions = BLANK
) {
let firstStart = 0,
includedNodes = 0;
let includedNodes = 0;
for (const { node, start, end } of getCommaSeparatedNodesWithBoundaries(
this.expressions,
code,
Expand All @@ -96,7 +96,9 @@ export default class SequenceExpression extends NodeBase {
continue;
}
includedNodes++;
if (firstStart === 0) firstStart = start;
if (includedNodes === 1 && preventASI) {
removeLineBreaks(code, start, node.start);
}
if (node === this.expressions[this.expressions.length - 1] && includedNodes === 1) {
node.render(code, options, {
isCalleeOfRenderedParent: renderedParentType
Expand Down
6 changes: 6 additions & 0 deletions src/ast/nodes/ThrowStatement.ts
@@ -1,3 +1,5 @@
import MagicString from 'magic-string';
import { RenderOptions } from '../../utils/renderHelpers';
import { ExecutionPathOptions } from '../ExecutionPathOptions';
import * as NodeType from './NodeType';
import { ExpressionNode, StatementBase } from './shared/Node';
Expand All @@ -9,4 +11,8 @@ export default class ThrowStatement extends StatementBase {
hasEffects(_options: ExecutionPathOptions) {
return true;
}

render(code: MagicString, options: RenderOptions) {
this.argument.render(code, options, { preventASI: true });
}
}
12 changes: 12 additions & 0 deletions src/utils/renderHelpers.ts
Expand Up @@ -165,3 +165,15 @@ export function getCommaSeparatedNodesWithBoundaries<N extends Node>(
});
return splitUpNodes;
}

export function removeLineBreaks(code: MagicString, start: number, end: number) {
let lineBreakPos = start;
while (true) {
lineBreakPos = findFirstLineBreakOutsideComment(code.original, lineBreakPos);
if (lineBreakPos === -1 || lineBreakPos >= end) {
break;
}
code.remove(lineBreakPos, lineBreakPos + 1);
lineBreakPos++;
}
}
Expand Up @@ -5,3 +5,10 @@ console.log(
console.log(
/* keep me */
'expected' );

console.log(
/* keep me */
'expected');

console.log((/* keep me */
'expected'));
8 changes: 8 additions & 0 deletions test/form/samples/keep-tree-shaking-comments-no-asi/main.js
Expand Up @@ -7,3 +7,11 @@ console.log(true ?
/* keep me */
'expected' :
'unexpected');

console.log(true &&
/* keep me */
'expected');

console.log((true,
/* keep me */
'expected'));
7 changes: 2 additions & 5 deletions test/function/samples/prevent-tree-shaking-asi/_config.js
@@ -1,9 +1,6 @@
const assert = require('assert');

module.exports = {
description: 'prevent automatic semicolon insertion from changing behaviour when tree-shaking',
exports(exports) {
assert.strictEqual(exports.test1(), 'expected');
assert.strictEqual(exports.test2(), 'expected');
options: {
treeshake: { tryCatchDeoptimization: false }
}
};
33 changes: 30 additions & 3 deletions test/function/samples/prevent-tree-shaking-asi/main.js
@@ -1,15 +1,42 @@
export function test1() {
function test1() {
return true ?

/* kept */

'expected' :
'unexpected';
}
assert.strictEqual(test1(), 'expected');

export function test2() {
function test2() {
return false ?
'unexpected' :
/* kept */

'expected';
}
assert.strictEqual(test2(), 'expected');

function test3() {
return true &&
/* kept */

'expected';
}
assert.strictEqual(test3(), 'expected');

function test4() {
return 'removed',
/* kept */

'expected';
}
assert.strictEqual(test4(), 'expected');

try {
throw true ?

new Error('expected') :
null;
} catch (err) {
assert.strictEqual(err.message, 'expected');
}

0 comments on commit 808d2da

Please sign in to comment.