Document how we sort imports

[wincent]

Trying to find some "prior art" on this one, this is the closest discussion I can put my hands on.

My understanding is that:

1. Sorting should ideally be done via tooling.
2. We should minimize side-effectful imports, especially ones that are order-sensitive, because they make "1." difficult.
3. Ideally we want rules that are easily explained and understood.

In service of that last point, my recommendation is:

• Group imports into two blocks: external (NPM packages) and internal (relative imports).
• Within each group, sort lexicographically by module name (ie. the thing on the right).

Why group imports? This is useful to define the boundary between "inside" and "outside" of a project.

Why sort lexicographically within groups? It is simple, machines can do it.

Some consequences:

Upper case sorts before lower case:

A
a 

Directories "higher up" sort before directories "lower down".

../../a
../a
./a

I think we should do this because it's simple, but the other consequence of it is that it mostly puts things in order from "more general, external" (ie. stuff higher up in the filesystem hierarchy) to "more local, specific" (ie. stuff in the current directory or closer to the leaves of the filesystem tree). Putting the NPM packages at the top reinforces the progression from "general, external" to "local, specific".

Why sort based on the module name? There is only one thing on the right, so you don't have to worry about tie-breaking multiple things on the left:

import {g, z} from 'one';
import {a} from 'other';

ie. "one" sorts before "other".

This approach also produces smaller and more readable diffs if you add or remove a binding on the left-hand side, because left-hand-side changes don't affect ordering; eg:

 import {something} from 'a';
-import {x} from 'b';
+import {a, x} from 'b';
 import {other} from 'c';

What about side-effectful imports? As I said above, hopefully we wouldn't have too many of these, but when we do, I think they should follow the same rules for simplicity.

What about types? In TypeScript, type imports look like regular imports, so they should be grouped and sorted just like any other imports.

What would an overall example look like?

1. NPM packages and Node built-ins.
2. Modules in parent directories (etc).
3. Modules in current directory.

import fs from 'fs';

import other from '../other';
import {thing} from './thing';

[bryceosterhaus]

Group imports into two blocks: external (NPM packages) and internal (relative imports).

Only reason I would be against this would be because our current tooling doesn't support this.

Overall though, I don't really care how things are sorted, so long as I can just hit save and it will auto sort and group for me. The tooling wouldn't necessarily need to know about "side-effectful imports" other than we would likely have an eslint-disable next to it.

[markocikos]

@wincent See https://github.com/liferay/liferay-portal/blob/d8ff39c41ff9c81a4959665c0469a30d34b17080/modules/apps/document-library/document-library-web/src/main/resources/META-INF/resources/document_library/js/categorization/EditCategories.es.js.

How would we sort by module name:

import 'asset-taglib/asset_categories_selector/AssetCategoriesSelector.es';
import 'frontend-js-web/liferay/compat/modal/Modal.es';
import templates from './EditCategories.soy';

asset_categories_selector is an odd alias; I assume only by common name pattern that asset-taglib is the module. frontend-js-web is a module, but EditCategories is not and dot is not. Technically speaking, name of the third module is document-library-web.

This can quickly become needlessly confusing. Maybe it's better to sort by whatever is next to import .

[wincent]

Two groups:

1. "External imports" (ie. asset-taglib, frontend-js-web)
2. "Internal imports" (ie. "./anything")

Within each group, sort lexicographically.

Sorting by the left-hand side is problematic because due to destructuring the left-hand side is a "compound" sort key, and it ends up producing non-minimal diffs like I said above. If you sort by the left-hand side and do the simplest possible thing there (a lexicographical sort), you wind up with odd stuff like the following, because { sorts before 'a':

import {zzz} from 'zzz';
import aaa from 'aaa';

[markocikos]

OK, makes sense. I understood from the original post that only npm packages would count as external imports. We should make this clear in docs.

[wincent]

We should make this clear in docs.

And even before that, I want to make sure that the proposal can be automatically enforced.

• ESLint has a built-in "sort-imports" rule that sorts based on the LHS. There are a bunch of forks and extensions of that rule; examples turned up in a quick search:
  • https://www.npmjs.com/package/eslint-plugin-import-order-autofix
  • https://www.npmjs.com/package/eslint-plugin-sort-imports-es6-autofix
• import-js sorts based on the RHS.