[Suggestion]: import/named-order : Enforce to sort specifiers of import, export, require

[ronparkdev]

Sorry, It was my first time posting a PR, so I didn't know that I had to create an issue first.

I experienced a significant decrease in readability as the number of specifiers increased in the named import. So, I made this rule.

I already add a rule to PR #2552

[ronparkdev]

import/named-order

Enforce a convention in the order of import / export / require() specifiers
+(fixable) The --fix option on the [command line] automatically fixes problems reported by this rule.

Rule Details

The following patterns are valid:

import { Alpha, Bravo } from 'foo'

const { Alpha, Bravo } = require('foo')

const Alpha = 'A'
const Bravo = 'B'

export { Alpha, Bravo }

The following patterns are invalid:

import { Bravo, Alpha } from 'foo' // <- reported

const { Bravo, Alpha } = require('foo') // <- reported

const Alpha = 'A'
const Bravo = 'B'

export { Alpha, Bravo } // <- reported

Options

order

There is a order option available to sort an order into either caseInsensitive or lowercaseFirst or lowercaseLast.

• caseInsensitive : Correct order is ['Bar', 'baz', 'Foo']. (This is the default.)
• lowercaseFirst: Correct order is ['baz', 'Bar', 'Foo'].
• lowercaseLast: Correct order is ['Bar', 'Foo', 'baz'].

When Not To Use It

If your environment specifically requires specifiers order of import / export / require.

[JounQin]

Related tslint rule:

https://palantir.github.io/tslint/rules/ordered-imports

---

Good for some dependencies like rxjs:

import {
  BehaviorSubject,
  combineLatest,
  distinctUntilChanged,
  map,
  switchMap,
  tap,
} from 'rxjs';

---

I don't think two options like esmodule/commonjs are needed like any other rules. Why should they be different?

[ronparkdev]

I don't think two options like esmodule/commonjs are needed like any other rules. Why should they be different?

Option names are weird when I look at it again. I want to control that applying or not of import, export, and require with esmodule/commonjs options.

In general, it may be an unnecessary option, but I guess that it is necessary to have an option to not apply import or export or require for some environment.

So I think there should be an option like below.

Options

• import : apply lint to specifiers of named import
• export : apply lint to specifiers of named export
• require : apply lint to specifiers of require

[JounQin]

but I guess that it is necessary to have an option to not apply import or export or require for some environment.

Any real usage case?

[ljharb]

I would assume ordering should apply to either/both imports and exports, configurably.

I’m still not clear on the benefit here. Forced sorting doesn’t have any impact on merge conflicts, and there shouldn’t be so many different things exported by one file anyways.

[JounQin]

@ljharb

Forced sorting doesn’t have any impact on merge conflicts

It does IMO.

For instance:

import {
  combineLatest,
  distinctUntilChanged,
  map,
  switchMap,
  tap,
} from 'rxjs';

If two users want to add BehaviorSubject, with this rule, we can guarantee the final result is consistent without conflicts.

But without this rule, the two users can have different results:

 import {
+  BehaviorSubject,
   combineLatest,

vs

   tap,
+  BehaviorSubject,
 } from 'rxjs';

and there shouldn’t be so many different things exported by one file anyways.

That's rxjs's or any library's choice, this is just valid and good for tree shaking already.

[ljharb]

I see what you mean - it doesn’t impact merge conflicts (it may even increase them) but it does prevent duplication conflicts.

[ljharb]

No, that’s not good for treeshaking - treeshaking can only ever be a half-assed attempt to delete unused code. The best thing for bundle size is always maximally granular deep imports, which does a much better job than treeshaking can, due to the nature of the JS language.

So no, it’s not a valid or a beneficial choice, although it certainly is a choice.

[JounQin]

@ljharb

it doesn’t impact merge conflicts (it may even increase them)

I'm not quite sure to understand why it may even increase merge conflicts by consistently sorting?

So no, it’s not a valid or a beneficial choice, although it certainly is a choice.

import {
  combineLatest,
  distinctUntilChanged,
  map,
  switchMap,
  tap,
} from 'rxjs';

vs

import { combineLatest } from 'rxjs/operators/combineLatest'
import { distinctUntilChanged } from 'rxjs/operators/distinctUntilChanged'
import { map } from 'rxjs/operators/map'
import { switchMap } from 'rxjs/operators/switchMap'
import { tap } from 'rxjs/operators/tap'

I'd prefer the first one. 😂 (Although this is not the key point for this rule here.)

[ronparkdev]

When importing an external library, I think it is a good direction to import it separately for tree shaking.

However, tree shaking is often unnecessary in internal code.

In my case, there are more than 20 specifiers of import and If this is not aligned, it will decrease readability (it cannot be shared because it is an internal code of my company 😢 ).

I looked for a similar example..

import type {
  EndpointDefinitions,
  EndpointBuilder,
  EndpointDefinition,
  ReplaceTagTypes,
} from './endpointDefinitions'

https://github.com/reduxjs/redux-toolkit/blob/db0d7dc20939b62f8c59631cc030575b78642296/packages/toolkit/src/query/apiTypes.ts#L1-L6

In the above case, it seems to be well aligned, but the EndpointDefinition location is wrong. It is used to catch small mistakes of these developers.

[ljharb]

@JounQin it increases merge conflicts because you can’t add items at arbitrary locations, so any changes to anything that’s an alphabetical neighbor will conflict with your change. As for the two options, the second is objectively superior even if you think it’s aesthetically worse, and aesthetics is much less important than bundle size/memory footprint as well as conceptual cleanliness.

[ljharb]

@ronparkdev treeshaking is only ever necessary when you import from a file that exports more than you need, internal or otherwise. Why are all 20 of those types coming from the same file?

[ronparkdev]

Why are all 20 of those types coming from the same file?

@ljharb It's an off-the-topic part of this discussion, It seems that a lot of interfaces or types are imported when the data model is deep and the business logic is complex in the typescript environment.

[ronparkdev]

but I guess that it is necessary to have an option to not apply import or export or require for some environment.

Any real usage case?

@JounQin I thought about it, but it seems unnecessary to provide as options. Even if there are some code in which the rule should not be used, it is likely to use eslint-disable-next-line.

[ljharb]

@ronparkdev I don't think it's off topic - if it's indeed a bad practice to have so many named imports (such that ordering is required), then there's no benefit in making a rule to deal with a case you shouldn't be experiencing in the first place.

[JounQin]

@ljharb

if it's indeed a bad practice to have so many named imports

You can't convince all library authors to agree with this statement. And which practice is better depends on the authors' and end users' preferences, IMO.

import {
  combineLatest,
  distinctUntilChanged,
  map,
  switchMap,
  tap,
} from 'rxjs';

This is how rxjs works, and the rxjs/operators entry will be removed totally in favor of rxjs entry even further in rxjs v8.

[ljharb]

That's totally fair that I can't convince every library author not to do something bad.

CJS only has default exports, so it seems like the sort-keys core eslint rule would cover destructured CJS requires, no?

[JounQin]

@ljharb

sort-keys is not auto-fixable, and has been frozen as stylistic rule. See eslint/eslint#16167 (comment)

[ronparkdev]

@ronparkdev I don't think it's off topic - if it's indeed a bad practice to have so many named imports (such that ordering is required), then there's no benefit in making a rule to deal with a case you shouldn't be experiencing in the first place.

@ljharb, I totally agree a case that using a lot of named imports is a bad practice. But, many developers try to match the order of named import, but they can not match it perfectly every times. That's why I thought of this rule to use in this case.

Like the case below, the number of named imports is a few but, maybe they need help from this rule.

AS-IS

import type {
  EndpointDefinitions,
  EndpointBuilder,
  EndpointDefinition,
  ReplaceTagTypes,
} from './endpointDefinitions'

TO-BE

import type {
  EndpointDefinition, // <- relocated
  EndpointDefinitions,
  EndpointBuilder,
  ReplaceTagTypes,
} from './endpointDefinitions'

But, to be honest, I think there are more developers who don't care about this strict order. So, It is ambiguous to be included in the recommended config.

But, I think some of the developers feel it's necessary?

[mkosir]

Great stuff @ronparkdev, would love to have this rule 🙇‍♂️ 🚀

[JensDll]

A useful rule, in my opinion. In addition, it would be nice if it also touches on "type Modifiers on Import Names". For example, I would like to enforce type modifiers to be ordered last and then maybe sort both individually:

import { type B, a, b, type A } from "some-module"
import { a, b, type A, type B } from "some-module" // Change the above to this

[duncanbeevers]

Such a rule would also help with find+replace across a codebase since each import of items would assume a canonical form.
When fixing or moving stuff around, it would be easier to trust that all imports will be in canonical form.

// find+replace on `import { Alpha, Bravo } from 'module';` misses the b.js reference

// file a.js
import { Alpha, Bravo } from 'module';

// file b.js
import { Bravo, Alpha } from 'module';

[manuth]

I just prepared a Pull Request for this feature as typescript-eslint dropped support for the tslint plugin.

I skimmed through the comments and will also add support for specifying whether or not checking the ordering of ESModule imports/exports and CommonJS imports separately (i.e. something like { esModule: true, commonJS: true } or whatnot)

Stay tuned and stuff!