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

Consistent object destructuring spacing #2955

Closed
dcousens opened this issue Jul 8, 2015 · 18 comments
Closed

Consistent object destructuring spacing #2955

dcousens opened this issue Jul 8, 2015 · 18 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion triage An ESLint team member will look at this issue soon

Comments

@dcousens
Copy link

dcousens commented Jul 8, 2015

Is it possible a rule could be added for the following cases:

var { x} = ...  // ERROR: Inconsistent spacing
var {x } = ...  // ERROR: Inconsistent spacing
var { x } = ... // OK (1)
var {x} = ... // OK (2)

Extra options (though less important for my case) could also be to allow strict enforcement over option 1 or option 2 ONLY.

If such a rule already exists, my apologies.

@gyandeeps gyandeeps added the triage An ESLint team member will look at this issue soon label Jul 9, 2015
@nzakas
Copy link
Member

nzakas commented Jul 9, 2015

I believe object-curly-spacing should cover this case: http://eslint.org/docs/rules/object-curly-spacing

@feross
Copy link
Contributor

feross commented Jul 10, 2015

@nzakas object-curly-spacing forces to you pick one style var {x} = ... or var { x } = ....

I believe @dcousens is asking for an option to allow either style, while preventing the use of inconsistent spacing like var { x} = ... or var {x } = ....

@xjamundx
Copy link
Contributor

So the proposal is a new option on http://eslint.org/docs/rules/object-curly-spacing#options to be "either"?

@dcousens
Copy link
Author

@xjamundx yes, this would allow us to enforce consistency in an otherwise ambiguous environment where currently anything goes.

@nzakas
Copy link
Member

nzakas commented Jul 11, 2015

Out of curiosity, why not just pick one style?

@dcousens
Copy link
Author

@nzakas semver.
This would classify as a patch for existing wrong behaviour, but previously we hadn't set the rule as to whether there should be a space or not.

We intend to enforce one eventually, but right now, it would break too much code (as standard is used for tests).

@mysticatea
Copy link
Member

The rule is off by default?
Only when you turn the rule on by yourself, it warns your code.

@dcousens
Copy link
Author

@mysticatea I was referring to standard's versioning, not eslint.
This would be a patch for standard, if we enforced one or the other, it would be a breaking change.

@nzakas
Copy link
Member

nzakas commented Jul 13, 2015

@dcousens it sounds like you're asking us to implement a change that you only intend to use temporarily so as not to require a major version bump for standard right now. I don't think that's a good reason for us to make such a change.

@dcousens
Copy link
Author

@nzakas I don't think that is true, as the end result is that we may not enforce 1 style XOR** the other.

We want to enforce consistent spacing throughout standard, in either style, for an indeterminate amount of time, and this is the best way we can do that.

I know this change would also be beneficial for other projects I work on where we want consistency, but also do not enforce 1 style or the other (and nor do those projects use standard).

@lo1tuma
Copy link
Member

lo1tuma commented Jul 13, 2015

ESLint could only enforce consistency within a file. If you don't pick one style you might end with inconsistent styles across all project files.

@mysticatea
Copy link
Member

We can choose turn the rule on/off each project.

@dcousens
Copy link
Author

@lo1tuma I'd rather see { x } and {x}, consistently, than "anything goes" inconsistently.
I didn't mean only 1 style consistently, I meant both.

The end result [may be] that we may don't enforce 1 style XOR** the other.

@nzakas
Copy link
Member

nzakas commented Jul 14, 2015

@dcousens we generally don't have rules that allow two styles at the same time, as this typically is too confusing for users.

Unless someone on the ESLint team wants to champion this change, I'm leaning towards encouraging you to create a custom rule.

@dcousens
Copy link
Author

Thanks @nzakas, it won't be ideal for us, but I understand where you are coming from.

@feross shall I close?

@feross
Copy link
Contributor

feross commented Jul 14, 2015

Sure, I think that's totally fair.
On Mon, Jul 13, 2015 at 6:48 PM Daniel Cousens notifications@github.com
wrote:

Thanks @nzakas https://github.com/nzakas.

@feross https://github.com/feross shall I close?


Reply to this email directly or view it on GitHub
#2955 (comment).

@xjamundx
Copy link
Contributor

@dcousens, @feross if you want help making a plugin to add this functionality, let me know. it shouldn't be too hard!

@feross
Copy link
Contributor

feross commented Jul 14, 2015

@xjamundx yes, that would be a great help! want to move the discussion over to standard/standard#182 ?

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion triage An ESLint team member will look at this issue soon
Projects
None yet
Development

No branches or pull requests

7 participants