Add plugin to sort imports consistently

[kiliman]

Since you've added a Prettier plugin to sort Tailwind classes, have you considered adding a plugin to sort imports as well? This will ensure consistency, especially when modifying a file for a PR.

I used to have the setting: source.organizeImports enabled in VS Code. This works great to keep my imports nice and tidy. However, it would cause spurious changes when editing code that didn't have this enabled.

With the plugin @trivago/prettier-plugin-sort-imports installed, everyone will have the same formatting.

Use an eslint plugin for sorting imports.

[kentcdodds]

While changing the order of CSS class names doesn't affect the runtime/specificity of those css classes, the import order does have semantic meaning. If your module has side-effects then you need to make sure that module is imported first. We have such a situation here:

epic-stack/tests/setup/setup-test-env.ts

Line 1 in 2c13e72

import './setup-env-vars.ts'

I suppose we could just add a // prettier-ignore comment there, but that's what makes me hesitate. I'm not super excited about formatters changing the semantics of my code 😬

[kiliman]

You can configure the sorting rules, so if you want, you can have the local imports first. Perhaps add a prefix to ensure those are always first, like './!setup-env-vars.ts'

// sort ./!* files first, then normal modules, followed by local files
importOrder: ['^[.]\\/!', '<THIRD_PARTY_MODULES>', '^[.]\\/'],

Although I prefer to avoid relying on side-effects, at least with this naming convention, it will signal to the developer that this is a special file.

[kentcdodds]

I dunno. Feels like that violates the principle of least surprise to me. I just don't think it's worth it.

[skjelmo]

Maybe it could be worthwhile adding an example @kiliman?

Thank you both for teaching me something new through this discussion. I didn't even consider imports with side-effects or the semantic meaning of the import order when sorting imports myself.

I added import sorting as one of the first things, since the imports weren't alphabetically sorted or consistent.

e.g. @remix-run/node in app/utils/misc.tsx is second last, while in app/entry.server.tsx it is first.

I still don't get if I'm missing out on some deliberate semantic meaning in the import order, or if there are too many undeliberate inconsistencies for it to be valuable. Maybe I'm missing something? I subjectively feel it's worth the cost of adding a prettier plugin to reduce the mental overhead of an inconsistent import order. It also seems hard to uphold a deliberate semantic import order without prettier.

I fully understand that you are hesitant to add it to the Epic Stack. I still believe many would benefit from an example. Maybe with a few pointers to tradeoffs to consider.

[MichaelDeBoey]

Not sure if we should use a formatter to sort imports tbh as for me a formatter is about formatting your code and sorting imports would be more a task for a linter imo
So eslint-plugin-import's order rule would be my go-to in this case

[kiliman]

Ok, I found another plugin that does the same as the others combined. I was able to patch it so it will sort the types at the end. It even sorts exports.

npm i -D eslint-plugin-simple-import-sort

// .eslintrc.cjs
`simple-import-sort/imports': `warn',
'simple-import-sort/exports': 'warn',

This patch moves the sort by kind to the top and reverses the order (making sure type comes after value).

eslint-plugin-simple-import-sort+10.0.0.patch

diff --git a/node_modules/eslint-plugin-simple-import-sort/shared.js b/node_modules/eslint-plugin-simple-import-sort/shared.js
index 60c92d9..a9c45e8 100644
--- a/node_modules/eslint-plugin-simple-import-sort/shared.js
+++ b/node_modules/eslint-plugin-simple-import-sort/shared.js
@@ -730,6 +730,11 @@ function sortImportExportItems(items) {
 function sortSpecifierItems(items) {
   return items.slice().sort(
     (itemA, itemB) =>
+      // Sort type specifiers after regular ones.
+      compare(
+        getImportExportKind(itemB.node),
+        getImportExportKind(itemA.node)
+      ) ||
       // Compare by imported or exported name (external interface name).
       // import { a as b } from "a"
       //          ^
@@ -745,11 +750,6 @@ function sortSpecifierItems(items) {
       // export { b as a }
       //          ^
       compare(itemA.node.local.name, itemB.node.local.name) ||
-      // Then put type specifiers before regular ones.
-      compare(
-        getImportExportKind(itemA.node),
-        getImportExportKind(itemB.node)
-      ) ||
       // Keep the original order if the names are the same. It’s not worth
       // trying to compare anything else, `import {a, a} from "mod"` is a syntax
       // error anyway (but @babel/eslint-parser kind of supports it).

Add.plugin.to.sort.imports.consistently.epicweb-dev_epic-stack.Discussion.245.-.7.August.2023.mp4

[kentcdodds]

Nice! I think I'd rather avoid patching, but if anyone wants to make a PR and get that in the official lib, I'd love to use it.

[kiliman]

BTW: This issue discusses import sorting lydell/eslint-plugin-simple-import-sort#124. The behavior in v10 is to sort import specifiers alphabetically, even types. So imports would sort like:

import {
	type ActionArgs,
	json,
	type LoaderArgs,
	redirect,
} from '@remix-run/node'

The main rationale is consistency with other tooling as well as minimizing diff changes (simply add or remove type specifier without resorting).

Using the patch will sort types at the end. Considering the issue above, not sure if they would accept a PR to add a setting for this.

[jacobparis]

One possible alternative is this lovely plugin that lets you register local Eslint rules, so you can add any Epic stack opinionated rules right into the codebase without needing to publish/patch

I've used this in the past to enforce capitalization styleguides in locale files via a local eslint plugin

https://www.npmjs.com/package/eslint-plugin-local-rules

[kiliman]

So if I'm not misunderstanding, that plugin allows you to create your own local plugin, essentially. Unfortunately, in this case, it would require essentially copying the original plugin with our custom change vs patch-package, since we need to update the internal sort algorithm.

With that being said, it looks like even the TypeScript team is adopting eslint-plugin-simple-sort with VS Code Organize Imports and auto-imports eventually doing the same thing (default to sort ordinal).

So perhaps it may be best to just go with the flow and let the plugin do its thing.

microsoft/TypeScript#52090
microsoft/TypeScript#52115
microsoft/TypeScript#52820