Enable linting of import order and sorting

[randycoulman]

We've historically had a rough convention for how we structure imports, but this convention has been largely unenforced by this lint config, and has suffered from a lack of tool support (i.e. eslint --fix).

Here are our current conventions as I understand them:

• all imports appear at the top of the file (enforced by the import/first rule

• there is a blank line after the imports before the rest of the code (enforced by the import/newline-after-import rule

• imports are broken into groups:

  • external dependencies, including from node_modules and/or node builtins
  • internal dependencies from other top-level modules within the codebase
  • parent dependencies (import Foo from '../components/Foo')
  • sibling/child dependencies (import Foo from './Foo')

• groups are separated by newlines

• within groups, sort lines alphabetically by the filename we're importing from

• when importing multiple items from a single file, sort the items alphabetically

• non-JS imports often come last (e.g., import styles from './styles.scss')

Because of the lack of enforcement/tool support, these conventions likely vary from project to project.

In addition to the rules we already support (mentioned above), we have the following options that I know of:

• import/order: this rule would allow us to enforce our grouping rules and the newline separation

• sort-imports: this rule would allow us to enforce sorting within multiple imports, and sorting of imports by the name of the first item being imported on a line (rather than the filename being imported from). However, it doesn't play nicely with import/order and is unable to auto-fix the ordering of import statements. It does auto-fix within multiple imports, at least.

• A new plugin, eslint-plugin-sort-imports-es6-autofix: I haven't tried this one, but it claims to address some of the limitations of the sort-order rule. In particular, it will auto-fix the order of import lines, and it also, "respects whitespace and comments between imports by only looking at (and sorting) consecutive import statements (those without newlines/comments in between them)."

• An open PR on eslint-import's import/order rule that adds the ability to sort import lines within groups by filename.

In order to get us some tooling support for imports, I'd like to propose the following. I'm doing this as an issue first to gather some consensus before going ahead with the changes.

1. Enable the import/order rule with the following configuration:

{
  groups: [['builtin', 'external'], 'internal', 'parent', ['sibling', 'index']],
  newlines-between: 'always'
}

This gets us the high-level structure of our import blocks.

2. Spike with the new plugin to see if it works well with the import/order rule. If it does, we'd change our convention to sort by imported name rather than filename with the benefit of having full tool support for our convention.

3. If that fails, try the new sort: 'alphabetical' option to import/order once it is merged (or by pointing at the PR branch). This would allow us to have tool support for our current within-group sorting rule, but still doesn't solve sorting of multiple imports in the same import statement.

Thoughts? Anything I'm missing?

[randycoulman]

I experimented with this on a couple of projects. The import/order rule works pretty much as advertised. The only oddity I ran into is that it didn't treat import reducer from '..' and import { action } from '../../actions' as being in the same group for some reason.

The eslint-plugin-sort-imports-es6-autofix plugin seemed to work OK, except that there's no way to not make it care about the single/multiple/all/none stuff that the original sort-order rule cares about. And I still find it weird to sort by the symbol being imported instead of the file being imported from. I think this will result in more churn in our imports, which I'm not a fan of.

I was not able to test out the forthcoming PR to the import/order rule. I pulled down the branch, but eslint couldn't find the plugin. I think there's some kind of build/publish step needed to make that work and I was not able to find the right incantation. I think that PR will get us closer to a desirable state.

I really wish there was a rule that would just enforce the sorting of imports within a multi-import statement and nothing else.