Skip to content

Commit

Permalink
Remove conditional expressions if they have no effect.
Browse files Browse the repository at this point in the history
- if the test evaluation produces an effect, keep the whole expression
- if the test evaluation is unkown but has no effect, remove the expression unless any branch has an effect
- if the test evaluation is known, replace the expression with the resulting branch (as before)
  • Loading branch information
Cornelius Weig committed Sep 23, 2017
1 parent b949eb0 commit ce0ceb8
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 15 deletions.
33 changes: 18 additions & 15 deletions src/ast/nodes/ConditionalExpression.js
Expand Up @@ -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 );
Expand All @@ -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.prependRight( branchToRetain.start, '(' );
code.appendLeft( branchToRetain.end, ')' );
}
this.alternate.render( code, es );
branchToRetain.render( code, es );
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions test/form/samples/conditional-expression/_config.js
@@ -0,0 +1,3 @@
module.exports = {
description: 'only retain branches with side-effects'
};
16 changes: 16 additions & 0 deletions 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();

});
14 changes: 14 additions & 0 deletions 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();
12 changes: 12 additions & 0 deletions 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();
17 changes: 17 additions & 0 deletions 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();

}());
20 changes: 20 additions & 0 deletions 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();

})));
21 changes: 21 additions & 0 deletions 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();

0 comments on commit ce0ceb8

Please sign in to comment.