Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule proposal: import-path-order #324

Open
MrHen opened this issue Jun 20, 2019 · 9 comments
Open

Rule proposal: import-path-order #324

MrHen opened this issue Jun 20, 2019 · 9 comments

Comments

@MrHen
Copy link
Contributor

MrHen commented Jun 20, 2019

I can't get what I want out of either ESLint's sort-imports or import/order and it looks like both rules are stuck in a world of too many configuration options and debate. What I want feels simple, though:

  • Sort all import lines by path
  • Relative paths (../foo) are sorted after absolute paths (foo)
  • ../../foo is before ../foo

I don't think this rule would be compatible with either of the above linked rules and I don't think it would need to be.


Fail:

import * as bar from 'bar.js';
import * as foo from 'foo.js';
import {alpha, beta} from 'alpha.js';
import {delta, gamma} from 'delta.js';
import a from 'baz.js';
import b from 'qux.js';

import w from './foo';
import x from '../foo';
import y from '../../foo';
import z from 'foo';

Pass:

import {alpha, beta} from 'alpha.js';
import * as bar from 'bar.js';
import a from 'baz.js';
import {delta, gamma} from 'delta.js';
import * as foo from 'foo.js';
import b from 'qux.js';

import z from 'foo';
import y from '../../foo';
import x from '../foo';
import w from './foo';
@sindresorhus
Copy link
Owner

I can't get what I want out of either ESLint's sort-imports or import/order and it looks like both rules are stuck in a world of too many configuration options and debate. What I want feels simple, though:

What are you trying to achieve that you cannot get out of those rules with the right config?

Personally, I don't find the sort-import rule useful, but I'm using import/order without any problems, using the default settings.


If we were to make this rule, it should at minimum also support Node.js built-ins first.

@MrHen
Copy link
Contributor Author

MrHen commented Jun 20, 2019

sort-imports does not sort by the path. It sorts by the name of the imported object. Nothing about that is what I want.

import/order only handles ./ and ../; it does not distinguish between ../ and ../../. This means that it doesn't handle complicated hierarchies very well.

The code base I am currently working with ends up with a lot of cross-referencing two or three layers deep. I want the code organized so that everything imported from one module or component are next to each other in the file. This is so I can quickly see what dependencies a particular file has by topic. When sorting by variable name, things are scattered all over the place and it makes the entire import section unreadable.

The rule I am proposing is more like the default behavior for the "Organize imports..." command in VS Code.


A specific example:

Screen Shot 2019-06-20 at 2 55 50 PM

What I want:

Screen Shot 2019-06-20 at 2 48 52 PM


Another example:

Screen Shot 2019-06-20 at 2 52 55 PM

What I want:

Screen Shot 2019-06-20 at 2 54 23 PM


If we were to make this rule, it should at minimum also support Node.js built-ins first.

Agreed. Really, import/order does almost what I want. That's the rule being used in the example screenshots above (before my proposed fixes.)

@zeorin
Copy link

zeorin commented Jun 21, 2019

Have you raised this idea with eslint-plugin-import? If it already does almost what you want, it sounds like that's the place to try first.

@MrHen
Copy link
Contributor Author

MrHen commented Jun 21, 2019

@zeorin No, I haven't. They already have 22 open issues about this rule and those are just the issues that have been tagged properly. I was concerned that it was getting bogged down in opinion farms around all of the various ways one could sort imports.

But yeah, you are probably right; I should probably just start there.

@sindresorhus
Copy link
Owner

it does not distinguish between ../ and ../../. This means that it doesn't handle complicated hierarchies very well.

That sounds like just a simple oversight though. Is there any issue about it on import/order? If not, I think you should open one regardless of the outcome of this issue.


While I agree with your use-case and that's how I like to sort my paths too (except for the newlines between groups), I have a feeling that this might be a slippery slope into an endless amount of issues opened about making it more flexible. Just look at import-js/eslint-plugin-import#951 and how many issues there are about the order rule: https://github.com/benmosher/eslint-plugin-import/search?q=%22import%2Forder%22&unscoped_q=%22import%2Forder%22&type=Issues However, another incentive is to get away from the import plugin which has been a pain since I added it to XO some years ago. It's not well maintained and it has just sooo many bugs.

@MrHen
Copy link
Contributor Author

MrHen commented Jun 21, 2019

That sounds like just a simple oversight though. Is there any issue about it on import/order? If not, I think you should open one regardless of the outcome of this issue.

Alright, I've opened import-js/eslint-plugin-import#1388.

I have a feeling that this might be a slippery slope into an endless amount of issues opened about making it more flexible.

Agreed. I'm very comfortable deliberately keeping the scope of this rule narrowly focused. I don't want it to turn into a swiss army knife rule -- it should just do one thing and do it well. If people want something else from their import ordering we can point them to the more complicated rules.

@sindresorhus
Copy link
Owner

Alright. Let's do this then. I think we only need one option, and that's whether to put a newline between groups. I think it should be off by default.


I don't want it to turn into a swiss army knife rule -- it should just do one thing and do it well. If people want something else from their import ordering we can point them to the more complicated rules.

We should clearly document that fact in the rule documentation.

@MrHen
Copy link
Contributor Author

MrHen commented Jun 21, 2019

I think we only need one option, and that's whether to put a newline between groups. I think it should be off by default.

I am completely comfortable just warning that things are out of order. If we auto-fix, the auto-fix can strip out newlines but I don't see a point in warning just because there is a gap between a valid sorting of imports. If people want to enforce particular gaps they can use import/order -- there shouldn't be anything colliding between this rule and that one.

Alternatively, if you in particular happen to want to warn on extra newlines we can make that the configuration option:

"unicorn/import-path-order": ["error", { allow-blank-lines: false }],

I don't particularly want to mess with the complexity of supporting all the various ways people put new lines between imports. Keep it simple: Allow or disallow.

@sindresorhus
Copy link
Owner

The reason I want to enforce no newline is that people keep putting it there and I have to manually remove it.


Keep it simple: Allow or disallow.

👍

@MrHen MrHen mentioned this issue Jun 30, 2019
7 tasks
@sindresorhus sindresorhus changed the title Rule Proposal: import-path-order Rule proposal: import-path-order Sep 5, 2019
@sindresorhus sindresorhus changed the title Rule proposal: import-path-order Rule proposal: import-path-order Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants