Make curried arrow function and normal arrow function consistent#14633
Conversation
|
@seiyab I've made changes I want. Please take a look, and feel free to make changes if you want. |
|
|
|
I didn't know curried-arrow-functions not calling |
|
I'll take a look. |
| const isArrowFunctionChain = (node) => | ||
| node.body.type === "ArrowFunctionExpression"; |
There was a problem hiding this comment.
[Question]
As of main, it looked to currentNode.body.type !== "ArrowFunctionExpression" || args.expandLastArg reading just rec() in printArrowFunction().
Do the difference cause a problem?
Though it doesn't cause any problem as far as I tried, I'm not sure about it because I'm not familiar with expandLastArg.
Example of input I tried:
fooooooooooooooooooooooo(action => a => b =>
(dispatch123456789098765432123451234567789(action), 1, 2)
);There was a problem hiding this comment.
Do the difference cause a problem?
I also worried about it, and it may cause problem, but I forgot about it...
Related logic here
We need check that.
My idea is get things out of rec(), so we don't need pass many arguments around.
There was a problem hiding this comment.
As my first impression, removing || args.expandLastArg int rec() looked to make sense.
But it will change some results, and previous format looks slightly better.
I'm still investigating.
- request.get(
- "https://preview-9992--prettier.netlify.app",
- (head) =>
- (body) =>
- (modyLoremIpsumDolorAbstractProviderFactoryServiceModule) => {
- console.log(head, body);
- },
- );
+ request.get("https://preview-9992--prettier.netlify.app", (head) =>
+ (body) =>
+ (modyLoremIpsumDolorAbstractProviderFactoryServiceModule) => {
+ console.log(head, body);
+ });There was a problem hiding this comment.
This yields same results regardless of || args.expandLastArg. So expandLastArg looks to work.
request.get("https://preview-9992--prettier.netlify.app", (head) => (body) => {
console.log(head, body);
})There was a problem hiding this comment.
I read the code again, when expandLastArg is true. functionBody can be an ArrowFunctionExpression, and functionBody decides how the body should be printed. Can't find a case to test. 98c36c2 (#14633) should restore the old behavior, I think.
There was a problem hiding this comment.
The following code can make functionBody be an ArrowFunctionExpression and effect shouldBreakChain
a("", "", ({}) => () => {
a;
});
- last argument is
ArrrowFunctionExpressionsatisfiescouldExpandArg - first two arguments satisfies
args.length !== 2inshouldExpandLastArg - first two arguments is very simple so they will pass
headArgs.some(willBreak)inprintCallArguments - the function parameter is not
Identifier, so it effectshouldBreakChain
There was a problem hiding this comment.
Finally find a case formats differently....
a("", "", ({}) => () => (a ? b : c));There was a problem hiding this comment.
# this one
>node bin/prettier test.js
a(
"",
"",
({}) =>
() => (a ? b : c),
);
# stable
>node node_modules/prettier/bin-prettier.js test.js
a(
"",
"",
({}) =>
() =>
a ? b : c
);
seiyab
left a comment
There was a problem hiding this comment.
Looks good, thanks!
|
@seiyab Can you make the following code work as v2? a(
"",
"",
({}) =>
() =>
() =>
() =>
() =>
() =>
() => (a ? b : c),
); |
|
I'll take a look. Thanks a lot for your detailed investigation. |
|
I don't understand it yet, but perhaps, we can say, current revision's behavior shows more consistent result with non-curried arrow function? a(
"",
"",
({}) =>
() =>
() =>
() =>
() =>
() =>
() => (a ? b : c),
);
a(
"",
"",
(
{},
argument,
argument,
argument,
argument,
argument,
argument,
argument,
argument,
) => (a ? b : c),
); |
|
But we put body on next line if it's an identifier |
|
Thank you for your review. (I don't mean to stop merging as long as you accept it. This comment is for information only.) |
|
We can always fix bugs when they are found. |
…ttier#14633) Co-authored-by: fisker Cheung <lionkay@gmail.com>
Description
Resolves #14290 .
What I did are...
printArrowChain()intoprintArrowFunction()to fix the issue and prevent other existing / future inconsistencies.printArrowFunction()(andprintArrowChain()) into newmayBreakAfterShortPrefix(),joinArrowFunctionSignatures()andprintArrowBody().I'm worrying that it might get somewhat harder to read and change.
But I don't have better ideas.
Old contents. Just for see history
Still in progress.
I think I need to
Edit: I'm busier than before. Go ahead if someone can go faster, and feel free to refer to or cherry pick my commits. I myself am still working on it.
Edit 2: I'm still working on it. I'm checking consistency more.
Checklist
(If changing the API or CLI) I’ve documented the changes I’ve made (in thedocs/directory).changelog_unreleased/*/XXXX.mdfile followingchangelog_unreleased/TEMPLATE.md.✨Try the playground for this PR✨