eslint-plugin-import: import order ignores imports that don't assign identifiers (unassigned)
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.
About this issue
- Original URL
- State: open
- Created 6 years ago
- Reactions: 11
- Comments: 24 (11 by maintainers)
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).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.
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.
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.
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.
@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.
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.
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 theorder
rule in the first place. That’s my opinion, just putting it into the discussion ¯\_(ツ)_/¯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:
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.+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 😃
@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.