Sorting of "type FooX" and "Foo" imports disagrees with typescript, dprint

[jakebailey]

I have something like this:

import { Foo, type FooX } from "foo";

TypeScript's sort/organize imports as well as dprint's (so, deno fmt too) case insensitive sortings both order these with Foo first, then type FooX, as above.

However, simple-import-sort appears to flip them, putting type FooX before Foo, like:

import { type FooX, Foo } from "foo";

Even though if the type keyword were removed, it'd sort the other way.

This leads to a frustrating scenario in my editor where the formatter and eslint fix both run and flip it back and forth, leaving nobody happy.

The only clue I have about this is the mention of \u0000 in the readme, but I'm not sure how "Foo" would sort after "FooX\u0000".

[lydell]

Hi!

The default in this plugin is to ignore type in import type, except if both import type and regular imports are used for the same source, the type imports come first. It’s possible to move import type to its own group by the \u0000 tricks.

Regarding type inside { and } though, there is no configuration. The type ones come first.

Now I have some questions:

1. What is “Organize imports”’s rule for import type?
2. What is the intended end user workflow here?
3. Why does “Organize imports” try to be compatible with other tools? My gut feeling is that if you use two different import sorters, there’s always gonna be edge cases where they aren’t compatible. Is it to enable people to use “Organize imports” in their editor, and enforce it in CI? But then, why does not TypeScript expose it as a tool or something like that? And why would people use “Organize imports” in their editor over running ESLint on save?

Just trying to understand all the ins and outs of this so the right decision can be made.

[jakebailey]

Thanks for the great questions! The context here is microsoft/TypeScript#52090, if you didn't see the cross link.

The reason that I personally care about Organize Imports being compatible with other tools is that there are many on my team who do not want to run ESLint live in their editor or enable auto-fixes. One reason is preference, but mainly that in the TypeScript compiler itself we have a few heavily-edited but massive files like checker.ts which really, really chug if you have ESLint enabled. I personally just live with it, but many on the team don't. So from that point of view, being able to have a setup which allows both to operate is a benefit.

But, moreso, the organization logic used by Organize Imports is also the same logic used by auto-imports; when we offer up an auto-import completion, we have to stick that import somewhere. If we can't detect the intended ordering (or, have settings to configure it), then we have to resort to sticking imports on the end. For those who don't run ESLint, this means they will have to fix their PRs up once they see that they fail in CI; for those who run ESLint in their editors, it's another quickfix that could have been avoided. So, getting this sort order to match the user's intent is just generally good.

What recently changed was the merge of this PR: microsoft/TypeScript#52115

Effectively, this means that tsserver can use Intl.Collator, and so can be configured in such a way that the sort order of specifiers and declarations (within a group) will match this plugin, as this plugin also uses that API. For example, setting this in VS Code gets you there:

"typescript.unstable": {
    "organizeImportsCollation": "unicode",
    "organizeImportsCaseFirst": "upper",
    "organizeImportsIgnoreCase": false,
    "organizeImportsNumericCollation": true
}

So, as a combined workflow, this means that users can run plugins like this one to enforce certain orderings, but still be able to use Organize Imports and auto-import completions without extra steps. (We are even considering offering up some presets for common sorting choices; the above being one that would match this plugin... besides the sorting inconsistency I found for this issue.)

I'll also note that neither of the other tools (TypeScript or dprint) will move imports between import groups, opting to preserve what the user wrote; this means that simple-import-sort's ordering would be preserved even on organize/sort/format, which is a nice pattern.

Unfortunately, in trying to answer your first question, I found that it's more inconsistent between these three toolings than I thought. Namely:

• simple-import-sort does in fact put all type imports first; my example was too simple and so I assumed a different behavior.
• TypeScript's sort imports (with the above configuration) puts type imports at the end. I need to go check to see if that was intentional or not.
• dprint does what I thought we all were doing, which is to just ignore the type keyword entirely. This at least to my eye is the "best" option because it means adding or removing a type keyword to a specifier doesn't move it in the list.

