-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Space after function keyword - MOVED to #3847! #1139
Comments
Personally, I prefer to omit the space when I make an anonymous function? Now that I'm thinking about it, though, I don't know when I last wrote an anonymous |
FWIW, here's a diff of the change in a potential PR (at least me taking a crack at it!). |
I'd also write |
This also appears to be the only rule that conflicts with standard after the 1.0 release. |
It also conflicts with standards |
We're not going to change this. It's a very bike-sheddy debate and doesn't really matter. You might be used to seeing it the other way, but prettier has already made a choice and this and we're not going to change it. If you want to add a space there, you can run your code through prettier and than through an eslint config that adds it with |
Any chance of making this configurable ? This issue possibly breaks space-before-function-paren and generator-star-spacing for my eslint checker. |
@bhanuc What not just disable those rules? What do they gain you when using prettier? :) |
@bhanuc I would accept a PR for this in |
Many people are using standard with prettier i think this should be an option with prettier. |
The space before a function parentheses is a minor visual cue that the author is declaring a function, and not calling one - aiding readability. At the very least, as others have suggested, it should be an option. |
We strongly recommend disabling any formatting rules with eslint-config-prettier (just list it after standard and it'll turn them all of). We aren't going to add options for such small changes, as if we did, we'd have to add options for a lot of other things. It's good for the JS community to stabilize on a general style (and as far as I'm concerned the majority of JS does not use a space). It doesn't really help readability in my opinion, you just might need to retrain how you see that pattern. |
This causes linting issues on any project using vue-cli |
@srhise i think prettier was created with react in mind. |
Yeah, i'm not a huge fan of the default linting rules used in vue-cli. Very strict. |
This decision doesn't make a lot of sense to me. If the goal is to be opinionated with no bike-shedding, why are there any options at all? Seems like if there is even 1 option (ex. single vs double quotes) then other options should be up for discussion. Most people were using standard to fix their code before prettier came along. Though I don't care much which way we head (spaces or no spaces before parens), I'd like to see a consensus reached between the two projects. I think a nice separation would be to use prettier to format your code and standard to lint your code. Right now they're conflicting with each other and therefore need either double parsing and fixing (e.g. prettier-standard) or custom eslint configuration for each project, both of which aren't ideal. |
Now that u mentioned single vs double quotes option, i think something like space_before_function_paren options: boolean seems a good choice. unless it's hard to implement. |
This corresponds to ESLint's `space-before-function-paren` "always" option: https://eslint.org/docs/rules/space-before-function-paren#always See prettier#1139
* Add test demonstrating behavior on eslint space-before-function-paren sample code * Add --space-before-function-paren option This corresponds to ESLint's `space-before-function-paren` "always" option: https://eslint.org/docs/rules/space-before-function-paren#always See prettier#1139 * Document --space-before-function-paren option
I just added |
tl;drProviding an option for a space before function parens can improve project search quality. ArgumentSo I read through this thread and I understand both sides of the argument, but I want to throw out an additional note regarding spaces after functions that hasn't been mentioned. For me, the space after a function name is particularly useful for searching projects. In large projects with potentially hundreds of function calls to the same function, searching for a function definition can be challenging. Having a space after a function name can help narrow (and often point exactly to) the function definition: Helpful scenariosclass Person {
sayName () {
}
} function sayName () {
} module.exports = function sayName () {
} Less helpful scenarios// say-name.js
module.exports = function () {
// Say the name
} const sayName = function () {
} let sayName
sayName = function () {
} ConclusionMany functions are defined like the top 3 scenarios. I've found a space before function parens very useful to quickly find function definitions in a project, or at least narrow the search results to something that is more manageable. As shown above, there are several scenarios where this isn't as helpful, but ultimately I think that having an option in prettier would prove to not only be helpful to those who are coming from Standard, but also improving search quality. |
And to think I've been searching for |
VS Code Tip: For example: Here my default settings for prettier-now-vscode : "prettier.singleQuote": true,
"prettier.trailingComma": "none",
"prettier.semi": false,
"prettier.useTabs": false,
"prettier.tabWidth": 2,
"prettier.spaceBeforeFunctionParen": true |
I'm using VS Code + TypeScript + Vue.js + Prettier Plugin + JavaScript Standard Style in my project, and I experience this issue. Prettier format my code without space before function parenthesis, I know there is prettier-eslint can auto fix it, but it doesn't support tslint. And, typescript-eslint-parser is not stable yet. So I run After that I installed Vetur, an VS Code plugin from Vue.js team, it use Prettier to format script in *.vue, but because of Prettier doesn't support Finally, I disabled Maybe I'm selfish —— I apologize in advance if I'm wrong —— if Prettier support this rule, I think it's easy to avoid upon problems for people who use TypeScript, Vue.js and JavaScript Standard Style just like me. |
Current instruction:
If Prettier adds support for printing that space:
That’s no easier, is it? |
@lydell I completely agree. There is clearly a huge demand for this to be supported as an option. I recognize that there are inherent issues with supporting options and that Prettier intends to be opinionated. With that being said, I too believe that this should have been the default behavior. As you've stated, it has an actual practical use case (as well has interoperability with one of the most popular JavaScript style guides, Standard). I firmly believe that this should be supported as an option and should be considered in Prettier 2.0 (#3503). It's very disappointing that it has been constantly considered as impractical. |
@lydell It can. It doesn't work out of the box though, which can be a bad experience for developers who are having their first go with vue from something like react where they are using prettier just fine. |
They give you the following options: Pick an ESLint preset (Use arrow keys)
And since standard conflicts with prettier, lot's of people run into problems. I think people want the same thing: just set up linting without having to bother with it's configuration. |
Interesting question: Where does the space go when using TypeScript or Flow generics? function identity<T>(value: T): T {
return value;
} |
I’d say before: function identity <T>(value: T): T {
return value;
}
type Identity = <T>(value: T) => T
// but:
type Identity<T> = (value: T) => T \o/ TS highlighting is back because the |
Would anyone be offended if I made a PR adding an option for this so that I stop getting notifications about it? (tongue in cheek) |
Seems like a good case for an option. At least until 2.0. |
Haven't considered Typescript generics. Looking at the Typescript docs, and also the C# code I have, there is no space at all (but also no space for method declaration, which does annoy me). For visual comparison: Space Before function identity <T>(value: T): T {
return value;
}
type Identity = <T>(value: T) => T
// but:
type Identity<T> = (value: T) => T Space After function identity<T> (value: T): T {
return value;
}
type Identity<T> = (value: T) => T
// but:
type Identity = <T>(value: T) => T (╯°□°)╯︵ ┻━┻ I haven't got a reasoned position on this one 😞 |
Hmm, it has to be before, right? function identity <T>(value: T): T {
return value;
} Because then you can still search for |
Spaces should go between the function keyword and parens because of names https://github.com/airbnb/javascript/#functions--signature-spacing const fn = function () {} But not for method shorthand etc const o = {
fn() {}
} |
We need to be careful interpreting the 👍s on the OP. I suspect most are asking for space before anonymous function paren, as that is what OP describes. I don't think adding a space before all function parens will be a popular change. |
Read it again, it isn't. The request is for a space before function declaration parens. |
Why not? The whole argument is to determine the difference between declaration and invocation. So, method shorthand should absolutely include a space. It must be consistent, otherwise it's not worth doing at all. |
@justrhysism Yes it does.
When I say all, I mean: function foo () {
^ space here
} |
That was the example of existing functionality. An example of requested functionality wasn't provided, but was described. |
It is the input, and the expected output. The rest of the post describes anonymous functions and issues such as no spaces after keywords. |
No, it's an example of the existing behaviour.
The words "For example, prettier converts this ... into this: ..." indicate that the OP is describing the current behaviour. I'm really not sure how to make this any clearer. |
It is stated several times in OP that the issue is raised for anonymous functions only.
|
OP here.... 😄 @azz is totally right on my intention. I even reference a potential PR diff in where the current code only adds a space after the function keyword in the event that an id exists on that particular AST node. This seems goofy and reflects what I think is the biggest sticking point with this whole style; which is that there should always be a space after a keyword. In this case, it's the function keyword, but I think this rule can be applied to every keyword consistently as a general rule of thumb......hence the "prior art" I linked to, including the eslint rule and airbnb style guide which state it pretty clearly. By the way, for what it's worth, I also personally prefer no space after a function name. e.g. the style reflected in point 5 in @azz's previous comment. Lastly, I personally do not necessarily think this should be a configurable option. I originally opened this just before the 1.0 release, hoping that given enough feedback, it would become the default style (space after the function keyword). It would have been interesting to know how much code in the wild uses/used |
@sparty02 fair enough. Since you've been fairly absent from the thread, it appears to have run away without your input and the waters muddied. I suspect the vague issue title may have contributed to the confusion. Do you feel that a separate issue should be opened for a space after a function name declaration? |
@justrhysism Yeah, given my clarification, if there are strong feelings about the space after the function name declaration, I think a separate issue would be appropriate to address that. To be clear, I'm going to change the title of this from |
Okay, so I've created a separate issue (#3845) for the discussion on space after function name declaration. I believe both space after function (or indeed, any) keyword and space after function name declaration should be the default - or, at the very least, configurable. |
It's not the same thing. const fn = function() {}
const fn = functionname() {} // error, no space between keywords is a bad idea const fn = function () {}
const fn = function name() {} // all good const o = {
fn() {} // this is logically equivalent to `function name() {}` (no space between name and parens)
} |
@atomiks that's not at all what I was saying. Anyhow, the original title of this issue was space before function parens (since changed for clarification), which I believe is the source of much of the confusion. I'm not going to comment on this issue any further as that discussion should now take place over on #3845. |
I apologize in advance if this has been discussed before (I did a quick search and didn't find it). One of the things that popped out when running prettier over our codebase was spacing on anonymous functions.
For example, prettier converts this:
into this:
While this pattern is becoming less common for us (we'd probably use an arrow function here now), we still have several cases of this. The lack of space after the function keyword just doesn't read right for me. It probably has to do with one or more of the following:
For prior art:
Are there strong feelings on this one?
The text was updated successfully, but these errors were encountered: