Eslint-plugin-import: import order ignores imports that don't assign identifiers (unassigned)

[lukeapage]

The import/order rule ignores imports that don't have an identifier e.g.

import 'internal/module';
import 'module'; // completely ignored, even though it is external
import 'internal/module';

It appears to be because of this if statement:

https://github.com/benmosher/eslint-plugin-import/blame/master/src/rules/order.js#L418

and removing the if appears to work on my project...

there are two related feature requests:
https://github.com/benmosher/eslint-plugin-import/issues/505
https://github.com/benmosher/eslint-plugin-import/issues/970

which both kind-of suggest that there should be a new import type "unassigned" - In my case I don't want to differentiate unassigned, I just want them sorted like the other imports.

Would you be ok me removing the if-statement and adding tests that treats unassigned requires like all other requires? as mentioned in one of the other issues it would mean that if the order was important for a project they would have to explicitly disable the rule.

[ljharb]

Unassigned imports have side effects - it’s impossible to safely use a linter to order them, because the order is almost certainly important.

[lukeapage]

It’s true and I wouldn’t disagree it’s unsafe in theory but in my case I have a lot of cases where the order doesn’t matter. Just because you have imports, it doesn’t mean there aren’t also side effects.

Could it be an option as to whether to ignore unassigned imports?

[ljharb]

I guess I’m confused how you have side-effecting imports but the order isn’t important. Can you elaborate?

[lukeapage]

the key is if you have one with side effects and six without, it doesn’t matter the order as long as the side effect import is at the top of the file, above side effect code.

E.g. we have a static redux store kept in a module that just keeps the reference to the global store and returns it with getStore - in tests we have a side effect import test/setupMockStore that sets up the global store with a mock one. Because most of our code doesn’t have side effects, nothing uses the store in the other imports.

We should possibly refactor our code, but I have other examples and an enormous codebase.

[ljharb]

I suppose we could add an option or something to report on unassigned imports using a group - but I’d be very hesitant allowing that to be autofixed.

The distinction here is super subtle, and while i believe you and your use case, i suspect those that can confidently and correctly claim the order doesn’t matter are exceedingly rare.

[jaydenseric]

All imports can have side effects, not just unassigned imports. By the logic of avoiding all risk, the import/order rule should not exist at all.

I would like to be able to enforce all my unassigned imports to the top, as almost always they are independant global polyfills. A lint error when they are accidentally imported after other stuff would catch more side effect bugs than it might hypothetically introduce.

[ljharb]

@jaydenseric that is totally fair, yet the prevailing convention is that "no bindings" means "definitely side effects" and "a binding" means "probably not side effects".

I think an option to do what you want is fine, as long as it's not autofixable or default.

[constgen]

Some time it is necessary to put all side effects in a correct spot (at the very top or the very bottom) which actually can prevent a lot of bugs. I think it worths to have an option in ESLint to enforce this.

In my experience it is very important to include unassigned CSS imports last. Otherwise there may be problems with styles cascading and overriding.

[Orodan]

+1

I understand that unassigned imports should have a precise order most of the time, but in some cases the only requirement is for the import to be here and it would be cool if the plugin could allow us to sort it correctly :)

For example, a css import to inject styles in DOM.
I think it's quite similar to what has been done in this issue : https://github.com/benmosher/eslint-plugin-import/issues/671 for the no-unassigned-import option.

If we understand that in some cases there is no point in blocking unassigned imports, why not considerer to sort it too ? It would be logical for me :)

[electrovir]

Another use case of unassigned imports:

When defining a web component in the browser (window.customElements.define) there's usually no need to import anything from that component's file. However, the side effect of the component being defined needs to take place, thus an unassigned import. Ordering doesn't matter at all with these (since they "pop" in once they're defined).

Imo @jaydenseric's comment:

All imports can have side effects

invalidates the argument that they should not be sorted because of side effect ordering. If ordering matters, then the user of this plugin simply shouldn't use the order rule, or should selectively disable it where needed.

The behavior I'm seeing right now is that the order rule is invalidated when imports are out of order and an unassigned import is in the middle of them but --fix won't do anything about it. This seems _worse_ to me than simply ignoring the unassigned import.

[ljharb]

@electrovir it doesn't invalidate it, because although all of them can, in practice, none of them do unless they're imported without a binding.

It's always fine if a rule is only capable of partially autofixing; as long as it's possible to manually make it pass, then there's no bug.

[electrovir]

I guess it depends on how you define a bug. The auto-fix behaves contrary to how I expect it to. I'd consider that a bug 😅

(Also I'm not sure what you mean in your first paragraph @ljharb ...)

[ljharb]

What I mean is, every import certainly can have side effects - but in actual practice, in actual real life, the only imports with side effects are the ones that export nothing and are consumed with import 'path'. Thus, it's a reliable convention, and it's important this plugin not violate it.

[electrovir]

I agree, that's a reliable convention. What I disagree with is the idea that the order rule violates that convention. If a dev is using side effects where order matters, I think that they shouldn't use the order rule in the first place. That's my opinion, just putting it into the discussion ¯\_(ツ)_/¯

[ljharb]

An autofix must always be safe. If that opinion is one we want the rule to follow, we'd have to remove the autofixer entirely, and then it'd be fine for people to use an override comment for those cases.

That doesn't seem, to me, like a better outcome. Does it, to you?

[okrechyk-gb]

On our project (React SPA) there are many CSS imports like import './index.scss', only one such import per file. So, in our case, it's absolutely safe to place such imports at the end of the imports block (we should do this due to our new import order policy). So we would love to have some option to enable unassigned imports for our case (I believe, that it could help many React teams).