eslint-plugin-import: `import/order` fixer is not working properly?

I am trying to use eslint with the plugin in my project, and I was happy when I saw that there are fixers for import ordering, so I don’t have to visit hundreds of files to satisfy the linter.

+(fixable) The --fix option on the [command line] automatically fixes problems reported by this rule.

^{(from https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/order.md)}

Unfortunately, in quite a lot of cases it does not work.

I use both plugin:import/recommended and plugin:import/typescript (project uses Typescript/React (.ts/.tsx)).

Rules (only import/* listed):

		"import/no-named-as-default-member": "off",
		"import/newline-after-import": "warn",
		"import/no-duplicates": "warn",
		"import/order": ["warn", {
			"groups": ["builtin", "external", "internal", "parent", "sibling", "index", "object"],
			"pathGroups": [
				{"pattern": "@flyparks/**", "group": "internal", "position": "before"}
			],
			"newlines-between": "never",
			"alphabetize": {
				"order": "desc",
				"caseInsensitive": true
			}
		}]

Settings (only import/* listed):

		"import/resolver": {
			"webpack": {
				"config": "../node_modules/react-scripts/config/webpack.config.js"
			}
		},

Example

File (only imports):

import { useHistory } from 'react-router-dom';
import React, {useEffect, useState } from 'react';
import { BookingEntry, getPathToBookingPage } from '@flyparks/models';
import './AccessBookingNoAuthPage.scss';
import { CircularProgress, TextField } from '@material-ui/core';
import { httpsCallable } from 'firebase/functions';
import FirebaseServices from 'utils/FirebaseServices';

Linter output:

   3:1   warning  There should be no empty line between import groups                           import/order
   7:1   warning  `@material-ui/core` import should occur before import of `@flyparks/models`   import/order
   8:1   warning  `firebase/functions` import should occur before import of `@flyparks/models`  import/order

Using --fix does nothing, despite I imagine removing empty lines should be easy (its obviously detected already, isn’t it?). Reordering the lines also should be quite easy…

Is there issue with Typescript/React files maybe?

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 19 (8 by maintainers)

Most upvoted comments

@AgainPsychoX I can suggest you prettier plugin for this job. It has some downsides, but works better for me.

sk1e on Jul 21, 2021

I still disagree with you and think that there are two good arguments to include imports without bindings in the rule:

You say that such dependencies happen all the time in codebases, but for me it never happen. I would say that such dependencies are a sign of a bad, very bad module design. It breaks abstraction in many ways, It even hardly can be named “module” because it becomes not modular. I would never design module like that and I would never use modules like that. So the line you draw affects me in a negative way. I think there are plenty of users like me.
It should be up to user to decide if he or she can use unsafe feature, but unsafe feature should be provided anyway if it can help user and there are no safe alternatives. There are a lot examples of software that provide unsafe features: git force push, reset, rebase, dangerouslySetInnerHTML in react elements. If feature considered unsafe we can: 2.1. Do not include it in default behavior, so none of users will be touched by unsafety unless user explicitly want it. 2.2. Add “unsafe” prefix to option, so user will take responsibility for using unsafe feature 2.3 Add mention to documentation that it is unsafe and why it is unsafe.

sk1e on Jul 20, 2021

Well, imports with bindings also could have side effects, so their ordering is not safe either. We can also have imports with bindings and side effects and imports without bindings and without side effects. Existence of bindings does not tell us anything about side effects. Nothing can tell, therefore import ordering can’t be safe at all.

We make software to automate some work. It sounds ridiculous to me to do that work manually because it is not safe in cases that are nonsense. It is not safe in other nonsense cases though, so it is even more ridiculous.

sk1e on Jul 19, 2021

The argument I’m seeing is that this option is not being considered because imports without bindings can have side effects, and these side-effects make re-ordering dangerous.

However, as @sk1e pointed out, even imports WITH bindings can have side-effects, and @ljharb you acknowledged this yourself:

Yes, that is all true - we had to draw a line somewhere, and this is where it’s been drawn.

It’s clear that there are a bunch of people in the community who support having this option and have listed a number of reasons for why it’s a valid feature. I don’t understand why a line has to be drawn here. I don’t think it has to be. Safe defaults are a great idea and I commend your resolve, but allowing flexibility here would better support the needs of the community, and is a win-win for everybody. The defaults can remain safe, while others can manually adjust those settings to what works best in their organization’s coding standards.

Thanks @sk1e for suggesting the Prettier plugin, it worked great for me.

thecodeboss on Feb 3, 2022

Experiencing the same issue. I think that safety should not take priority over usability. User could be provided with unsafe option, if that option can really help him.

I have hundreds of components with such css imports and I don’t have imports that can change program in any way because of import order changed. Program that depends on import order is a bad program, most likely.

Eslint’s fix function could have much higher value in my case and probably many other cases if rule could affect imports with side effects.

sk1e on Jul 19, 2021