eslint-plugin-import
eslint-plugin-import copied to clipboard
[Suggestion]: `import/named-order` : Enforce to sort specifiers of import, export, require
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 https://github.com/import-js/eslint-plugin-import/pull/2552
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
.
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?
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
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?
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.
@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.
I see what you mean - it doesn’t impact merge conflicts (it may even increase them) but it does prevent duplication conflicts.
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.
@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.)
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.
@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.
@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?
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.
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
.
@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
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.
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?
@ljharb
sort-keys
is not auto-fixable, and has been frozen as stylistic rule. See https://github.com/eslint/eslint/issues/16167#issuecomment-1199880245
@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?
Great stuff @ronparkdev, would love to have this rule 🙇♂️ 🚀
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
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';