chore(js): add lint rules for import order: eslint-plugin-simple-import-sort

[IanLondon]

Overview

Closes #5624

Settled on eslint-plugin-simple-import-sort for this one. Eslint has https://eslint.org/docs/rules/sort-imports but I don't like it because it cares about the "style" of your import "memberSyntaxSortOrder": ["none", "all", "multiple", "single"] whereas we care about alphabetical and ../../../ nesting depth ordering but we don't want import something from '../something to be sorted differently than import {something} from '../something. (It also didn't seem to autofix properly but I didn't try too hard after finding out about memberSyntaxSortOrder)

Changelog

• eslint-autofix all JS imports
• manually fix app epic tests that rely on a side-effecting mock import

Review requests

This PR is 2 commits, the first adds eslint-plugin-simple-import-sort and contains eslint autofix changes for all js projects, the second manually fixes failing jest tests in app for several epics -- these failed when imports were re-ordered, because they relied on importing files that set up jest mocks before importing other files. To fix this, I imported the mock-setup file at the top.

• Happy with eslint-plugin-simple-import-sort and .eslint.js?
• (App people) does second commit look good, or should the side-effecting mock setup imports be addressed some other way?
• Smoke test all JS projects

Risk assessment

medium. If there are any side-effecting imports in JS land, re-ordering the imports could break them. Hopefully we only rely on side-effecting imports in tests (for mocking), and we'll get ❌'s if the test fail.

[Kadee80]

🍾 This is some first class charlie work!

[mcous]

From looking at these changes and reading through the README, I think I would choose the eslint-plugin-import/order rule over this plugin, especially considering the fact that eslint-plugin-import is already in our build toolchain.

Unfortunately, neither eslint-plugin-import/order nor eslint-plugin-simple-import-sort allow configuring "put type imports last". For me personally, I'd rather have no import order checking than being forced to mix my type imports and code imports, which would harm how I like to read the code

[mcous]

Can't say I'm a fan of this. "Import the mock before you import the code under test" is my preferred way to do this, which is how things were ordered before

[IanLondon]

I'm not sure what you mean, isn't this importing the mocks before any other imports?

[mcous]

It is, but we only need it because this plugin arbitrarily put them out of order in the first place. The "after" here seems arbitrary and (as noted in my other comment) not what the plugin is supposed to do

// before re-ordering
// import mocks
import { setupEpicTestMocks, runEpicTest } from '../../../robot-api/__utils__'
// import other stuff
import * as Fixtures from '../../__fixtures__'
import * as Actions from '../../actions'
// import code under test
import { calibrationEpic } from '..'

// after re-ordering
// import mocks but only for their `jest.mock` side effects
import '../../../robot-api/__utils__/epic-test-mocks'
// import code under test
import { calibrationEpic } from '..'
// import mocks and other stuff
import * as Fixtures from '../../__fixtures__'
import { runEpicTest, setupEpicTestMocks } from '../../../robot-api/__utils__'
import * as Actions from '../../actions'

[mcous]

According to the documentation of the plugin:

Since “.” sorts before “/”, relative imports of files higher up in the directory structure come before closer ones – "../../utils" comes before "../utils". Perhaps surprisingly though, ".." would come before "../../utils" (since shorter substrings sort before longer strings). For that reason there’s one addition to the alphabetical rule: "." and ".." are treated as "./" and "../".

This seems to be the exact opposite of what has happened in this file, which is confusing to me

[IanLondon]

confuses me too!

[shlokamin]

me three

[shlokamin]

For me personally, I'd rather have no import order checking than being forced to mix my type imports and code imports, which would harm how I like to read the code

I agree with this. Annoying that we can't have types go last :/