import/order relative import sorting order changed in 2.27.0

[remcohaszing]

Given the following rule config:

module.exports.rules = {
  'import/order': [
    'error',
    {
      alphabetize: { order: 'asc', caseInsensitive: true },
      groups: ['builtin', 'external', 'internal', ['parent', 'sibling', 'index'], 'object'],
      'newlines-between': 'always',
      warnOnUnassignedImports: true,
    }
  ]
}

In eslint-plugin-import 2.26, sibling imports were sorted after parent imports. In versions 2.27.0 to 2.27.5, siblings are sorted before parents.

Here’s an example of previously valid, but now invalid code:

import { fooz } from '../baz.js'
import { foo } from './bar.js'

[remcohaszing]

I think this is also what @cojoclaudiu meant with https://github.com/import-js/eslint-plugin-import/issues/2669#issuecomment-1382716158, but it appears to be unrelated to that issue.

[btecu]

Also having issues with the import/order getting: error `@ember-data/model` import should occur after import of `@ember/template` import/order

[ljharb]

Indeed, this seems to be broken. cc @Pearce-Ropion

[Twipped]

Definitely should have been a breaking change, at a minimum.

[Pearce-Ropion]

Given that its a bug, I don't think a breaking change would have made sense since it wasn't intentional.

[ljharb]

