feat: add eslint import plugin for sorting and other validations to the import section

[zebateira]

To add advanced sorting validation to the import section, I propose using eslint-import-plugin:

• import/order: sort order will be the default group sort "builtin", "external", "parent", "sibling", "index", then alphabetical

Extra:

• import/first: do not allow import statements to be defined outside the import section
• import/no-commonjs: replace prefer-import with import/no-commonjs, import/no-amd and import/no-dynamic-require.

There are a couple of caveats with the proposed solution:

1. Scoped require statements no longer show the warning to use the import statement (since it's not possible on a scope). Is this still something we want to flag as an issue? (see test fix here)
2. Alphabetic sorting within import groups leads to an unfortunate order of prop-types being imported before react, so we "lose" the "importance" sorting we had (see test fix here)

Observations:

1. I tried to make newlines mandatory between each import group, but it didn't work, so we'll have to make a PR to the plugin to fix this (rule import/order - newlines-between) - either that or I'm doing something wrong.
2. There's a useful option caseInsensitive in the import/order rule but it hasn't been released. When it is, would be nice to turn it on as well.

[codecov[bot]]

Codecov Report

Merging #79 into master will not change coverage. The diff coverage is n/a.

@@          Coverage Diff          @@
##           master    #79   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          23     23           
  Lines          30     30           
=====================================
  Hits           30     30

Impacted Files	Coverage Δ
rules/es6.js	100% <ø> (ø)	:arrow_up:
addons/jest.js	100% <ø> (ø)	:arrow_up:
addons/es-modules.js	100% <ø> (ø)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9f5375d...7b5457b. Read the comment docs.

[zebateira]

@acostalima forgot to add the BREAKING CHANGE keywords to the commits to mark this for breaking change. I can just add a commit to mark it as such, or force push with a commit message updated. What do you think?

[satazor]

@zebateira maybe it's better to rebase everything into a single commit and add the breaking part there.

[satazor]

One thing to note. I think we are configuring the rules just for es-modules, when in fact we should do for node as well. Perhaps it would be better to do it in the base config? Or replicate this into the node addon?

[zebateira]

One thing to note. I think we are configuring the rules just for es-modules, when in fact we should do for node as well. Perhaps it would be better to do it in the base config? Or replicate this into the node addon?

That's a good point. These rules should be applied in commonjs scripts as well.

[satazor]

Scoped require statements no longer show the warning to use the import statement (since it's not possible on a scope). Is this still something we want to flag as an issue? (see test fix here)

That's completely fine.

[acostalima]

I tried to make newlines mandatory between each import group, but it didn't work, so we'll have to make a PR to the plugin to fix this (rule import/order - newlines-between) - either that or I'm doing something wrong.

@zebateira the following is not working?

'import/order': [1, { 'newlines-between': 'always' }]

[zebateira]

@acostalima

@zebateira the following is not working?

'import/order': [1, { 'newlines-between': 'always' }]

There's a weird issue: it reports that there should be a newline after every single import statement. Possibly it might work if we use eslint-plugin-import-helpers.

[acostalima]

@zebateira @satazor should we create a RFC to discuss this?

[zebateira]

yah sounds good @acostalima @satazor 👍

[zebateira]

closing this: will create an RFC for discussion

[acostalima]

@zebateira Thanks!