Merge pull request #1650 from corneliusweig/treeshake-sequence-expres…

…sion

Treeshake sequence expression

src/ast/nodes/ConditionalExpression.js

@@ -27,6 +27,15 @@ export default class ConditionalExpression extends Node {
 		return testValue ? this.consequent.getValue() : this.alternate.getValue();
 	}
 
+	hasEffects ( options ) {
+		return (
+			this.included
+			|| this.test.hasEffects( options )
+			|| (this.testValue === UNKNOWN_VALUE && (this.consequent.hasEffects( options ) || this.alternate.hasEffects( options )))
+			|| (this.testValue ? this.consequent.hasEffects( options ) : this.alternate.hasEffects( options ))
+		);
+	}
+
 	render ( code, es ) {
 		if ( !this.module.bundle.treeshake ) {
 			super.render( code, es );
@@ -37,22 +46,16 @@ export default class ConditionalExpression extends Node {
 				super.render( code, es );
 			}
 
-			else if ( this.testValue ) {
-				code.remove( this.start, this.consequent.start );
-				code.remove( this.consequent.end, this.end );
-				if ( this.consequent.type === 'SequenceExpression' ) {
-					code.prependRight( this.consequent.start, '(' );
-					code.appendLeft( this.consequent.end, ')' );
-				}
-				this.consequent.render( code, es );
-			} else {
-				code.remove( this.start, this.alternate.start );
-				code.remove( this.alternate.end, this.end );
-				if ( this.alternate.type === 'SequenceExpression' ) {
-					code.prependRight( this.alternate.start, '(' );
-					code.appendLeft( this.alternate.end, ')' );
+			else {
+				const branchToRetain = this.testValue ? this.consequent : this.alternate;
+
+				code.remove( this.start, branchToRetain.start );
+				code.remove( branchToRetain.end, this.end );
+				if ( branchToRetain.type === 'SequenceExpression' ) {
+					code.prependLeft( branchToRetain.start, '(' );
+					code.appendRight( branchToRetain.end, ')' );
 				}
-				this.alternate.render( code, es );
+				branchToRetain.render( code, es );
 			}
 		}
 	}

src/ast/nodes/SequenceExpression.js

@@ -0,0 +1,59 @@
+import Node from '../Node.js';
+
+export default class SequenceExpression extends Node {
+	getValue () {
+		return this.expressions[ this.expressions.length - 1 ].getValue();
+	}
+
+	hasEffects ( options ) {
+		return this.expressions.some( expression => expression.hasEffects( options ) );
+	}
+
+	includeInBundle () {
+		if ( this.isFullyIncluded() ) return false;
+		let addedNewNodes = false;
+		if ( this.expressions[ this.expressions.length - 1 ].includeInBundle() ) {
+			addedNewNodes = true;
+		}
+		this.expressions.forEach( node => {
+			if ( node.shouldBeIncluded() ) {
+				if ( node.includeInBundle() ) {
+					addedNewNodes = true;
+				}
+			}
+		} );
+		if ( !this.included || addedNewNodes ) {
+			this.included = true;
+			return true;
+		}
+		return false;
+	}
+
+	render ( code, es ) {
+		if ( !this.module.bundle.treeshake ) {
+			super.render( code, es );
+		}
+
+		else {
+			const last = this.expressions[ this.expressions.length - 1 ];
+			const included = this.expressions.slice( 0, this.expressions.length - 1 ).filter( expression => expression.included );
+
+			if ( included.length === 0 ) {
+				code.remove( this.start, last.start );
+				code.remove( last.end, this.end );
+			}
+
+			else {
+				let previousEnd = this.start;
+				for ( const expression of included ) {
+					code.remove( previousEnd, expression.start );
+					code.appendLeft( expression.end, ', ' );
+					previousEnd = expression.end;
+				}
+
+				code.remove( previousEnd, last.start );
+				code.remove( last.end, this.end );
+			}
+		}
+	}
+}

src/ast/nodes/index.js

@@ -38,6 +38,7 @@ import Property from './Property.js';
 import RestElement from './RestElement.js';
 import ReturnStatement from './ReturnStatement.js';
 import Statement from './shared/Statement.js';
+import SequenceExpression from './SequenceExpression.js';
 import SwitchCase from './SwitchCase.js';
 import SwitchStatement from './SwitchStatement.js';
 import TaggedTemplateExpression from './TaggedTemplateExpression.js';
@@ -94,6 +95,7 @@ export default {
 	Property,
 	RestElement,
 	ReturnStatement,
+	SequenceExpression,
 	SwitchCase,
 	SwitchStatement,
 	TaggedTemplateExpression,

test/form/samples/conditional-expression/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'only retain branches with side-effects'
+};

test/form/samples/conditional-expression/_expected/amd.js

@@ -0,0 +1,16 @@
+define(function () { 'use strict';
+
+	// side-effect in condition
+	var a = foo() ? 1 : 2;
+
+	var unknownValue = bar();
+
+	// unknown branch with side-effect
+	var c = unknownValue ? foo() : 2;
+	var d = unknownValue ? 1 : foo();
+
+	// known side-effect
+	var h = foo();
+	var i = foo();
+
+});

test/form/samples/conditional-expression/_expected/cjs.js

@@ -0,0 +1,14 @@
+'use strict';
+
+// side-effect in condition
+var a = foo() ? 1 : 2;
+
+var unknownValue = bar();
+
+// unknown branch with side-effect
+var c = unknownValue ? foo() : 2;
+var d = unknownValue ? 1 : foo();
+
+// known side-effect
+var h = foo();
+var i = foo();

test/form/samples/conditional-expression/_expected/es.js

@@ -0,0 +1,12 @@
+// side-effect in condition
+var a = foo() ? 1 : 2;
+
+var unknownValue = bar();
+
+// unknown branch with side-effect
+var c = unknownValue ? foo() : 2;
+var d = unknownValue ? 1 : foo();
+
+// known side-effect
+var h = foo();
+var i = foo();

test/form/samples/conditional-expression/_expected/iife.js

@@ -0,0 +1,17 @@
+(function () {
+	'use strict';
+
+	// side-effect in condition
+	var a = foo() ? 1 : 2;
+
+	var unknownValue = bar();
+
+	// unknown branch with side-effect
+	var c = unknownValue ? foo() : 2;
+	var d = unknownValue ? 1 : foo();
+
+	// known side-effect
+	var h = foo();
+	var i = foo();
+
+}());

test/form/samples/conditional-expression/_expected/umd.js

@@ -0,0 +1,20 @@
+(function (global, factory) {
+	typeof exports === 'object' && typeof module !== 'undefined' ? factory() :
+	typeof define === 'function' && define.amd ? define(factory) :
+	(factory());
+}(this, (function () { 'use strict';
+
+	// side-effect in condition
+	var a = foo() ? 1 : 2;
+
+	var unknownValue = bar();
+
+	// unknown branch with side-effect
+	var c = unknownValue ? foo() : 2;
+	var d = unknownValue ? 1 : foo();
+
+	// known side-effect
+	var h = foo();
+	var i = foo();
+
+})));

test/form/samples/conditional-expression/main.js

@@ -0,0 +1,21 @@
+// side-effect in condition
+var a = foo() ? 1 : 2;
+
+var unknownValue = bar();
+
+// unknown branch without side-effects
+var b = unknownValue ? 1 : 2;
+
+// unknown branch with side-effect
+var c = unknownValue ? foo() : 2;
+var d = unknownValue ? 1 : foo();
+
+// no side-effects
+var e = true ? 1 : foo();
+var f = false ? foo() : 2;
+var g = true ? 1 : 2;
+
+// known side-effect
+var h = true ? foo() : 2;
+var i = false ? 1 : foo();
+

test/form/samples/conditional-put-parens-around-sequence/_expected/amd.js

@@ -1,10 +1,10 @@
 define(function () { 'use strict';
 
-	var a = (1, 2, 3 );
-	var b = (4, 5, 6 );
+	var a = (foo(), 3);
+	var b = (bar(), 6);
 	foo( a, b );
 
 	// verify works with no whitespace
-	bar((1,2),(7,8));
+	bar((foo(), 2),(bar(), 8));
 
 });

test/form/samples/conditional-put-parens-around-sequence/_expected/cjs.js

@@ -1,8 +1,8 @@
 'use strict';
 
-var a = (1, 2, 3 );
-var b = (4, 5, 6 );
+var a = (foo(), 3);
+var b = (bar(), 6);
 foo( a, b );
 
 // verify works with no whitespace
-bar((1,2),(7,8));
+bar((foo(), 2),(bar(), 8));

test/form/samples/conditional-put-parens-around-sequence/_expected/es.js

@@ -1,6 +1,6 @@
-var a = (1, 2, 3 );
-var b = (4, 5, 6 );
+var a = (foo(), 3);
+var b = (bar(), 6);
 foo( a, b );
 
 // verify works with no whitespace
-bar((1,2),(7,8));
+bar((foo(), 2),(bar(), 8));

test/form/samples/conditional-put-parens-around-sequence/_expected/iife.js

@@ -1,11 +1,11 @@
 (function () {
 	'use strict';
 
-	var a = (1, 2, 3 );
-	var b = (4, 5, 6 );
+	var a = (foo(), 3);
+	var b = (bar(), 6);
 	foo( a, b );
 
 	// verify works with no whitespace
-	bar((1,2),(7,8));
+	bar((foo(), 2),(bar(), 8));
 
 }());

test/form/samples/conditional-put-parens-around-sequence/_expected/umd.js

@@ -4,11 +4,11 @@
 	(factory());
 }(this, (function () { 'use strict';
 
-	var a = (1, 2, 3 );
-	var b = (4, 5, 6 );
+	var a = (foo(), 3);
+	var b = (bar(), 6);
 	foo( a, b );
 
 	// verify works with no whitespace
-	bar((1,2),(7,8));
+	bar((foo(), 2),(bar(), 8));
 
 })));

