Add import-order rule

[jfmengels]

This is a port from https://github.com/jfmengels/eslint-plugin-import-order.

Now that the project type is available in importType, we could add it in the order list. It should not have to be there by default though, so we have to loosen up the JSON schema.

And while we're at it, I like the flexibility of the grouping described here #246 (comment), which can complement what already exists too (the old way would still be possible).

Let me know what you guys think of these, and if there's anything I forgot.

cc @singles @sindresorhus @duncanbeevers

task list:

• rename to order
• support groups
• inferred final group contains all unspecified types
• sort require independently as a second group

(@jfmengels: just added this to aggregate ideas from thread and track, you can check off/edit/delete as you see fit --ben)
(@benmosher: All checked, thanks :D)

[benmosher]

Looks good to me, if it works for you. I don't have a lot of opinions about this, beyond just supporting the notion of the project type as separate from external (which you should feel free to rename, I'm not sure that's a great moniker for it).

I did just push some README/CHANGELOG changes that caused the conflicts.

Can you rebase and update CHANGELOG to reference this PR? Also, thank yourself? 😉

[jfmengels]

I think I'll apply the changes I proposed above tonight, if you don't mind waiting a bit before merging this.

which you should feel free to rename

What about internal?

[benmosher]

internal works for me!

As for merge timeline, totally up to you. Let me know when you're ready. 😄

[jfmengels]

I'd like some thoughts on what we should do if some types are omitted. Should we ignore those imports, lint an error if they are encountered (saying that they should cover it in the configuration), give a default to each rule, ...?

[benmosher]

if some types are omitted

i.e. spec is just order: [ external, internal ]?

I imagine there being an implicit rest type at the end, so any defined types come first, and any omitted types must be strictly after the configured types, but in any order.

So absolute-first could be achieved via order: [ external ], for example. (assuming I understood the intent of #246, and ignoring the whitespace bit for the moment)

[jfmengels]

Hmm, so

order: [
  [ "external", "internal" ]
]

would implicitly be

order: [
  [ "external", "internal" ],
  [ "builtin-in", "sibling", "index", "..." ],
]

I like it.

So absolute-first could be achieved via order: [ external ], for example.

I think so yes.

I think that I'll add internal by default between external and sibling. Or should it be grouped with sibling? Any chance that users that have NOT configured their path/webpack could get bitten by it, or will an import be considered internal only if the path or webpack are configured?

[benmosher]

I think that I'll add internal by default between external and sibling.

Makes sense to me, I'd expect it to come after external packages and before relative imports.

will an import be considered internal only if the path or webpack are configured?

That's right, I think any unresolved paths import modules will be considered external for compatibility with your existing userbase.

Resolved paths that aren't rooted in node_modules will be considered internal, though; I was thinking about how bower_components and jspm_modules currently fall into that camp. Webpack config has a notion of moduleDirectories that could be leveraged for this.

I think it probably makes sense to wait for the issue to be filed by the 1 person who is using Webpack, Bower, and import-order, though.

[benmosher]

Also, brain blast: should this rule be named order now? then the config is

---
rules:
  import/order: [ error, groups: [...] ]

Seems more natural than import/import-order.

I've also renamed the order config key to groups there, for dramatic effect.

No worries either way, just dawned on me, and now is a better time than later.

[jfmengels]

• Allowed grouping
• Renamed project type to internal
• Added internal type to default groups
• Made omitted types as last
• Loosened JSON schema (:sad:)
• Lint an error (and that's it) if configuration has duplicates or elements that are not in the allowed types
• Renamed param order to groups
• Renamed rule to order

Previous and default behavior still work :)
Ready for review :D

[benmosher]

Sweet! If you're happy with it, I think it's probably all good, but I'll try to take a look in the next day or two! Out of town this weekend so it might be a bit, though. Just heads up.

[jfmengels]

No rush here :)
If someone else has any comments, now is the time. After this gets merged (and released), I'll probably deprecate eslint-plugin-import-order ( 😢 ).

[sindresorhus]

LGTM

[sindresorhus]

@jfmengels Is jfmengels/eslint-plugin-import-order#10 taken into account here?

[jfmengels]

Ah no, didn't think of adding that.

I could add it, though not sure whether to add it as an option (probably), and if so, enabled by default (probably not)?

What do you think about {"putFirst": "import|require"} (or should it be "ES|AMD"?), and allow them to be mingled if omitted?

[sindresorhus]

Why would you want it as an option? This is about sorting them in separate steps. There's no good reason to mix them. import should be above require as it's hoisted above it by the engine internally anyways.

[jfmengels]

That makes sense 👍
I'll try to get around to that then (probably not tonight though)

[benmosher]

I just added an item to the checklist, just trying to remind myself so I don't prematurely merge. Seems like a good thing to get in before this incarnation is published.

[jfmengels]

Separated import and require by default, was an easier fix than expected :)

Also, updated importType to account for scoped packages like @cycle/core (as external), totally forgot about those!

Let me know what you think. (I rebased, but only the two last commits are responsible for the changes this time)

[sindresorhus]

LGTM

I'm super excited about this rule. Great work @jfmengels ✨

[benmosher]

I'm happy with it whenever you guys are. 😎

I'd merge it now but I'm still AFK and I think merging the no-nodes-modules created the merge conflicts.

@jfmengels, if you want to go ahead and merge directly to master that's fine with me, or if you want to just rebase I can push the button.

[jfmengels]

@benmosher Rebased it (yes, it was my own no-nodejs-modules that bit me :p)
I'll merge as soon as the tests pass, and then I'll go and deprecate my package... (:cry:)

[jfmengels]

Well, that was a longer process than expected, but it went well :)

[benmosher]

I think so too.

And your package lives on here! 😄

[jfmengels]

And your package lives on here!

And even better than before!

[calebmer]

🎉💃 can't wait for a release!

[singles]

Awesome job @jfmengels!

I'm implementing that rule now and just noticed that if we have order rule then absolute-first option for imports-first is redundant and could be replaced with something like:

"import/order": ["error", {"groups": [
    ["builtin", "external"], //whatever order here
    ["internal", "index", "sibling", "parent"] //whatever order here
]}]

[jfmengels]

@singles Thanks! And yes, you're right : #259 ;)