[New] order: enable intragroup sorting of type-only imports

[Xunnamius]

Resolves #2615
Resolves #2441
Resolves #2347
Resolves #2172
Related #2912

First, thanks for making eslint-plugin-import! It has saved me a lot of time and headache.

This PR implements in import/order: a new option enabling intragroup sorting of type-only imports to match intergroup sorting of normal imports.

A demo package containing this feature is temporarily available for easy testing:

npm install --save-dev eslint-plugin-import@npm:@-xun/eslint-plugin-import-experimental

Allow intragroup sorting of type-only imports

This is implemented via sortTypesAmongThemselves. The proposed documentation corresponding to this new feature can be previewed here.

Example

Given this code (which is already correctly ordered):

import type A from 'fs';
import type B from 'path';
import type C from '../foo.js';
import type D from './bar.js';
import type E from './';

import a from 'fs';
import b from 'path';
import c from '../foo.js';
import d from './bar.js';
import e from './';

And the following settings, the rule check will fail:

{
  "groups": ["type", "builtin", "parent", "sibling", "index"],
  "alphabetize": { "order": "asc" }
}

With --fix yielding:

import type C from '../foo.js'; // ???
import type A from 'fs';
import type B from 'path';
import type D from './bar.js';
import type E from './';

import a from 'fs';
import b from 'path';
import c from '../foo.js';
import d from './bar.js';
import e from './';

This is currently the technically correct behavior, but the wrong outcome in my opinion.

However, with the following settings, the rule check will succeed instead:

{
  "groups": ["type", "builtin", "parent", "sibling", "index"],
  "alphabetize": { "order": "asc" },
+ "sortTypesAmongThemselves": true
}

Of course, achieving the reverse order (type-only imports after normal imports) is possible by moving "type" to the end of groups:

{
+ "groups": ["builtin", "parent", "sibling", "index", "type"],
  "alphabetize": { "order": "asc" },
  "sortTypesAmongThemselves": true
}

It was only after implementing this feature, when I started reviewing the issues literature, that I saw #2441, which lead me to eslint-plugin-perfectionist (neat) and its implementation of "builtin-type," "external-type," etc. I much prefer this PR's implementation, which takes a similar approach to this comment. I don't see how allowing type-only builtin/external/etc imports to be sorted in a different order than non-type builtin/external/etc imports makes things less confusing. And that's before we consider custom pathGroups.

Instead, the goal of sortTypesAmongThemselves is to allow the "type" group to be sorted among itself in a backward-compatible way without the added complexity (and potential inconsistency) of adding regexp or "*-type" groups to groups. I'm already writing too much configuration. I just want to flick a switch 😅.

Additionally, the code that makes sortTypesAmongThemselves work could be extended to make something like #2912 work too (perhaps through another option), though that use case is not of interest to me currently.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.61%. Comparing base (8b2d570) to head (0c179cd).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3104      +/-   ##
==========================================
- Coverage   95.04%   94.61%   -0.43%     
==========================================
  Files          84       84              
  Lines        3634     3643       +9     
  Branches     1279     1283       +4     
==========================================
- Hits         3454     3447       -7     
- Misses        180      196      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ljharb]

This is doing a lot, and is a bit of a long diff. It seems like there's 4 separate PRs to be had here:

1. NaN bugfix
2. newline controls for type-only imports
3. intragroup sorting of type-only imports
4. a new option to consolidate newlines and save space where possible.

I definitely would prefer to see 3 of those peeled off into separate PRs, so that each thing can be easily reviewed separately. Thanks!

(also, could you please add parens to ensure proper precedence is conveyed when there's a combination of && and ||?)

[Xunnamius]

I definitely would prefer to see 3 of those peeled off into separate PRs, so that each thing can be easily reviewed separately. Thanks!

Gotcha, I'll slice it up this weekend.

(also, could you please add parens to ensure proper precedence is conveyed when there's a combination of && and ||?)

I thought I ran my modifications through Prettier, but I probably missed a few blocks. I'll take a look

[ljharb]

I think prettier removes those parens, which is one of its many flaws :-)

[jftanner]

For the record, I definitely would like this feature. As evidenced by the fact that I tried to use it per the docs on main. (Edit: Because I mistakenly thought they were for the latest release.) :)

[ljharb]

@Xunnamius any progress? i'd love to get this merged for the next release.

[Xunnamius]

@ljharb Sorry, something I'm working on is, of course, taking longer than I anticipated. I hope to wrap up today or tomorrow. This PR is up next 😊

[ljharb]

this is a bit wordy; let's rename it throughout to just sortTypes?

[Xunnamius]

I considered the shorter name at first, but I don't believe it's descriptive enough. import/order already sorts type-only imports; "type" is already a valid groups name, and its groups that actually controls if type-only imports are sorted or not sorted, right? With sortTypes, it wouldn't be immediately clear what turning off "sorting types" is supposed to mean with respect to the other rule options. You'd have scenarios where sortTypes: false but types are still being sorted...? And would sortTypes: false disable named.types sorting? This kind of ambiguity would confuse future me, at least, and would cause an unnecessary trip to the docs, docs which would have to become even more verbose to explain why sortTypes doesn't control if types are sorted.

Imo, sortTypes doesn't communicate the effect of flipping this switch and can even be confusing. We'd have sortTypes, "type" in groups, "type" in pathGroups.group, "type" in pathGroupExcludedImportTypes (which is already wordy and already somewhat-confusingly uses the word "Types" in its own name different to how the word is used in other option names like named.types and newlines-between-types), etc.

Whereas sortTypesAmongThemselves makes it clear that we're not interfering with the other "type"-related options, and we're not controlling if type-only imports are sorted vs not sorted but if they're sorted among themselves wrt groups vs if they're sorted among the normals as a single "type" blob.

[ljharb]

hmm, fair points. what about sortTypesGroup?

[Xunnamius]

I think that works 😊