test/form/samples/conditional-put-parens-around-sequence/main.js

@@ -1,6 +1,6 @@
-var a = true  ? ( 1, 2, 3 ) : ( 4, 5, 6 );
-var b = false ? ( 1, 2, 3 ) : ( 4, 5, 6 );
+var a = true  ? ( 1, foo(), 3 ) : ( 4, bar(), 6 );
+var b = false ? ( 1, foo(), 3 ) : ( 4, bar(), 6 );
 foo( a, b );
 
 // verify works with no whitespace
-bar(true?(1,2):(3,4),false?(5,6):(7,8));
+bar(true?(foo(),2):(bar(),4),false?(foo(),6):(bar(),8));

test/form/samples/sequence-expression/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'only retain expressions with effects in sequence expressions (#1649)'
+};

test/form/samples/sequence-expression/_expected/amd.js

@@ -0,0 +1,14 @@
+define(function () { 'use strict';
+
+	// should remove expressions without side-effect, multiple effects
+	var a = (foo(), foo(), 2);
+	// without white-space, effect at the end
+	var b = (foo());
+
+	// should only keep final expression
+	var d = (2);
+	console.log(d);
+
+	// should infer value
+
+});

test/form/samples/sequence-expression/_expected/cjs.js

@@ -0,0 +1,12 @@
+'use strict';
+
+// should remove expressions without side-effect, multiple effects
+var a = (foo(), foo(), 2);
+// without white-space, effect at the end
+var b = (foo());
+
+// should only keep final expression
+var d = (2);
+console.log(d);
+
+// should infer value

