Skip to content

Commit

Permalink
Share argument rendering logic between call and new expression
Browse files Browse the repository at this point in the history
Solves an issue where trying to render a non-included argument crashes
  • Loading branch information
lukastaegert committed Jun 1, 2022
1 parent 0da6a80 commit aea0df3
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 36 deletions.
38 changes: 3 additions & 35 deletions src/ast/nodes/CallExpression.ts
@@ -1,11 +1,8 @@
import type MagicString from 'magic-string';
import type { NormalizedTreeshakingOptions } from '../../rollup/types';
import { BLANK } from '../../utils/blank';
import {
findFirstOccurrenceOutsideComment,
type NodeRenderOptions,
type RenderOptions
} from '../../utils/renderHelpers';
import { renderCallArguments } from '../../utils/renderCallArguments';
import { type NodeRenderOptions, type RenderOptions } from '../../utils/renderHelpers';
import type { DeoptimizableEntity } from '../DeoptimizableEntity';
import type { HasEffectsContext, InclusionContext } from '../ExecutionContext';
import { EVENT_CALLED } from '../NodeEvents';
Expand Down Expand Up @@ -116,36 +113,7 @@ export default class CallExpression extends CallExpressionBase implements Deopti
isCalleeOfRenderedParent: true,
renderedSurroundingElement
});
if (this.arguments.length > 0) {
if (this.arguments[this.arguments.length - 1].included) {
for (const arg of this.arguments) {
arg.render(code, options);
}
} else {
let lastIncludedIndex = this.arguments.length - 2;
while (lastIncludedIndex >= 0 && !this.arguments[lastIncludedIndex].included) {
lastIncludedIndex--;
}
if (lastIncludedIndex >= 0) {
for (let index = 0; index <= lastIncludedIndex; index++) {
this.arguments[index].render(code, options);
}
code.remove(
findFirstOccurrenceOutsideComment(
code.original,
',',
this.arguments[lastIncludedIndex].end
),
this.end - 1
);
} else {
code.remove(
findFirstOccurrenceOutsideComment(code.original, '(', this.callee.end) + 1,
this.end - 1
);
}
}
}
renderCallArguments(code, options, this);
}

protected applyDeoptimizations(): void {
Expand Down
8 changes: 8 additions & 0 deletions src/ast/nodes/NewExpression.ts
@@ -1,4 +1,7 @@
import MagicString from 'magic-string';
import type { NormalizedTreeshakingOptions } from '../../rollup/types';
import { renderCallArguments } from '../../utils/renderCallArguments';
import { RenderOptions } from '../../utils/renderHelpers';
import type { CallOptions } from '../CallOptions';
import type { HasEffectsContext } from '../ExecutionContext';
import { InclusionContext } from '../ExecutionContext';
Expand Down Expand Up @@ -54,6 +57,11 @@ export default class NewExpression extends NodeBase {
};
}

render(code: MagicString, options: RenderOptions) {
this.callee.render(code, options);
renderCallArguments(code, options, this);
}

protected applyDeoptimizations(): void {
this.deoptimized = true;
for (const argument of this.arguments) {
Expand Down
41 changes: 41 additions & 0 deletions src/utils/renderCallArguments.ts
@@ -0,0 +1,41 @@
import MagicString from 'magic-string';
import CallExpression from '../ast/nodes/CallExpression';
import NewExpression from '../ast/nodes/NewExpression';
import { findFirstOccurrenceOutsideComment, RenderOptions } from './renderHelpers';

export function renderCallArguments(
code: MagicString,
options: RenderOptions,
node: CallExpression | NewExpression
): void {
if (node.arguments.length > 0) {
if (node.arguments[node.arguments.length - 1].included) {
for (const arg of node.arguments) {
arg.render(code, options);
}
} else {
let lastIncludedIndex = node.arguments.length - 2;
while (lastIncludedIndex >= 0 && !node.arguments[lastIncludedIndex].included) {
lastIncludedIndex--;
}
if (lastIncludedIndex >= 0) {
for (let index = 0; index <= lastIncludedIndex; index++) {
node.arguments[index].render(code, options);
}
code.remove(
findFirstOccurrenceOutsideComment(
code.original,
',',
node.arguments[lastIncludedIndex].end
),
node.end - 1
);
} else {
code.remove(
findFirstOccurrenceOutsideComment(code.original, '(', node.callee.end) + 1,
node.end - 1
);
}
}
}
}
2 changes: 1 addition & 1 deletion test/form/samples/side-effect-es5-classes/_expected.js
Expand Up @@ -22,7 +22,7 @@ var Arrow = ( x ) => {
console.log( 'before' );
new Bar(5);
new Baz(5);
new Qux(5);
new Qux();
Corge(5);
new Arrow(5);

Expand Down
@@ -0,0 +1,8 @@
const assert = require('assert');

module.exports = {
description: 'handles unused logical expressions as constructor arguments (#4517)',
exports({ create }) {
assert.strictEqual(create().foo, 'foo');
}
};
@@ -0,0 +1,7 @@
function Note() {
this.foo = 'foo';
}

export function create(data) {
return new Note(data || {});
}

0 comments on commit aea0df3

Please sign in to comment.