Use eslint-plugin-simple-import-sort

[jakebailey]

This is an alternative to #51916, keeping sensitive sorting but correctly handling cases like HasDecorators and hasDecorators (which don't predicably sort) as well as sorting declarations themselves.

The sorting algorithm this uses is very simple: https://github.com/lydell/eslint-plugin-simple-import-sort/#sorting

Unfortunately, this still doesn't enable using "organize imports" as that really changes things. But, this one is "fixable" and fully defines the ordering of all aspects of the imports for us, so --fix or a quickfix will always do the right thing.

Organize imports now works as of #52115, I have added the unstable config to the settings template to see.

Fixes #51617

[jakebailey]

The other PR I plan to send after (maybe during?) this one is one that enforces the style of having a vertical import list. Right now, we have some that are vertical that I fixed myself, but some that aren't because they're newer. In lieu of a formatter which does this, the plugin is sort of needed to complete the "never have conflicts again" thing.

[jakebailey]

Just found one difference; our organize call moves the export statements in the Herebyfile too:

diff --git a/Herebyfile.mjs b/Herebyfile.mjs
index 4171dea36f..56b6005c3d 100644
--- a/Herebyfile.mjs
+++ b/Herebyfile.mjs
@@ -351,6 +351,10 @@ const { main: tsc, watch: watchTsc } = entrypointBuildTask({
     mainDeps: [generateLibs],
 });
 export { tsc, watchTsc };
+export { services, watchServices };
+export { tsserver, watchTsserver };
+export { lssl, watchLssl };
+export { tests, watchTests };
 
 
 const { main: services, build: buildServices, watch: watchServices } = entrypointBuildTask({
@@ -364,7 +368,6 @@ const { main: services, build: buildServices, watch: watchServices } = entrypoin
     mainDeps: [generateLibs],
     bundlerOptions: { exportIsTsObject: true },
 });
-export { services, watchServices };
 
 export const dtsServices = task({
     name: "dts-services",
@@ -388,7 +391,6 @@ const { main: tsserver, watch: watchTsserver } = entrypointBuildTask({
     output: "./built/local/tsserver.js",
     mainDeps: [generateLibs],
 });
-export { tsserver, watchTsserver };
 
 
 export const min = task({
@@ -417,7 +419,6 @@ const { main: lssl, build: buildLssl, watch: watchLssl } = entrypointBuildTask({
     mainDeps: [generateLibs],
     bundlerOptions: { exportIsTsObject: true },
 });
-export { lssl, watchLssl };
 
 export const dtsLssl = task({
     name: "dts-lssl",
@@ -455,7 +456,6 @@ const { main: tests, watch: watchTests } = entrypointBuildTask({
         }
     },
 });
-export { tests, watchTests };
 
 
 export const runEslintRulesTests = task({

[jakebailey]

Maybe this is the export logic not respecting the change we made to grouping for imports? Will file a bug.

[jakebailey]

I'm going to hold off on this for now pending lydell/eslint-plugin-simple-import-sort#124; I need to check to see if we have anything that fits this issue in our own repo as it'll end up breaking organize I'm pretty sure?

[jakebailey]

It turns out there are some edge cases with the type keyword that turn out not to agree between the tools. Given:

import {
    Apple,
    Pineapple,
    type Banana,
    Mango,
} from "foo";

TypeScript (with the unstable options) says:

import {
    Apple,
    Mango,
    Pineapple,
    type Banana,
} from "foo";

simple-import-format says:

import {
    type Banana,
    Apple,
    Mango,
    Pineapple,
} from "foo";

And dprint says:

import {
    Apple,
    type Banana,
    Mango,
    Pineapple,
} from "foo";

My personal feeling is that the last one is the "correct" one, as it's the one where you can add or remove a type keyword and the ordering doesn't change, but I also don't know if our behavior is intended. See also lydell/eslint-plugin-simple-import-sort#124.

[jakebailey]

On second thought, I'm not going to mark this as a draft; we don't have any type keywords at the moment so I'm pretty sure that this PR is still an improvement on the current state of things, but it's something we should look into.

[jakebailey]

As of eslint-plugin-simple-import-sort v10 (lydell/eslint-plugin-simple-import-sort#125), there are now only two behaviors, the one used by TypeScript's current sorter (type imports at the end), and the one used by dprint/simple-import-sort (sort as though the type keyword was not present). Moving towards convergence.

[jakebailey]

Per offline discussion, I'm going to merge this as-is; I am working on adding configurability for the type keyword (it's tricky) and since we don't use any in the codebase at the moment, we can work on adding that later.