test/form/samples/sequence-expression/_expected/es.js

@@ -0,0 +1,10 @@
+// should remove expressions without side-effect, multiple effects
+var a = (foo(), foo(), 2);
+// without white-space, effect at the end
+var b = (foo());
+
+// should only keep final expression
+var d = (2);
+console.log(d);
+
+// should infer value

test/form/samples/sequence-expression/_expected/iife.js

@@ -0,0 +1,15 @@
+(function () {
+	'use strict';
+
+	// should remove expressions without side-effect, multiple effects
+	var a = (foo(), foo(), 2);
+	// without white-space, effect at the end
+	var b = (foo());
+
+	// should only keep final expression
+	var d = (2);
+	console.log(d);
+
+	// should infer value
+
+}());

test/form/samples/sequence-expression/_expected/umd.js

@@ -0,0 +1,18 @@
+(function (global, factory) {
+	typeof exports === 'object' && typeof module !== 'undefined' ? factory() :
+	typeof define === 'function' && define.amd ? define(factory) :
+	(factory());
+}(this, (function () { 'use strict';
+
+	// should remove expressions without side-effect, multiple effects
+	var a = (foo(), foo(), 2);
+	// without white-space, effect at the end
+	var b = (foo());
+
+	// should only keep final expression
+	var d = (2);
+	console.log(d);
+
+	// should infer value
+
+})));