-
-
Notifications
You must be signed in to change notification settings - Fork 2.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
New rules: Disallow or enforce spaces inside of curly braces and brackets (object-curly-spacing and array-bracket-spacing) #609
Comments
Agreed. Now that this is automatically fixable with |
I personally prefer without spaces. The reason is saving editor space-the line has only 80 characters and even two spaces can make a difference whether the line is overflowing or not. Many people would argue that it's more apparent when the braces are spaced-to those I would say-configure your editor to highlight them. I use a plugin https://marketplace.visualstudio.com/items?itemName=2gua.rainbow-brackets to make them more apparent and that works out better than trying to use spaces. |
🗒 There's already a ban on spaces within curly braces inside of template strings. |
All the documentation shows spaces in object in the rules list so that should be whats used since it looks like its implied that it is. |
@feross v9 is already behind us, really wish we could decide on this. I think this is the biggest part missing in standard and it's driving me nuts because there's still no consistency. In standard/eslint-config-standard#35 it looks like most people are leaning towards spaces around objects and not arrays. |
I really think it's time that we take a decision on this! This is something that constantly creeps up during code review for me, and the promise of Standard was that that shouldn't happen. Let's fix this 🙌 Here is the current eco-system impact:
I think we should just go with I'll send a PR now 🚀 |
Per discussion [here](standard/standard#609 (comment)) (standard/standard#609 (comment)) it seems like it's both time for this to happen, and that the community prefers `"always"`.
FWIW as I have been using standard I have gone back and forth between never and always since standard allows both. Over time I have ended up with the always style. |
This should be added as always, just because it's simply more readable and more consistent within object declarations. a: there is already a space after : it looks less awkward if u add all the spaces |
Thank you to everyone for sharing your thoughts on this issue. Thanks for sending the PR, @LinusU! Let's merge it and put this one behind us! 🙌 |
Hi, first of all thank for all the effort you put into this project, I've been using it for years now (before that I was on "npm style guide") and I love the minimalist code it enforce. With regard to this rule addition to standard you render your tool useless for almost all my projects and other developers that use javascript functional programming approach. The minimal object notation reduce a lot of space on one liners and make the functional flow clearer. const foo = ({in1, in2}) => ({out1: in1.reduce((acc, el, i) => (acc[el] = {x: in2[i], y: i}) && acc, {})}) This is conventional functional code example, readability is a matter of opinion, but I think that most would agreed that spacing out the objects won't make it any clearer. Just to clear that this not an opinionated view, check the docs for Ramda and Bobab - "the granddaddy" of functional programming in javascript: |
I don't think that this is true. I use functional programming a lot, and have no problems with having spaces in the curly objects.
I'm not sure I agree with this, the flow will still be exactly the same? 🤔 I personally think that the spacing makes it more clear where the objects begin and end, which I think makes it easier to see where the definition of const foo = ({in1, in2}) => ({out1: in1.reduce((acc, el, i) => (acc[el] = {x: in2[i], y: i}) && acc, {})})
const foo = ({ in1, in2 }) => ({ out1: in1.reduce((acc, el, i) => (acc[el] = { x: in2[i], y: i }) && acc, {}) }) Ultimately, we are of course going to upset a few people with this change, but I hope that you can see the benefit of unifying behind a single standard. As our testing indicated, most people already used spacing, and that's why we went with that... |
With that approach we should have stayed with semicolon 😄 But honestly, I really can't argue with the your decisions regarding the future of standard, just to note that as a long time community user I find this decision as contradiction to the reasons I choose to use standard. For now I'll stay on v11, but I hope that more people would raised concerns about this issue. |
I was referring to the tests I ran on existing packages using standard, so no semicolons there 😄
I'm not sure that we are aiming for minimalistic, the goal is to increase readability not remove as much as possible. e.g. this is still valid JS, but not too easy to read: const foo=({in1, in2})=>({out1:in1.reduce((acc,el,i)=>(acc[el]={x:in2[i],y:i})&&acc,{})})
Sounds like a good plan for now, if there is much push back on this we can of course reconsider. Although I don't think that many people will have a problem with it |
@LinusU It looks like we addressed the object-curly-spacing but the array-bracket-spacing is still not decided? We currently use standardx just to enforce these:
|
@flipxfx oh, I thought we already were using Let's run the test and see how breaking it would be to add, as far as I've seen, it's not common to add spaces to arrays. If you want to run the ecosystem impact test that would be much appreciated
|
|
@LinusU should we create a new issue for this? It would be a simple fix and it seems |
Yes please, sounds good 👍 |
http://eslint.org/docs/rules/object-curly-spacing
http://eslint.org/docs/rules/array-bracket-spacing
Standard JS should define if there should be spaces inside of brackets and curly braces or not.
By now, following code is allowed but looks awkward:
Personally I have no preference if there should be spaces or not.
The text was updated successfully, but these errors were encountered: