Add sort-imports member sorting into a separate option

[remcohaszing]

What rule do you want to change?

sort-imports

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

Fewer

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

Currently, sort-imports does two checks. One is always on, the other is optional. I would like to only use the optional part.

One part is that it checks the order of import statements. This can’t be turned off. I would like to check this using import/order, which offers more advanced features.

The part of the rule I would like to use, is the sorting of the imported members.

My proposal is to split the distinct memberSort option into a separate rule: sort-import-members.

Please provide some example code that this change will affect:

// Builtins
import { join, resolve } from 'path';

// External
import { map } from 'lodash';

What does the rule currently do for this code?

sort-imports will cause conflicts with import/order, but also make sure import members are sorted nicely.

What will the rule do after it's changed?

sort-imports should be disabled to prevent conflicts with import/order.

sort-import-members will ensure the imported members are sorted, as sort-imports does now.

[kaicataldo]

Given the pluggable nature of ESLint and the size of the plugin community, we can't always ensure that all rules work with plugins. I'm not convinced we need an entirely new rule to solve this problem.

I wonder if a better solve would be to deprecate the ignoreMemberSort option in favor of an option that allows the toggling of sorting import declarations and imported members individually. I'm imagining something like:

{
    "sort-imports": ["error", {
        "sortDeclarations": true,
        "sortMembers": true,
    }]
}

If we decide to go down this route, we should do this in a backwards compatible way.

[remcohaszing]

That proposal seems just as good to me. 👍

For backwards compatibility, I suggest the following signature:

{
  "sort-imports": ["error", {
    ignoreMemberSort: false,  // Already exists
    ignoreDeclarationSort: false,  //  Analogous to `ignoreMemberSort`. The default matches the current behavious.
    ...otherOptions
  }]
}

[nzakas]

The proposal to add an option to turn off declaration sort seems reasonable to me. 👍

@remcohaszing are you will to submit a pull request for this proposal?

[remcohaszing]

I'll give it a try somewhere this week.

[AdrienLemaire]

Can someone look into the related PR ? There hasn't been any action taken in over a month, and I'm also very interested in having a rule to check ordered and sorted imports.
Thank you for your PR @remcohaszing :)

[nzakas]

Thanks for the ping, we'll take a look.

[nzakas]

TSC Summary: This proposal seeks to add another option, ignoreDeclarationSort to sort-imports to allow turning off a part of the rule that previously was required. The option is backwards-compatible and there is already a pull request (#11040) for implementation. This issue fell off of the radar and so is missing feedback, but I will champion.

TSC Questions:

• Should this proposal be accepted?
• Should Update: Add sort-imports ignoreDeclarationSort (fixes #11019) #11040 be merged?

[ilyavolodin]

Rule changes do not require TSC consent, unless they are breaking. In that case, it looks like there's a champion and 1 up vote already on the proposal. It's now up to the champion to gather more support from the rest of the team, until this issue can be accepted.

[nzakas]

@ilyavolodin oh right, duh! Thanks.

[remcohaszing]

Is any action required from me? I thought not, but it’s taking much longer to get merged than I expected.

[nzakas]

@remcohaszing thanks for the ping. Sorry, a lot of team members are taking time off around this time of year, so things can take a bit longer than usual. It looks like enough team members support this change so I'm marking as accepted. The next step is to review the PR.