[sort-imports] Allow the users to group their imports in a simple way

[dmitov]

What rule do you want to change?
sort-imports

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

How will the change be implemented? (New option, new default behavior, etc.)?
Add a new option called ignoreBlankLines, defaults to true, which when set to false will not trigger the alphabetical sort if the previous line is blank.

Please provide some example code that this change will affect:

🔴Examples of incorrect code for this rule with the { "ignoreBlankLines": true } option:

/*eslint sort-imports: "error" */ 
import b from 'foo.js';
import c from 'baz.js';

import a from 'bar.js';
import x from 'qux.js';

🟢Examples of correct code for this rule with the { "ignoreBlankLines": false } option:

/*eslint sort-imports: ["error", { "ignoreBlankLines": false }]*/
import b from 'foo.js';
import c from 'baz.js';

import a from 'bar.js';
import x from 'qux.js';

What does the rule currently do for this code?
Currently the rule doesn't respect blank lines, meaning that even if there is a line break between imports, it will still error out that they're not sorted alphabetically.

What will the rule do after it's changed?
After the change, the new option will allow grouping of imports, each of which will be alphabetically sort.

Example:

/*eslint sort-imports: ["error", { "ignoreBlankLines": false }]*/
import React from 'react';
import moment from 'moment';

// Components
import Footer from '@app/footer.js';
import Header from '@app/header.js';

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

[mdjermanovic]

Should the option look specifically for blank lines somewhere between import lines, or allow all cases where imports are not on consecutive lines:

/*eslint sort-imports: ["error", { "ignoreBlankLines": false }]*/

import b from 'foo.js';
import c from 'baz.js';
// comment
import a from 'bar.js'; // error?
import x from 'qux.js';

/*eslint sort-imports: ["error", { "ignoreBlankLines": false }]*/

import b from 'foo.js';
import c from 'baz.js';
foo();
import a from 'bar.js'; // error?
import x from 'qux.js';

[dmitov]

@mdjermanovic In my opinion, both should be correct with the option set to false

[mdjermanovic]

@mdjermanovic In my opinion, both should be correct with the option set to false

It makes sense to me, especially for the example with another statement between imports (less sure about the comment example).

Then, we should probably find a new name for the option, since it isn't about blank lines. Maybe something like allowSeparatedGroups (or just allowGroups) or onlyConsecutive.

[dmitov]

@mdjermanovic Agreed, maybe it's a good idea to take something from the TSLint ordered-imports property, which is behaving very similar to my proposal and it was the inspiration for it.

However, both libraries are different and might not be applicable to ESLint. Let's keep this discussion open and get a third opinion!

Thanks for your feedback!

[mdjermanovic]

Just to mention that, regardless of its name, in a certain way this option isn't compatible with the padding-line-between-statements rule.

This would be a misconfiguration:

{    
    "rules": {
        "padding-line-between-statements": ["error",
            { "blankLine": "always", "prev": "import", "next": "import" }
        ],
        "sort-imports": ["error", { "ignoreBlankLines": false }]
    }
}

Autofix for padding-line-between-statements would insert blank lines and thus basically disable the sort-imports rule (aside from enforcing the order of named imports within import statements).

I'm not sure if that's an issue.

[dmitov]

@mdjermanovic Agreed! Although when used in conjunction the developer should be aware of the results. The padding-line-between-statements will cause no unwanted results when used together with sort-imports

[eslint-deprecated]

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

[aladdin-add]

just need 1 champion to accept it! friendly ping @eslint/eslint-team

[mdjermanovic]

I'll champion this, it needs one more 👍 now.

[btmills]

Added my 👍 and marked as accepted.

[mdjermanovic]

We still need to figure out the option's name.

Thoughts about allowSeparatedGroups ?