import-js/eslint-plugin-import

Allow enforcement of import-specifiers

manuth opened this issue · 20 comments

Imports can have multiple specifiers (objects which are being imported).
Would be awesome if there would be a way to enforce alphabetization there, too:

Example:

import { isNull, isNaN } from "util"; // reports an error: "isNaN" should appear before "isNull"

Related to #1577, #1670.

While eslint’s included sort-imports rule covers this, I came here looking for an alternative because that one doesn’t account for TypeScript type-only imports (and I don’t know enough about eslint internals to know whether it could, or whether they would accept such an addition even if it can). TypeScript’s own convention is to sort type-only import specifiers last:

import { x, y, z, type A, type B, type C } from "./mod";

But I haven’t found an eslint rule capable of enforcing this style in my brief search so far. I had thought I might find it in this plugin, since it does recognize import type and import typeof.

Does the typescript-eslint plugin have a sort-imports rule that covers it?

No, I couldn’t find anything related in typescript-eslint.

ljharb commented

@bradzacher do you think this would make sense in the TS eslint plugin?

I think that it makes more sense in this repo so we don't "compete" on rules.
"for everything related to imports go here" is more user-friendly than "for sorting imports go here and for everything else go here".

I've been trying to draw the line of "if you're encoding TS-specific semantics about imports in a rule, typescript-eslint, otherwise, import-js"

Which is why I've added/proposed things over here instead of in our repo.

ljharb commented

Makes sense in general; since sort-imports is a core rule already it seemed a reasonable suggestion :-)

Yeah we have done it in the past with no-duplicate-imports - we added an extension for it because it's a base rule, then we realised that ofc there's import/no-duplicates which already existed and supported what users wanted.

We're removing our extension in the next major and directing people towards your rule instead.

So this would be the same case - instead of adding an extension rule to add support for type imports and competing, we'll just want a rule here that can freely support TS, JS and flow for the whole community!

I think we (the TypeScript team) would be pretty interested in having an option for this. When we migrated our codebase to modules, we enabled the core sort-imports rule but noticed that our own auto-imports sorting logic doesn’t play nicely with it. I just wrapped up some work to make sure we can detect and conform to however sort-imports is configured, but if we start using any type-only import specifiers, our rules will again disagree with sort-imports, and I’m less willing to give up grouping of type-only imports. (cc @jakebailey)

The other thing I noticed about eslint's base sort-imports rule is that it only handles named imports if it's all on the same line. In many cases I have instances that I rolled a number of nested modules into one top-level module that re-exports everything. This allows importing a bunch of things from one convenient module. Autoformatters will then line-break them. It would be nice if this rule could also handle ordering multi-line named imports too.

Example:

import {
    b,
    a,
} from './module';

gets sorted to ->

import {
    a,
    b,
} from './module';
manuth commented

For the time being, you might want to use @typescript-eslint/tslint in combination with tslint which does provide such a rule.

ljharb commented

tslint has been deprecated since 2019, so i'm not sure that's a good recommendation, unless this is just a tslint-like eslint-based replacement?

@basicdays it's very unfortunate that eslint won't make changes to stylistic rules anymore, or else that would be a perfect place for that modification to be made.

Does anyone have a concrete suggestion for adding an option that would govern sorting of named imports within curly braces, whether single or multi-line? Note that with multiline, comment attachment becomes quite tricky wrt autofix.

manuth commented

It does use the actual tslint under the hood but it's integrated in eslint

ljharb commented

oof, then I would strongly avoid it, since the tool's been deprecated (and afaik, unmaintained) for 4 years.

Does anyone have a concrete suggestion for adding an option that would govern sorting of named imports within curly braces, whether single or multi-line? Note that with multiline, comment attachment becomes quite tricky wrt autofix.

I would recommend using the same sort order that simple-import-sort and dprint use, which in effect is to use:

const comapre = new Intl.Collator("en", {
  sensitivity: "accent",
  caseFirst: "upper",
  numeric: true,
}).compare;

Which is to say "sort case-insensitively with natural numbers, tie breaking with plain string comparison". simple-import-sort does it slightly differently via an explicit fallback, but I think those collator options should work.

We're actually looking to even this behavior out across tooling and get TS's organize import to be able to handle them evenly (via some preset mechanism).

That, and, ignore the type qualifier when sorting inside of curlies, which I'm currently trying to get finished in TS as well (right now we always put them at the end, but dprint and simple-import-sort more correctly place them inline, which prevents total reorderings when adding/removing the modifier). It's just pretty tricky to implement and I probably won't have it ready before TS 5.1 ☹️

All in all, it's easiest to just play around with https://dprint.dev/playground/#code/Q/language/typescript which should show what I'm meaning.

ljharb commented

@jakebailey sometimes people want ascending, sometimes descending; using Intl would be a fine implementation suggestion if we could use it, but we can't because we support older nodes than where it's available. I'm asking about the schema for this option.

sometimes people want ascending, sometimes descending

Who wants descending?

using Intl would be a fine implementation suggestion if we could use it, but we can't because we support older nodes than where it's available

Intl.Collator has been present in Node since 0.12, so I would be shocked if you couldn't use it...

I'm asking about the schema for this option.

Er, wouldn't that depend on its functionality and implementation? e.g. TS only had a couple of knobs, but has gained a few more (hidden for now) in 5.0, and may gain a couple more in the future as we refine it to try and accept the various different sorting mechanisms.

manuth commented

I think the same settings like the ones mentioned in alphabetize should cover everything you'd ever want to configure:

  • order: use asc to sort in ascending order, and desc to sort in descending order (default: ignore).
  • orderImportKind: use asc to sort in ascending order various import kinds, e.g. imports prefixed with type or typeof, with same import path. Use desc to sort in descending order (default: ignore).
  • caseInsensitive: use true to ignore case, and false to consider case (default: false).

I think we could even apply the same default settings as the alphabetize settings...

Not sure about the name, though... what about alphabetizeMembers, alphabetizeNamedImports or something similar?

ljharb commented

huh, good call, i didn't realize Intl was available in node 0.12, for some reason I thought it was only available by default in v12.

I would assume that the hope would be to design the entirety of the functionality before adding it, and I agree with @manuth that the same settings makes sense.

huh, good call, i didn't realize Intl was available in node 0.12, for some reason I thought it was only available by default in v12.

Checking MDN, the API is available, but only supported en-US until v13, which is thankfully the "right" one to use for this particular application (based on the behavior of other tools).