Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep annotations and comments when simplifying logical and conditional expressions #2955

Merged
merged 1 commit into from Jun 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/ast/nodes/ConditionalExpression.ts
@@ -1,6 +1,10 @@
import MagicString from 'magic-string';
import { BLANK } from '../../utils/blank';
import { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers';
import {
findFirstOccurrenceOutsideComment,
NodeRenderOptions,
RenderOptions
} from '../../utils/renderHelpers';
import { removeAnnotations } from '../../utils/treeshakeNode';
import CallOptions from '../CallOptions';
import { DeoptimizableEntity } from '../DeoptimizableEntity';
Expand Down Expand Up @@ -150,8 +154,18 @@ export default class ConditionalExpression extends NodeBase implements Deoptimiz
{ renderedParentType, isCalleeOfRenderedParent }: NodeRenderOptions = BLANK
) {
if (!this.test.included) {
code.remove(this.start, (this.usedBranch as ExpressionNode).start);
code.remove((this.usedBranch as ExpressionNode).end, this.end);
const colonPos = findFirstOccurrenceOutsideComment(code.original, ':', this.consequent.end);
if (this.consequent.included) {
const questionmarkPos = findFirstOccurrenceOutsideComment(
code.original,
'?',
this.test.end
);
code.remove(this.start, questionmarkPos + 1);
code.remove(colonPos, this.end);
} else {
code.remove(this.start, colonPos + 1);
}
removeAnnotations(this, code);
(this.usedBranch as ExpressionNode).render(code, options, {
isCalleeOfRenderedParent: renderedParentType
Expand Down
18 changes: 15 additions & 3 deletions src/ast/nodes/LogicalExpression.ts
@@ -1,6 +1,10 @@
import MagicString from 'magic-string';
import { BLANK } from '../../utils/blank';
import { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers';
import {
findFirstOccurrenceOutsideComment,
NodeRenderOptions,
RenderOptions
} from '../../utils/renderHelpers';
import { removeAnnotations } from '../../utils/treeshakeNode';
import CallOptions from '../CallOptions';
import { DeoptimizableEntity } from '../DeoptimizableEntity';
Expand Down Expand Up @@ -154,8 +158,16 @@ export default class LogicalExpression extends NodeBase implements Deoptimizable
{ renderedParentType, isCalleeOfRenderedParent }: NodeRenderOptions = BLANK
) {
if (!this.left.included || !this.right.included) {
code.remove(this.start, (this.usedBranch as ExpressionNode).start);
code.remove((this.usedBranch as ExpressionNode).end, this.end);
const operatorPos = findFirstOccurrenceOutsideComment(
code.original,
this.operator,
this.left.end
);
if (this.right.included) {
code.remove(this.start, operatorPos + 2);
} else {
code.remove(operatorPos, this.end);
}
removeAnnotations(this, code);
(this.usedBranch as ExpressionNode).render(code, options, {
isCalleeOfRenderedParent: renderedParentType
Expand Down
7 changes: 0 additions & 7 deletions src/ast/nodes/SequenceExpression.ts
Expand Up @@ -84,7 +84,6 @@ export default class SequenceExpression extends NodeBase {
{ renderedParentType, isCalleeOfRenderedParent }: NodeRenderOptions = BLANK
) {
let firstStart = 0,
lastEnd,
includedNodes = 0;
for (const { node, start, end } of getCommaSeparatedNodesWithBoundaries(
this.expressions,
Expand All @@ -98,7 +97,6 @@ export default class SequenceExpression extends NodeBase {
}
includedNodes++;
if (firstStart === 0) firstStart = start;
lastEnd = end;
if (node === this.expressions[this.expressions.length - 1] && includedNodes === 1) {
node.render(code, options, {
isCalleeOfRenderedParent: renderedParentType
Expand All @@ -110,10 +108,5 @@ export default class SequenceExpression extends NodeBase {
node.render(code, options);
}
}
// Round brackets are part of the actual parent and should be re-added in case the parent changed
if (includedNodes > 1 && renderedParentType) {
code.prependRight(firstStart, '(');
code.appendLeft(lastEnd as number, ')');
}
}
}
Expand Up @@ -3,7 +3,7 @@ define(['exports'], function (exports) { 'use strict';
var hsl2hsv = (h, s, l) => {
const t = s * (l < 0.5 ? 1 : 1 - l),
V = 1 + t,
S = 2 * t / V;
S = 2 * t / V ;
return [h, S, V];
};

Expand Down
Expand Up @@ -5,7 +5,7 @@ Object.defineProperty(exports, '__esModule', { value: true });
var hsl2hsv = (h, s, l) => {
const t = s * (l < 0.5 ? 1 : 1 - l),
V = 1 + t,
S = 2 * t / V;
S = 2 * t / V ;
return [h, S, V];
};

Expand Down
@@ -1,7 +1,7 @@
var hsl2hsv = (h, s, l) => {
const t = s * (l < 0.5 ? 1 : 1 - l),
V = 1 + t,
S = 2 * t / V;
S = 2 * t / V ;
return [h, S, V];
};

Expand Down
Expand Up @@ -6,7 +6,7 @@ System.register([], function (exports) {
var hsl2hsv = exports('default', (h, s, l) => {
const t = s * (l < 0.5 ? 1 : 1 - l),
V = 1 + t,
S = 2 * t / V;
S = 2 * t / V ;
return [h, S, V];
});

Expand Down
@@ -1,6 +1,6 @@
define(function () { 'use strict';

var main = null;
var main = null;

return main;

Expand Down
@@ -1,5 +1,5 @@
'use strict';

var main = null;
var main = null;

module.exports = main;
@@ -1,3 +1,3 @@
var main = null;
var main = null;

export default main;
Expand Up @@ -3,7 +3,7 @@ System.register([], function (exports) {
return {
execute: function () {

var main = exports('default', null);
var main = exports('default', null);

}
};
Expand Down
12 changes: 6 additions & 6 deletions test/form/samples/conditional-expression-paths/_expected.js
Expand Up @@ -7,10 +7,10 @@ var a2 = (unknownValue ? foo : baz).y.z;
var b2 = (unknownValue ? foo : baz).x();

// known branch with side-effect
var a4 = (baz).y.z;
var b4 = (baz).y.z;
var c4 = (baz).x();
var d4 = (baz).x();
var a4 = ( baz ).y.z;
var b4 = ( baz).y.z;
var c4 = ( baz ).x();
var d4 = ( baz).x();
var baz3 = {};
(baz3).y.z = 1;
(baz3).y.z = 1;
( baz3 ).y.z = 1;
( baz3).y.z = 1;
12 changes: 6 additions & 6 deletions test/form/samples/conditional-expression/_expected/amd.js
Expand Up @@ -11,11 +11,11 @@ define(function () { 'use strict';
(unknownValue ? function () {} : function () {this.x = 1;})();

// known side-effect
var h1 = foo();
var h2 = (function () {this.x = 1;})();
var h3 = (() => () => console.log( 'effect' ))()();
var i1 = foo();
var i2 = (function () {this.x = 1;})();
var i3 = (() => () => console.log( 'effect' ))()();
var h1 = foo() ;
var h2 = ( function () {this.x = 1;} )();
var h3 = ( () => () => console.log( 'effect' ) )()();
var i1 = foo();
var i2 = ( function () {this.x = 1;})();
var i3 = ( () => () => console.log( 'effect' ))()();

});
12 changes: 6 additions & 6 deletions test/form/samples/conditional-expression/_expected/cjs.js
Expand Up @@ -11,9 +11,9 @@ var d = unknownValue ? 1 : foo();
(unknownValue ? function () {} : function () {this.x = 1;})();

// known side-effect
var h1 = foo();
var h2 = (function () {this.x = 1;})();
var h3 = (() => () => console.log( 'effect' ))()();
var i1 = foo();
var i2 = (function () {this.x = 1;})();
var i3 = (() => () => console.log( 'effect' ))()();
var h1 = foo() ;
var h2 = ( function () {this.x = 1;} )();
var h3 = ( () => () => console.log( 'effect' ) )()();
var i1 = foo();
var i2 = ( function () {this.x = 1;})();
var i3 = ( () => () => console.log( 'effect' ))()();
12 changes: 6 additions & 6 deletions test/form/samples/conditional-expression/_expected/es.js
Expand Up @@ -9,9 +9,9 @@ var d = unknownValue ? 1 : foo();
(unknownValue ? function () {} : function () {this.x = 1;})();

// known side-effect
var h1 = foo();
var h2 = (function () {this.x = 1;})();
var h3 = (() => () => console.log( 'effect' ))()();
var i1 = foo();
var i2 = (function () {this.x = 1;})();
var i3 = (() => () => console.log( 'effect' ))()();
var h1 = foo() ;
var h2 = ( function () {this.x = 1;} )();
var h3 = ( () => () => console.log( 'effect' ) )()();
var i1 = foo();
var i2 = ( function () {this.x = 1;})();
var i3 = ( () => () => console.log( 'effect' ))()();
12 changes: 6 additions & 6 deletions test/form/samples/conditional-expression/_expected/iife.js
Expand Up @@ -12,11 +12,11 @@
(unknownValue ? function () {} : function () {this.x = 1;})();

// known side-effect
var h1 = foo();
var h2 = (function () {this.x = 1;})();
var h3 = (() => () => console.log( 'effect' ))()();
var i1 = foo();
var i2 = (function () {this.x = 1;})();
var i3 = (() => () => console.log( 'effect' ))()();
var h1 = foo() ;
var h2 = ( function () {this.x = 1;} )();
var h3 = ( () => () => console.log( 'effect' ) )()();
var i1 = foo();
var i2 = ( function () {this.x = 1;})();
var i3 = ( () => () => console.log( 'effect' ))()();

}());
12 changes: 6 additions & 6 deletions test/form/samples/conditional-expression/_expected/system.js
Expand Up @@ -14,12 +14,12 @@ System.register([], function () {
(unknownValue ? function () {} : function () {this.x = 1;})();

// known side-effect
var h1 = foo();
var h2 = (function () {this.x = 1;})();
var h3 = (() => () => console.log( 'effect' ))()();
var i1 = foo();
var i2 = (function () {this.x = 1;})();
var i3 = (() => () => console.log( 'effect' ))()();
var h1 = foo() ;
var h2 = ( function () {this.x = 1;} )();
var h3 = ( () => () => console.log( 'effect' ) )()();
var i1 = foo();
var i2 = ( function () {this.x = 1;})();
var i3 = ( () => () => console.log( 'effect' ))()();

}
};
Expand Down
12 changes: 6 additions & 6 deletions test/form/samples/conditional-expression/_expected/umd.js
Expand Up @@ -14,11 +14,11 @@
(unknownValue ? function () {} : function () {this.x = 1;})();

// known side-effect
var h1 = foo();
var h2 = (function () {this.x = 1;})();
var h3 = (() => () => console.log( 'effect' ))()();
var i1 = foo();
var i2 = (function () {this.x = 1;})();
var i3 = (() => () => console.log( 'effect' ))()();
var h1 = foo() ;
var h2 = ( function () {this.x = 1;} )();
var h3 = ( () => () => console.log( 'effect' ) )()();
var i1 = foo();
var i2 = ( function () {this.x = 1;})();
var i3 = ( () => () => console.log( 'effect' ))()();

}));
@@ -1,5 +1,5 @@
var a = (foo(), 3 );
var b = (bar(), 6 );
var a = ( foo(), 3 ) ;
var b = ( bar(), 6 );
foo( a, b );

// verify works with no whitespace
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Expand Up @@ -5,11 +5,11 @@ define(['exports'], function (exports) { 'use strict';
logicalAExp.bar = 1;

var bExp = {};
var logicalBExp = bExp;
var logicalBExp = bExp;
logicalBExp.bar = 1;

var cExp = {};
var logicalCExp = cExp;
var logicalCExp = cExp;
logicalCExp.bar = 1;

exports.aExp = aExp;
Expand Down
Expand Up @@ -7,11 +7,11 @@ var logicalAExp = aExp || {};
logicalAExp.bar = 1;

var bExp = {};
var logicalBExp = bExp;
var logicalBExp = bExp;
logicalBExp.bar = 1;

var cExp = {};
var logicalCExp = cExp;
var logicalCExp = cExp;
logicalCExp.bar = 1;

exports.aExp = aExp;
Expand Down
Expand Up @@ -3,11 +3,11 @@ var logicalAExp = aExp || {};
logicalAExp.bar = 1;

var bExp = {};
var logicalBExp = bExp;
var logicalBExp = bExp;
logicalBExp.bar = 1;

var cExp = {};
var logicalCExp = cExp;
var logicalCExp = cExp;
logicalCExp.bar = 1;

export { aExp, bExp, cExp };