@Twipped indeed, it's only a breaking change if it's intentional (and i'm not sure what would be larger than "breaking change")

[Pearce-Ropion]

Ok, I've confirmed that this is caused by the alphabetical sortingFn introduced in https://github.com/import-js/eslint-plugin-import/commit/347d78b678a772d5d04e56ff36c131e46d0423ef. The sorter is sorting parent imports after siblings because ../ is sorted after ./ alphabetically. Currently the sorting fn has no knowledge of nested groups, it will just sort all elements that are in the same group alphabetically.

@ljharb What is the expected behavior for nested groups? The docs aren't very clear. https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/order.md#groups-array

In the description for groups it says:

The enforced order is the same as the order of each element in a group.

But in the example given for groups it says that nested groups can be mingled together

> ['sibling', 'parent'], // Then sibling and parent types. They can be mingled together

And then there is nothing in the alphabetize description that relates to the sorting of nested groups.

---

My assumption is that we should sort nested groups in element order and then alphabetically. If this is the case, the alphabetical mutations will need to be aware of the group order which will make that code much more complicated.

Alternatively, we can change how the initial ranks are set by increasing the rank within nested groups in addition to just between groups.

Thoughts?

[ljharb]

I think that the primary thing is to avoid the regression - so the default behavior should remain "whatever it did before".

However, if the ideal behavior we decide on is different, we should also provide an option for that behavior.

I would assume that ['sibling', 'parent'] means that neither one should have any preference. If they want siblings sorted before parents, they'd do 'sibling', 'parent' without the array.

[Pearce-Ropion]

I would assume that ['sibling', 'parent'] means that neither one should have any preference. If they want siblings sorted before parents, they'd do 'sibling', 'parent' without the array.

Unless you want sibling to be sorted before parent without newlines.

[ljharb]

I believe that's what the distinctGroup option is for?

[jraoult]

Like @Pearce-Ropion, I've been relying on the fact that nested groups respect the array order and then set "newlines-between": "always" to get a split between them.

I believe that's what the distinctGroup option is for?

The doc seems to imply that distincGroup is for pathGroups only (cf. https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/order.md#distinctgroup-boolean). So are you saying that to get stable ordering in a nested group and newlines between groups, we should use pathGroups instead of groups?

[ljharb]

ah, fair point. We don't seem to have a way to avoid newlines between groups.

or, rather - the only way we currently have to signal "treat these two groups differently" is putting them in an array. One thing that could imply is "don't newline between them, but keep their relative ordering", and another thing that could imply is "treat all groups in the array as being of equal rank".

It can't imply both, so which one should it imply?

The enforced order is the same as the order of each element in a group

This in the docs implies to me that it should be the former. If that's what previous versions of the plugin in v2 do, then I think that's what we should go with - and if someone wants the intermingling, then we'd need a new feature request filed for that, after this bugfix is done.

[Pearce-Ropion]

So in order to fix the regression, we would essentially need to revert https://github.com/import-js/eslint-plugin-import/commit/347d78b678a772d5d04e56ff36c131e46d0423ef.

The following uses the example ../barz.js and ./bar.js:

The problem is that the new sorter (v2.27+) splits sorting of import paths by the / character (added in https://github.com/import-js/eslint-plugin-import/commit/347d78b678a772d5d04e56ff36c131e46d0423ef). This results in an attempt to sort .. and . instead of the full path. This results in ./bar.js being placed before ../barz.js.

In 2.26 and below, we attempt to sort on the the full import path. Thus ../barz.js and ./bar.js results in ../barz.js being placed before ./bar.js.

---

It is worth noting that before the "alphabetization" takes place, both the parent and sibling imports have the same rank. So in both v2.26 and v2.27+, the ordering of the groups in the nested array has no affect on the final order of the imports, just that they are placed together without new lines.

Instead of reverting, one option could be adding a case that specifically targets parent imports and places them before sibling imports. The question would then be, what if the nested group array had sibling placed first (ie, ['sibling', 'parent', 'index'],)? Should we only do this if parent specifically comes before sibling?

---

So in my mind, a real fix would be to change the alphabetical sorter to sort based on the original groups configuration option and not just the ranks.

[ljharb]

Seems like you have the real fix in mind :-) a PR would be appreciated!

[Pearce-Ropion]

So the only issue with the proposed fix is that it will probably change the sorting for many people across the board since it won't just affect the sorting in this parent/sibling case. It will change how alphabetical sorting works for every nested group which may constitute a minor or major version change.

[ljharb]

hmm. i think the first thing would be to address the regression without breaking any other tests.

if it's going to drastically change the ordering then it's probably a breaking change and it'd have to sit for awhile.

[Pearce-Ropion]

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

[mihkeleidast]

I've filed https://github.com/import-js/eslint-plugin-import/pull/2885, which should fix the regression, while keeping everything else working as-is.

[alex-tsx]

Low-cost way to deal with this was to change this setting

  "groups": [["external", "builtin"], "internal", ["parent", "sibling", "index"]],

into:

  "groups": [["external", "builtin"], "internal", "parent", "sibling", "index"],

Then you will only need to insert empty lines between parent & sibling import groups in your codebase (low-cost part).

And I highly recommend doing this, if you rely on import/no-cycle rule. Bumping up from v2.26 to v2.29 reduced ESLint checks for us by ~ 6 minutes !

[gustavopch]

I just tested v2.30.0 (https://github.com/import-js/eslint-plugin-import/pull/2885) and it still sorts index differently from what I expect (i.e. differently from v2.26.0).

My config:

{
  alphabetize: { order: 'asc' },
  groups: [
    ['builtin', 'external'],
    ['unknown'],
    ['parent', 'sibling', 'index'],
    ['type'],
  ],
  'newlines-between': 'always',
}

What I expect (because index is the last item in the group):

import { foo } from '../app/foo.js'
import { index } from './index.js'

What it actually does:

import { index } from './index.js'
import { foo } from '../app/foo.js'

[ljharb]

@gustavopch i think the idea was that that’s a bug fix, because actually “./index.js” isn’t an index, it’s a sibling. “./“ would be an index.

[Trainmaster]

Tested with 2.30.0:

import { Foo } from '../Foo';
import { Bar } from './Bar';

-> No error

import { Foo } from '../someDirectory/Foo';
import { Bar } from './Bar';

->

2:1  error  `./Bar` import should occur before import of `../someDirectory/Foo`  import/order

[ljharb]

again, that seems like a bugfix to me - ./ should come before ../.

[rgant]

Seems like something that should be configurable as there are several code styles.

[remcohaszing]

I would say an import of a closer file should be sorted later. My mental model distinguishes the following groups:

Runtime builtin:

• bun:test
• node:path

Dependencies:

• lodash
• react

Internal:

• #some-internal

Relative:

• ../../grandparent-sibling.js
• ../parent-sibling.js
• ./sibling.js

So when comparing relative path segments, .. should come first, . second, and everything else alphabetically.

Relative imports used to be sorted that way, until it broke in 2.27.0.

[simon-abbott]

I've given up on this being fixed and have migrated to using simple-import-sort/imports instead of import/order.

[Pearce-Ropion]

The problem is that in the current implementation, items in a group are not sorted in the order they are listed. It just bundles all items in a group together with no official sorting. It gives an error because it expects some amount of ordering, but there is no enforcement of the order listed in the group in the config.

https://github.com/import-js/eslint-plugin-import/pull/2721 would still solve this issue and give you full control of the intra-group ordering.

[Trainmaster]

again, that seems like a bugfix to me - ./ should come before ../.

According to the documentation the default is

["builtin", "external", "parent", "sibling", "index"].

[ljharb]

ah, true. if that's your config then ../ should indeed come before ./