Skip to content

Reproduce issue #426#427

Draft
lucivpav wants to merge 1 commit into
estools:masterfrom
lucivpav:bug-426
Draft

Reproduce issue #426#427
lucivpav wants to merge 1 commit into
estools:masterfrom
lucivpav:bug-426

Conversation

@lucivpav

@lucivpav lucivpav commented Oct 7, 2020

Copy link
Copy Markdown

Reproduces #426

@Meir017

Meir017 commented Oct 12, 2020

Copy link
Copy Markdown

@lucivpav I just ran into this issue too.
I found a solution for this -

escodegen/escodegen.js

Lines 946 to 949 in ab53cd5

expr = this.generateExpression(node.body, Precedence.Assignment, E_TTT);
if (expr.toString().charAt(0) === '{') {
expr = ['(', expr, ')'];
}

this should be

var strExpr = expr.toString();
if (strExpr[0] === '{' || strExpr[strExpr.length - 1] === '}') {
  expr = ['(', expr, ')'];
}

I'm pretty sure the last character cannot be part of the comment, in that case the comment would not be part of the node

@lucivpav

Copy link
Copy Markdown
Author

I wonder if this fix works for the affected parenthesized expressions in general, or just in function bodies 🤔

@Meir017

Meir017 commented Oct 14, 2020

Copy link
Copy Markdown

@lucivpav a better solution is to check the node types:

if (node.type === Syntax.ArrowFunctionExpression && node.body.type === Syntax.ObjectExpression) {
  expr = ['(', expr, ')'];
}

@Itazulay

Copy link
Copy Markdown

@lucivpav This PR does not cover this scenario:
var a = b => ({}.hasOwnProperty.call(b, "c"));
This is a CallExpression and not an ObjectExpression.
Maybe it should be simply:
if (node.type === Syntax.ArrowFunctionExpression) { expr = ['(', expr, ')']; }

@lucivpav

Copy link
Copy Markdown
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants