eslint-plugin-import: [import/order] Regression in behavior with parent and sibling

Since Version 2.27.0 we’ve recognized a change in over 500 files with the ordering rule:

        "import/order": [
          "error",
          {
            "groups": ["builtin", "external", "parent", "sibling", "index"],
            "newlines-between": "always",
            "alphabetize": {
              "order": "asc",
              "caseInsensitive": true
            }
          }
        ],

For example:

@my-local-package/mysub-path @my-local-package/mysub/path

They are now handled different than before and leads to hundres of linting errors where no before.

About this issue

Original URL
State: open
Created a year ago
Reactions: 15
Comments: 18 (5 by maintainers)

Commits related to this issue

fix: https://github.com/import-js/eslint-plugin-import/issues/2685 — committed to tjx666/eslint-config by deleted user a year ago

Most upvoted comments

Quick progress update, I have a fix for this working locally. I’m working on adding a few more unit test cases to maintain code coverage. I should have a PR up by the end of the week.

+12

Pearce-Ropion on Feb 1, 2023

According to the 347d78b change, this is not a bug. The change was made to specifically address this specific use case (as far as I understand it)

I would hazard to say it’s a bug since the expected behavior, as described in the docs is not the actual one.

notaphplover on Feb 5, 2023

We use a similar configuration to @JohnnyDevNull in a large codebase and the import-order rule considered this order valid in 2.26:

import abc from '../../abc'
import def from '../def'
import ghi from './ghi'
import jkl from './jkl'

But in 2.27 it’s forcing the following order, which doesn’t make sense to us:

import ghi from './ghi'
import jkl from './jkl'
import abc from '../../abc'
import def from '../def'

It looks like our only workaround right now would be to change our config to put "parent" and "sibling" into different groups, forcing adding a blank line between parent and sibling imports.

This regression does seem to come from this change:

According to the 347d78b change, this is not a bug. The change was made to specifically address this specific use case (as far as I understand it)

Looking at https://github.com/import-js/eslint-plugin-import/pull/2396, it looks like the intent was to sort forward slashes (/) before hyphens (-), so that alphabetical order would be:

something
something/subpath
something-else

But the change also triggered sorting the / character before dots (.), resulting in the issues reported here for parent and sibling imports in the same alphabetized group.

So the importsSorter function should probably be expanded on a bit to sort dots before slashes, hopefully fixing this while still passing the tests from #2396.

fvsch on Jan 29, 2023

Quick progress update, I have a fix for this working locally. I’m working on adding a few more unit test cases to maintain code coverage. I should have a PR up by the end of the week.

@Pearce-Ropion How’s this coming along?

I’ve put up #2721. I think it needs more unit testing but figured I’d get the PR out there

Pearce-Ropion on Feb 21, 2023

for anyone can’t wait, recommend: https://github.com/lydell/eslint-plugin-simple-import-sort

tjx666 on Aug 23, 2023

Seems many people stay at 2.26.0 because of this issue: Screenshot_2023-05-09-03-54-26-944-edit_com mmbox xbrowser

tjx666 on May 8, 2023

According to the https://github.com/import-js/eslint-plugin-import/commit/347d78b678a772d5d04e56ff36c131e46d0423ef change, this is not a bug. The change was made to specifically address this specific use case (as far as I understand it)

Pearce-Ropion on Jan 19, 2023

Still not fixed, staying at 2.26.0

JohnnyDevNull on Dec 15, 2023

Still not fixed, staying at 2.26.0

JohnnyDevNull on Dec 15, 2023

Quick progress update, I have a fix for this working locally. I’m working on adding a few more unit test cases to maintain code coverage. I should have a PR up by the end of the week.

@Pearce-Ropion How’s this coming along?

jefflage on Feb 13, 2023

We’re also downgrading to 2.26.0 in TinyMCE to avoid this issue, the PR that resulted was too much https://github.com/tinymce/tinymce/commit/841176b5013c113c6be4605ccf8d06e9c417e43d

TheSpyder on May 19, 2023

@JohnnyDevNull how many of those changes are fixable with the autofixer?

It takes too long to run. We have round about 300 libraries in a NX Monorepo. It’s impossible to get this through a code review without having new merge conflicts within a week. The only solution is to keep the local path as before or uninstalling the plugin sadly to say.

So we stay at 2.26.0 until it’s fixed or we have to remove it because of eslint version upgrades.

JohnnyDevNull on May 9, 2023