diff --git a/src/ast/nodes/CallExpression.ts b/src/ast/nodes/CallExpression.ts index dc363ec0002..14fdb7c6002 100644 --- a/src/ast/nodes/CallExpression.ts +++ b/src/ast/nodes/CallExpression.ts @@ -247,11 +247,10 @@ export default class CallExpression extends NodeBase implements DeoptimizableEnt { renderedParentType, renderedSurroundingElement }: NodeRenderOptions = BLANK ): void { const surroundingELement = renderedParentType || renderedSurroundingElement; - this.callee.render( - code, - options, - surroundingELement ? { renderedSurroundingElement: surroundingELement } : BLANK - ); + this.callee.render(code, options, { + isCalleeOfRenderedParent: true, + renderedSurroundingElement: surroundingELement + }); if (this.arguments.length > 0) { if (this.arguments[this.arguments.length - 1].included) { for (const arg of this.arguments) { diff --git a/src/ast/nodes/ConditionalExpression.ts b/src/ast/nodes/ConditionalExpression.ts index 64b49b0c6fa..e5040b58b56 100644 --- a/src/ast/nodes/ConditionalExpression.ts +++ b/src/ast/nodes/ConditionalExpression.ts @@ -19,7 +19,6 @@ import { SHARED_RECURSION_TRACKER, UNKNOWN_PATH } from '../utils/PathTracker'; -import CallExpression from './CallExpression'; import * as NodeType from './NodeType'; import SpreadElement from './SpreadElement'; import { ExpressionEntity, LiteralValueOrUnknown, UnknownValue } from './shared/Expression'; @@ -180,7 +179,12 @@ export default class ConditionalExpression extends NodeBase implements Deoptimiz render( code: MagicString, options: RenderOptions, - { isCalleeOfRenderedParent, renderedParentType, preventASI }: NodeRenderOptions = BLANK + { + isCalleeOfRenderedParent, + preventASI, + renderedParentType, + renderedSurroundingElement + }: NodeRenderOptions = BLANK ): void { const usedBranch = this.getUsedBranch(); if (!this.test.included) { @@ -200,15 +204,15 @@ export default class ConditionalExpression extends NodeBase implements Deoptimiz } removeAnnotations(this, code); usedBranch!.render(code, options, { - isCalleeOfRenderedParent: renderedParentType - ? isCalleeOfRenderedParent - : (this.parent as CallExpression).callee === this, + isCalleeOfRenderedParent, preventASI: true, - renderedParentType: renderedParentType || this.parent.type + ...(renderedSurroundingElement + ? { renderedSurroundingElement } + : { renderedParentType: renderedParentType || this.parent.type }) }); } else { this.test.render(code, options, { - renderedSurroundingElement: renderedParentType + renderedSurroundingElement: renderedParentType || renderedSurroundingElement }); this.consequent.render(code, options); this.alternate.render(code, options); diff --git a/src/ast/nodes/LogicalExpression.ts b/src/ast/nodes/LogicalExpression.ts index 46e3516d78e..e4f694dc17d 100644 --- a/src/ast/nodes/LogicalExpression.ts +++ b/src/ast/nodes/LogicalExpression.ts @@ -19,7 +19,6 @@ import { SHARED_RECURSION_TRACKER, UNKNOWN_PATH } from '../utils/PathTracker'; -import CallExpression from './CallExpression'; import * as NodeType from './NodeType'; import { ExpressionEntity, LiteralValueOrUnknown, UnknownValue } from './shared/Expression'; import { MultiExpression } from './shared/MultiExpression'; @@ -190,11 +189,11 @@ export default class LogicalExpression extends NodeBase implements Deoptimizable } removeAnnotations(this, code); this.getUsedBranch()!.render(code, options, { - isCalleeOfRenderedParent: renderedParentType - ? isCalleeOfRenderedParent - : (this.parent as CallExpression).callee === this, + isCalleeOfRenderedParent, preventASI, - renderedParentType: renderedParentType || this.parent.type + ...(renderedSurroundingElement + ? { renderedSurroundingElement } + : { renderedParentType: renderedParentType || this.parent.type }) }); } else { this.left.render(code, options, { diff --git a/src/ast/nodes/MemberExpression.ts b/src/ast/nodes/MemberExpression.ts index 7b6dfc704f9..88613f0896a 100644 --- a/src/ast/nodes/MemberExpression.ts +++ b/src/ast/nodes/MemberExpression.ts @@ -281,17 +281,15 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE renderedSurroundingElement }: NodeRenderOptions = BLANK ): void { - const isCalleeOfDifferentParent = - renderedParentType === NodeType.CallExpression && isCalleeOfRenderedParent; if (this.variable || this.replacement) { let replacement = this.variable ? this.variable.getName() : this.replacement; - if (isCalleeOfDifferentParent) replacement = '0, ' + replacement; + if (renderedParentType && isCalleeOfRenderedParent) replacement = '0, ' + replacement; code.overwrite(this.start, this.end, replacement!, { contentOnly: true, storeName: true }); } else { - if (isCalleeOfDifferentParent) { + if (renderedParentType && isCalleeOfRenderedParent) { code.appendRight(this.start, '0, '); } const surroundingElement = renderedParentType || renderedSurroundingElement; diff --git a/src/ast/nodes/SequenceExpression.ts b/src/ast/nodes/SequenceExpression.ts index 64fac143d0e..5e833f6a7d0 100644 --- a/src/ast/nodes/SequenceExpression.ts +++ b/src/ast/nodes/SequenceExpression.ts @@ -10,9 +10,8 @@ import { treeshakeNode } from '../../utils/treeshakeNode'; import { CallOptions } from '../CallOptions'; import { DeoptimizableEntity } from '../DeoptimizableEntity'; import { HasEffectsContext, InclusionContext } from '../ExecutionContext'; -import { EVENT_CALLED, NodeEvent } from '../NodeEvents'; +import { NodeEvent } from '../NodeEvents'; import { ObjectPath, PathTracker } from '../utils/PathTracker'; -import CallExpression from './CallExpression'; import ExpressionStatement from './ExpressionStatement'; import * as NodeType from './NodeType'; import { ExpressionEntity, LiteralValueOrUnknown } from './shared/Expression'; @@ -23,7 +22,7 @@ export default class SequenceExpression extends NodeBase { type!: NodeType.tSequenceExpression; deoptimizePath(path: ObjectPath): void { - if (path.length > 0) this.expressions[this.expressions.length - 1].deoptimizePath(path); + this.expressions[this.expressions.length - 1].deoptimizePath(path); } deoptimizeThisOnEventAtPath( @@ -32,14 +31,12 @@ export default class SequenceExpression extends NodeBase { thisParameter: ExpressionEntity, recursionTracker: PathTracker ): void { - if (event === EVENT_CALLED || path.length > 0) { - this.expressions[this.expressions.length - 1].deoptimizeThisOnEventAtPath( - event, - path, - thisParameter, - recursionTracker - ); - } + this.expressions[this.expressions.length - 1].deoptimizeThisOnEventAtPath( + event, + path, + thisParameter, + recursionTracker + ); } getLiteralValueAtPath( @@ -69,9 +66,9 @@ export default class SequenceExpression extends NodeBase { } hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean { - return ( - path.length === 0 || - this.expressions[this.expressions.length - 1].hasEffectsWhenAssignedAtPath(path, context) + return this.expressions[this.expressions.length - 1].hasEffectsWhenAssignedAtPath( + path, + context ); } @@ -123,13 +120,17 @@ export default class SequenceExpression extends NodeBase { if (includedNodes === 1 && preventASI) { removeLineBreaks(code, start, node.start); } - if (node === lastNode && includedNodes === 1) { - node.render(code, options, { - isCalleeOfRenderedParent: renderedParentType - ? isCalleeOfRenderedParent - : (this.parent as CallExpression).callee === this, - renderedParentType: renderedParentType || this.parent.type - }); + if (includedNodes === 1) { + if (node === lastNode) { + node.render(code, options, { + isCalleeOfRenderedParent, + renderedParentType: renderedParentType || this.parent.type + }); + } else { + node.render(code, options, { + renderedSurroundingElement: renderedParentType || this.parent.type + }); + } } else { node.render(code, options); } diff --git a/src/ast/nodes/TaggedTemplateExpression.ts b/src/ast/nodes/TaggedTemplateExpression.ts index 5679372c7f4..012eceb05dd 100644 --- a/src/ast/nodes/TaggedTemplateExpression.ts +++ b/src/ast/nodes/TaggedTemplateExpression.ts @@ -1,3 +1,5 @@ +import MagicString from 'magic-string'; +import { RenderOptions } from '../../utils/renderHelpers'; import { CallOptions, NO_ARGS } from '../CallOptions'; import { HasEffectsContext } from '../ExecutionContext'; import { EMPTY_PATH } from '../utils/PathTracker'; @@ -28,17 +30,6 @@ export default class TaggedTemplateExpression extends NodeBase { this.start ); } - - if (name === 'eval') { - this.context.warn( - { - code: 'EVAL', - message: `Use of eval is strongly discouraged, as it poses security risks and may cause issues with minification`, - url: 'https://rollupjs.org/guide/en/#avoiding-eval' - }, - this.start - ); - } } } @@ -56,4 +47,9 @@ export default class TaggedTemplateExpression extends NodeBase { withNew: false }; } + + render(code: MagicString, options: RenderOptions): void { + this.tag.render(code, options, { isCalleeOfRenderedParent: true }); + this.quasi.render(code, options); + } } diff --git a/test/form/samples/conditional-expression/_expected.js b/test/form/samples/conditional-expression/_expected.js index e9277e08e7a..7250914dc0c 100644 --- a/test/form/samples/conditional-expression/_expected.js +++ b/test/form/samples/conditional-expression/_expected.js @@ -10,8 +10,8 @@ unknownValue ? 1 : foo(); // known side-effect foo() ; -(function () {this.x = 1;} )(); +((function () {this.x = 1;}) )(); (() => () => console.log( 'effect' ) )()(); foo(); -(function () {this.x = 1;})(); +((function () {this.x = 1;}))(); (() => () => console.log( 'effect' ))()(); diff --git a/test/function/samples/sequence-expressions-template-tag/_config.js b/test/function/samples/sequence-expressions-template-tag/_config.js new file mode 100644 index 00000000000..6af722b1760 --- /dev/null +++ b/test/function/samples/sequence-expressions-template-tag/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'uses corrrect "this" for tagged template expressions in simplified sequences' +}; diff --git a/test/function/samples/sequence-expressions-template-tag/main.js b/test/function/samples/sequence-expressions-template-tag/main.js new file mode 100644 index 00000000000..7bcdd059109 --- /dev/null +++ b/test/function/samples/sequence-expressions-template-tag/main.js @@ -0,0 +1,13 @@ +let o = { + f() { + assert.ok(this !== o); + } +}; +(1, o.f)(); +(1, o.f)``; + +(true && o.f)(); +(true && o.f)``; + +(true ? o.f : false)(); +(true ? o.f : false)``; diff --git a/test/function/samples/simplify-sequence-expressions/_config.js b/test/function/samples/simplify-sequence-expressions/_config.js new file mode 100644 index 00000000000..cb5578b64ab --- /dev/null +++ b/test/function/samples/simplify-sequence-expressions/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: + 'correctly simplify sequence expressions if the new leading element would require parentheses' +}; diff --git a/test/function/samples/simplify-sequence-expressions/main.js b/test/function/samples/simplify-sequence-expressions/main.js new file mode 100644 index 00000000000..fbc070d97f1 --- /dev/null +++ b/test/function/samples/simplify-sequence-expressions/main.js @@ -0,0 +1,31 @@ +let value = 0; + +1, function(){ value = 1 }(); +assert.strictEqual(value, 1); + +1, function(){ value = 2 }(), 2; +assert.strictEqual(value, 2); + +1, {foo: value = 3 }; +assert.strictEqual(value, 3); + +1, {foo: value = 4 }, 2; +assert.strictEqual(value, 4); + +1, true ? function(){ value = 5 }() : false; +assert.strictEqual(value, 5); + +1, true ? function(){ value = 6 }() : false, 2; +assert.strictEqual(value, 6); + +1, function(){ value = 7; return true }() ? true : false; +assert.strictEqual(value, 7); + +1, function(){ value = 8; return true }() ? true : false, 2; +assert.strictEqual(value, 8); + +1, false || function(){ value = 9 }(); +assert.strictEqual(value, 9); + +1, false || function(){ value = 10 }(), 2; +assert.strictEqual(value, 10);