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

fix: parenthesizes around arrow function body returning object #744

Closed

Conversation

coderaiser
Copy link
Contributor

Fixes #743. Using recast with babel-parser and traverser break code with arrow function expression returning an object.

Code:

(a) => ({a: 'b'});

Becomes:

() => {a: 'b'}

After removing function argument from @babel/traverse.

@coderaiser coderaiser force-pushed the fix/arrow-body-parenthesizes branch from 71d09fb to c3507bc Compare August 30, 2020 08:43
@eventualbuddha
Copy link
Collaborator

This seems okay at first glance, but then I don’t understand why it isn’t already handled by this:

recast/lib/fast-path.ts

Lines 500 to 507 in 8cc1f42

case "ObjectExpression":
if (
parent.type === "ArrowFunctionExpression" &&
name === "body" &&
parent.body === node
) {
return true;
}
.

@coderaiser
Copy link
Contributor Author

I don’t understand why it isn’t already handled by this

Looks like it doesn't handled by ObjectExpression switch case from fast-path, because it is:

  • not get intoObjectExpression switch in the printer, it get only to ArrowFunctionExpression
  • so if fast-path ArrowFunctionExpression switch-case will be updated, result will be something like this:
(b => {a: 'b'})
  • fast-path stack doesn't have ObjectExpression, it ends on ArrowFunctionExpression

Screen Shot 2020-08-31 at 11 39 48 AM

@coderaiser
Copy link
Contributor Author

Looks like this code https://github.com/benjamn/recast/blob/v0.20.3/lib/patcher.ts#L478-L494 returns false and exits loop when it checks params, so there is no recursive execution for body which is ObjectExpression and no check for parenthesis.

@coderaiser
Copy link
Contributor Author

any progress on this?

@coderaiser
Copy link
Contributor Author

Would be amazing to merge this, or fix in any other way, so I don't have to support my own fork of recast accessible by npm i @putout/recast, where this bug is fixed already :).

@zxbodya
Copy link
Contributor

zxbodya commented Dec 16, 2020

Hi,
caught the same issue, and probably found the change causing it - made a pr reverting that change and adding a test from this PR - #830

@conartist6
Copy link
Contributor

Yep, it was me that broke this, and I also have a duplicate issue and a good fix. I'm hoping to get the attention of @benjamn to resolve this, and if not I'm actually more than happy to maintain a fork so one way or another I think we can move forward on this finally!

@conartist6
Copy link
Contributor

I'm also working full time on tooling that will make it significantly easier to develop packages in a decentralized way, that is to say without the single point of failure of maintainers who are the sole holders of publish perms to the central npm repo and whose focus tends to shift for perfectly natural and understandable reasons.

@gnprice
Copy link
Contributor

gnprice commented May 25, 2022

The issue appears to have been fixed now by #1068, so I believe this PR can be closed.

@eventualbuddha
Copy link
Collaborator

Verified this no longer reproduces. Thanks for the nudge, @gnprice.

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.

Brocken parenthesizes around arrow function body when return an object
5 participants