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:
#505
#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 : #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).

[tylertrotter]

I'd just like to add to the cacophony of voices saying this is important. We had a bug caused by the side effects of css imports being in an unpredictable order. We decided to fix it by always importing css first. It was exciting to find this plugin to hopefully enforce our new rule, only to learn that the very side effect bug we're trying to prevent with this plugin is the reason why this plugin won't allow us to use it. Seems like a catch-22 (or something).

[mikestopcontinues]

For my projects, I've found that import 'package'; can all always go safely at the top of the imports. This covers css imports, polyfills, and dotenv. I would not complain if this group ignored alphabetization, but otherwise, it's relatively safe—and completely safe if it's opt-in rather than opt-out.

[StephanBijzitter]

Perhaps an option can be made to configure the position of side-effects?

For example, dotenv is indeed a common dependency most people import at the very first line of a file and *.scss is most often imported last. If we could enter glob patterns in the config to tell ESLint where we would expect these kinds of imports to be, that'd be super sweet.

Personally, I just find it really annoying that my IDE (intelliJ) automatically appends all imports at the end of the list and that often leads to imports coming after an SCSS import, preventing ESLint from applying automatic fixes.

[ljharb]

I do think a reasonable middle ground might be a config option to indicate which bindingless imports do not in fact have side effects, and their ordering.

[GoodbyeNJN]

Maybe we could consider the "side-effect import" as a group, then the order of groups could be configured properly.
In addition, an option should be provided to control whether imports should be automatically sorted in the "side-effect import" group or not.

[ljharb]

Side-effecting imports often need to be run in specific orders, both before and after normal imports, so I don't think it makes sense to automatically lump together everything side-effecting.

[andrezero]

The CSS use is so common that it might be interest to address it seperately?

In every single component file of every project that uses pure CSS imports, this happens:

• type a name
• use the IDE auto import feature
• the new import is added at the end of the list
• save the file (set to auto-fix)

• nothing happens
• fix the order manually

I understand the concern about side effects, but when we have something like pathGroups that could explicitly transfer the responsibility to the consumer

                {
                    pattern: './*.css',
                    group: 'internal',
                    position: 'after',
                },

I tried that before coming here to find that unassigned imports are simply ignored.

[andrezero]

Another idea would be to add an config option to sortUnassignedImports: boolean (default false)

People can always disabled the rule where it matters, which would be an awesome way of immediatelly documenting that import order matters for this particular line

// eslint-disable-next-line import/order
import '../THIS ONE MATTERS';

import { BreadcrumbFolderIcon } from '@/atoms/BreadcrumbFolderIcon/BreadcrumbFolderIcon';
import { useUrl } from '@/navigation/useUrl';

import './FolderLink.css';

In the project I am working on there are 139 '.css' imports and only in 1 file there's a couple that need to be in a certain order. And even there I would rather rename the files to have alphabetic auto ordering than use the disable comment 😅