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

[array-bracket-newline] Add option consistent to the object configuration #13582

Closed
mdziekon opened this issue Aug 18, 2020 · 3 comments
Closed
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules

Comments

@mdziekon
Copy link
Contributor

What rule do you want to change?

array-bracket-newline

Does this change cause the rule to produce more or fewer warnings?

By default: no change.
When option enabled: fewer.

How will the change be implemented? (New option, new default behavior, etc.)?

New option

Please provide some example code that this change will affect:

// .eslintrc (fragment)
{
    'rules': [
        'array-bracket-newline': {
            'error',
            {
                'multiline': true,
            }
        }
    ]
}
const test1 = [
    'test',
];

const test2 = [ 'test' ];

const test3 = [
    'test1',
    'test2',
    'test3',
];

What does the rule currently do for this code?

It reports two errors for the test1 array:

1:15 error There should be no linebreak after '[' array-bracket-newline
3:1 error There should be no linebreak before ']' array-bracket-newline

What will the rule do after it's changed?

It won't report any errors, meaning that it will allow for an array to either be multiline with new lines between elements, or force consistent line breaks for opening and closing brackets.

Important note: this proposal is NOT about changing the behaviour of the already existing consistent (string) variant of this rule, but about a new property to the object-like configuration. Please read my explanation in its entirely before jumping to conclusions about possible redundancy.

The reasoning behind this is as follows:

  • The multiline: true option forces to either write all elements of an array in one line (one after another), or split them into multiple lines. This behaviour is already implemented and well-documented (... requires line breaks if there are line breaks inside elements or between elements.). This however, does not allow one-line element enumerations (whether it's just one or multiple elements) to be started from a new line (when looking at the bracket opening -> first element), because now the rule does not consider this situation to require line breaks, and in turn forbids line breaks entirely.
    • The string-like option [ 'error', 'consistent' ] is not able to achieve the same effect, because even though it allows for one-liners to start from a new line, it only checks the brackets, disregarding whether there are any line breaks between elements. multiline checks both brackets and elements line breaks. To better visualise this, using only the string-like option consistent treats this code as valid:
    const test = [ 'test',
        'test',
        'test',
        'test' ];
  • The consistent: true option would additionally force to consistently open & close brackets in the same line as all the elements, or in new lines, regardless of whether the elements are actually split or not (as a whole). Therefore, it would not affect the multiline option, but rather provide additional constraint with accordance to how the object-shaped configuration for this rule is supposed to work (... Requires line breaks if any of properties is satisfied.).
  • Adding the consistent option to the object-shaped configuration would additionally bring this rule closer to how object-curly-newline works (which already does provide what I propose here).

To prevent breaking changes, this option would have to be set to false by default. For consistency with object-curly-newline, it could be put into consideration to enable it by default, but that's a matter for another PR.

There was a discussion similar to this one in the past (#12736), however I believe that either the author did not describe their intent correctly, or the ESLint's contributors were too absorbed with the proposed "multiline option extension", so hopefully my explanation will bring a new perspective to this old proposal.

Are you willing to submit a pull request to implement this change?

Yes, I believe this can be achieved in a similar fashion to how object-curly-newline does this and can do it in my spare time.

@mdziekon mdziekon added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules triage An ESLint team member will look at this issue soon labels Aug 18, 2020
@anikethsaha
Copy link
Member

Thanks for the issue.

array-bracket-newline is a stylistic rule and all stylistic rules are currently [frozen as per our new policy] that means no new enhancement would be accepted only bug reports.

You can always create a custom rule/plugin that can extend this rule and add the new option.

@anikethsaha anikethsaha removed the triage An ESLint team member will look at this issue soon label Aug 18, 2020
@mdziekon
Copy link
Contributor Author

Thanks for the answer. Even though I've already closed the issue as this is basically a lost cause here, I still have a question.

Is there currently any equivalent of ESLint's stylistic rules already forked in a form of a plugin, or did anyone from the maintainers consider doing this, even in an unofficial manner to jump-start this kind of project? The biggest problem with this new approach that I can see is the "community maintainability" of such plugins, or rather, new plugins like this one would be - even though they would be based on existing, ESLint's rules, they would still need to gain some traction in order to thrive. Obviously, even ESLint had to deal with this in the past, but if it would be possible to make it a bit easier for new plugins based on the old, frozen rules, it could potentially speed up community building process. Also, don't get me wrong - this idea of mine is not about making maintainers of ESLint to maintain yet another project, but rather to make a statement to the community that "this code was once maintained by us, we don't want to do this anymore in core, but this is a place where you can continue extending it out of core".

@anikethsaha
Copy link
Member

As of now, I don't think atm there is any plugin that is managing the core stylistic rules (not sure tho.).

Stylistic rules are maintained here but no new feature will be implemented. I don't think there is any interest to have an official plugin that would maintain that atm. Most of the users use eslint-plugin-prettier to manage the style across the project.

But yeah, extending a rule is really simple so I guess maybe someone will do so.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 15, 2021
@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 15, 2021
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 enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

2 participants