Simple arrow function parens option #3324
Conversation
docs/options.md
Outdated
| @@ -96,6 +96,19 @@ Put the `>` of a multi-line JSX element at the end of the last line instead of b | |||
| | ------- | ------------------------- | ---------------------------- | | |||
| | `false` | `--jsx-bracket-same-line` | `jsxBracketSameLine: <bool>` | | |||
|
|
|||
| # Arrow Function Parentheses | |||
There was a problem hiding this comment.
This should probably be a ## heading? 😅
There was a problem hiding this comment.
LGTM
✨
Thanks for doing this. Disappointed to miss the callbacks option this time around, of course, but I agree it's best to proceed with this for now.
We can wait and see if there's community demand and maybe merge in those changes later.
I don't think there's any lingering doubts about what you have here, so I think it's fine to merge.
I'll be happy to do so within a couple days if I don't hear concerns.
src/printer.js
Outdated
| return canPrintParamsWithoutParens(node); | ||
| } | ||
|
|
||
| return false; |
There was a problem hiding this comment.
Probably worth adding a comment indicating that this should be unreachable
There was a problem hiding this comment.
or just
function shouldPrintParamsWithoutParens(path, options) {
if (options.arrowFunctionParentheses === "avoid") {
const node = path.getValue();
return canPrintParamsWithoutParens(node);
}
// possible comment here to explain it :)
return false;
}There was a problem hiding this comment.
That works too, though I like the explicitness (and having something to grep for, and having a safe default-fallback).
|
Oops, looks like you have some Travis failures (ironically looks like prettier needs to be run; do we have a command for that?). Also need to fix the docs issue wKovacs64 mentioned. |
|
Obligatory bikeshed... |
|
(Aside: should we continue using milestones, or use release labels?) |
|
Let's use milestones instead of release labels; I forgot that milestones existed, and they're nicer for this kind of thing. |
These new snapshots are from prettier#2676
Based on prettier#2676 * Thread `path` and `options` through helpers so we don't need to add `needsParens` onto the AST node anymore (mutation) * Pull test call detection logic out into helper method so it can be re-used for arrow function parens * Add arrow function parens option implementation (avoid/always) * Don't break arrow function parens around (done) in test call
Whoops, forgot this
suchipi
force-pushed
the
arrow_parens
branch
from
73ac57e
to
41a9aba
Compare
|
Looks like I'll need to add the option to the website, too. |
|
And we should have a way to coordinate pre-release playground options too... |
|
Any news on adding a |
|
@qleguennec doubtful; personally, I've been using the "always" option and am much happier with it than I expected. I don't mind the extra parens when the computer does all the typing 😀 Which are you currently using / why is it problematic? |
|
@rattrayalex My company uses eslint with some default values, including the |
|
@qleguennec You might be able to work around this using prettier-eslint. |
lock
bot
added
the
locked-due-to-inactivity
This is based on #2676 by @rattrayalex. Originally, I was going to rebase that branch and make changes there, but rebasing proved difficult due to unrelated changes in master.
This PR adds an "--arrow-function-parentheses" choice option with two options: "always" and "avoid". Avoid is the same as now (try not to print them if you can). Always is the new option (always print them). Avoid is the default, for backwards-compatibility.
Unlike #2676, this does not include the callbacks option AKA functional. Even though I find it interesting, I don't think there is community/collaborator buy-in or consensus, and I don't want to not add "always" because of that.
This PR also includes a fix that @rattrayalex started in his branch to inline type parameters on test callback functions (
it("very long description", <T>(done) => { /* ... */ })).