So just to answer your questions specifically:

1. What is “Organize imports”’s rule for import type?

• TypeScript's sort ordering doesn't seem to be how I expected, so I probably filed this too early; I'm going to be looking into this one.

2. What is the intended end user workflow here?

• The "intended" workflow in my mind is that this plugin's lint is always present, but that tooling like TypeScript agree, allowing for people working on our repo to use what they prefer, but mainly that a feature that is sort-order dependent like auto-imports works with minimal edits.

3. -

• Why does “Organize imports” try to be compatible with other tools?
  • As in question 2, auto-imports uses this ordering, so it ends up mattering if you don't want extra fixes.
• Is it to enable people to use “Organize imports” in their editor, and enforce it in CI?
  • Yes, kinda; obviously, simple-import-sort does more especially when it comes to grouping, so that would still be checked later, but I'm hoping for a situation in which running organize doesn't regress from the user's intended ordering.
• But then, why does not TypeScript expose it as a tool or something like that?
  • I can't find it anymore, but there was an eslint plugin which did expose this; but in general, TypeScript's organize, format, etc, are pretty unopinionated and so can't enforce a specific style (e.g. why someone would use prettier, dprint, etc). That being said, if we can set the new organize options, maybe that would improve. But I don't see us adding anything quite as powerful as simple-import-sort's group rules (which I personally like quite a bit).
• And why would people use “Organize imports” in their editor over running ESLint on save?
  • Some is just user preference, but mainly that (at least in our codebase), ESLint tends to be very slow.

[lydell]

This at least to my eye is the "best" option because it means adding or removing a type keyword to a specifier doesn't move it in the list.

That’s a very good point. Minimizing unnecessary diff is an important idea of this plugin. I thought that things would only ever be types or values, but I now realize that when you import a class it can be either to use it in a type annotation or to do new on it (or both). This also feels more consistent with how import type is handled.

I now feel like making that change. What do you say?

[jakebailey]

To give more context, I actually found this bug while dogfooding the new TS 5.0 option verbatimModuleSyntax, which forces you to use type-only imports if the thing you're using isn't used as a value. My code ended up with quite a few imports that had to change, and to my surprise, adding the type keyword moved them in the list.

I think it's likely that people are going to be in the same situation as me, so I would agree that it's a good change to make. It would reduce the number of behaviors between TS, dprint, and this plugin from three to two (as dprint and simple-import-format would agree), and I think it's something to look at from TypeScript's side as well (which I'm hoping brings it down to just one behavior, the golden number).

(There's an aside here about how to handle the case where you have both Foo and type Foo as that's another tooling-dependent inconsistency I've found, but nobody should actually have that code so it's not worth expanding on.)

[lydell]

I just released 10.0.0-beta.1 with this change. Could you try it out in your TypeScript PR before I release for real?

[jakebailey]

I tested it out on my personal project and TypeScript, and it seems to work as expected. As a goofy edge case, this:

import {
    type Foo as Foo2,
    type Foo,
    Foo,
    Foo as Foo3
} from "foo";

Now becomes:

import {
    type Foo,
    Foo,
    type Foo as Foo2,
    Foo as Foo3
} from "foo";

Which exactly matches dprint too.

TypeScript itself still puts all of the type imports at the end, but that's what I knew would already mismatch (and has to be fixed); there are now only two orders in total between the three tools. So, LGTM!

Thank you so much for looking into this!

[jakebailey]

FWIW testing on microsoft/TypeScript#52090 didn't net anything as our repo doesn't have any type imports (yet!), hence coming up with contrived examples and using my own projects for testing.

(TypeScript 5.0 is the first release that has imports internally, which is why we're discovering all of these things now. Yay dogfooding!)

[lydell]

Awesome! Thanks for reporting, answering all my questions, testing and suggestions! Released 10.0.0.