diff --git a/src/ast/nodes/ConditionalExpression.js b/src/ast/nodes/ConditionalExpression.js index b16b5e72b71..99368e2bcde 100644 --- a/src/ast/nodes/ConditionalExpression.js +++ b/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 ); } } } diff --git a/src/ast/nodes/SequenceExpression.js b/src/ast/nodes/SequenceExpression.js new file mode 100644 index 00000000000..cbc0a1805ed --- /dev/null +++ b/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 ); + } + } + } +} diff --git a/src/ast/nodes/index.js b/src/ast/nodes/index.js index dc483304304..dc9bb950ae7 100644 --- a/src/ast/nodes/index.js +++ b/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, diff --git a/test/form/samples/conditional-expression/_config.js b/test/form/samples/conditional-expression/_config.js new file mode 100644 index 00000000000..aadb52f877d --- /dev/null +++ b/test/form/samples/conditional-expression/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'only retain branches with side-effects' +}; diff --git a/test/form/samples/conditional-expression/_expected/amd.js b/test/form/samples/conditional-expression/_expected/amd.js new file mode 100644 index 00000000000..ebe3a3b3fa7 --- /dev/null +++ b/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(); + +}); diff --git a/test/form/samples/conditional-expression/_expected/cjs.js b/test/form/samples/conditional-expression/_expected/cjs.js new file mode 100644 index 00000000000..53d73e8a23e --- /dev/null +++ b/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(); diff --git a/test/form/samples/conditional-expression/_expected/es.js b/test/form/samples/conditional-expression/_expected/es.js new file mode 100644 index 00000000000..78ec545357e --- /dev/null +++ b/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(); diff --git a/test/form/samples/conditional-expression/_expected/iife.js b/test/form/samples/conditional-expression/_expected/iife.js new file mode 100644 index 00000000000..85e8abd32cb --- /dev/null +++ b/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(); + +}()); diff --git a/test/form/samples/conditional-expression/_expected/umd.js b/test/form/samples/conditional-expression/_expected/umd.js new file mode 100644 index 00000000000..41c39819cbb --- /dev/null +++ b/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(); + +}))); diff --git a/test/form/samples/conditional-expression/main.js b/test/form/samples/conditional-expression/main.js new file mode 100644 index 00000000000..0eb71136dd9 --- /dev/null +++ b/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(); + diff --git a/test/form/samples/conditional-put-parens-around-sequence/_expected/amd.js b/test/form/samples/conditional-put-parens-around-sequence/_expected/amd.js index 046859685f6..1b7a81ff208 100644 --- a/test/form/samples/conditional-put-parens-around-sequence/_expected/amd.js +++ b/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)); }); diff --git a/test/form/samples/conditional-put-parens-around-sequence/_expected/cjs.js b/test/form/samples/conditional-put-parens-around-sequence/_expected/cjs.js index 9bedf35cd81..5a8c1cd92ed 100644 --- a/test/form/samples/conditional-put-parens-around-sequence/_expected/cjs.js +++ b/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)); diff --git a/test/form/samples/conditional-put-parens-around-sequence/_expected/es.js b/test/form/samples/conditional-put-parens-around-sequence/_expected/es.js index f33e372379e..df64ef5945d 100644 --- a/test/form/samples/conditional-put-parens-around-sequence/_expected/es.js +++ b/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)); diff --git a/test/form/samples/conditional-put-parens-around-sequence/_expected/iife.js b/test/form/samples/conditional-put-parens-around-sequence/_expected/iife.js index 97612fbc8da..84d9d7c3f45 100644 --- a/test/form/samples/conditional-put-parens-around-sequence/_expected/iife.js +++ b/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)); }()); diff --git a/test/form/samples/conditional-put-parens-around-sequence/_expected/umd.js b/test/form/samples/conditional-put-parens-around-sequence/_expected/umd.js index 2885e8ce5ee..da177429595 100644 --- a/test/form/samples/conditional-put-parens-around-sequence/_expected/umd.js +++ b/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)); }))); diff --git a/test/form/samples/conditional-put-parens-around-sequence/main.js b/test/form/samples/conditional-put-parens-around-sequence/main.js index 60c7062e66b..5087720d1bc 100644 --- a/test/form/samples/conditional-put-parens-around-sequence/main.js +++ b/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)); diff --git a/test/form/samples/sequence-expression/_config.js b/test/form/samples/sequence-expression/_config.js new file mode 100644 index 00000000000..0b822def3d8 --- /dev/null +++ b/test/form/samples/sequence-expression/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'only retain expressions with effects in sequence expressions (#1649)' +}; diff --git a/test/form/samples/sequence-expression/_expected/amd.js b/test/form/samples/sequence-expression/_expected/amd.js new file mode 100644 index 00000000000..976cfc810df --- /dev/null +++ b/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 + +}); diff --git a/test/form/samples/sequence-expression/_expected/cjs.js b/test/form/samples/sequence-expression/_expected/cjs.js new file mode 100644 index 00000000000..b26a5f65a3c --- /dev/null +++ b/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 diff --git a/test/form/samples/sequence-expression/_expected/es.js b/test/form/samples/sequence-expression/_expected/es.js new file mode 100644 index 00000000000..66136f6070a --- /dev/null +++ b/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 diff --git a/test/form/samples/sequence-expression/_expected/iife.js b/test/form/samples/sequence-expression/_expected/iife.js new file mode 100644 index 00000000000..af169b5ee86 --- /dev/null +++ b/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 + +}()); diff --git a/test/form/samples/sequence-expression/_expected/umd.js b/test/form/samples/sequence-expression/_expected/umd.js new file mode 100644 index 00000000000..57f9a02664c --- /dev/null +++ b/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 + +}))); diff --git a/test/form/samples/sequence-expression/main.js b/test/form/samples/sequence-expression/main.js new file mode 100644 index 00000000000..ffb3fefc939 --- /dev/null +++ b/test/form/samples/sequence-expression/main.js @@ -0,0 +1,17 @@ +// should remove expressions without side-effect, multiple effects +var a = (0, foo(), 1, foo(), 2); +// without white-space, effect at the end +var b = (0,1,foo()); + +// should remove variable without effect +var c = (1, 2); + +// should only keep final expression +var d = (1, 2); +console.log(d); + +// should infer value +if ((1, 2) !== 2) { + console.log( 'effect' ); +} +