Regression in nested group order after v2.26

[jraoult]

This is a follow-up to #2854 (comment). I'm stuck in 2.26 because of a regression introduced in 2.27. Multiple reverts and updates try to fix the regression, but I still have a few spots where the regression shows up:

This is my configuration:

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

and typically, this used to pass in 2.26:

import { isNotNull } from "option-t/esm/Nullable";

import { combineNullables } from "../util/combineNullables.js";
import type { BigFraction } from "./BigFraction.js";
import { FRACTION_ZERO } from "./fraction.js";

but fails after 2.26 with message: ../util/combineNullables.js import should occur after import of ./fraction.js

[yndajas]

Per #2854 (comment), it seems the issue here is wanting to use groups to enforce line breaks AND wanting the order to matter within groups, whereas by default the point of groups is to define multiple import types that must be grouped together but within which group their type is irrelevant (and optionally to add a new line after each group)

It seems there are few different configuration needs here:

• new line between groups: boolean (already supported)
• order by type within groups: boolean (not yet supported; default false)

The second one has changed a few times with recent releases and the documentation is slightly confusing since it says both "The enforced order is the same as the order of each element in a group" and "They can be mingled together", which are arguably in opposition and causing different understandings?

@jraoult I think what you're after would essentially be true and true in the configuration above?

@ljharb is it worth adding an extra configuration option to support these different use cases? I don't mind having a go at that, although I can't guarantee a quick turnaround!

[jraoult]

@yndajas, I appreciate you taking the time to look at that, and it seems like your proposal could work.

I want to stress that I was happy with how it was pre-2.27. I'm reporting the regression in the following minor version jumps. The ordering inside the nested group changed from 2.27 and it should either be a major version jump or it is a regression and should be restored to the behaviour pre-2.27.

I realise I'm probably nitpicking too much around semver semantic here but maybe it is easier to roll back to previous behaviour than introducing a new configuration?

[yndajas]

I realise I'm probably nitpicking too much around semver semantic here but maybe it is easier to roll back to previous behaviour than introducing a new configuration?

In terms of restoring semantic versioning principles, perhaps it would be a case of reverting the default (and clarifying the expected default in the docs), but I think combining that with a config extension might be useful to support both interpretations of the docs so it doesn't feel like a regression in the opposite direction 😅

[ljharb]

I think that it's important to restore whatever default behavior in v2.26 wasn't broken, and I'm fine with adding options to keep the otherwise-reverted functionality.

[hood]

I confirm this behaviour is totally unexpected on my side too.

My suggested import order post 2.27.0:

import ItemA from "./ItemA";
import ItemC from "../../ItemC";
import ItemB from "../ItemB";

[yndajas]

I confirm this behaviour is totally unexpected on my side too.

My suggested import order post 2.27.0:

import ItemA from "./ItemA";
import ItemC from "../../ItemC";
import ItemB from "../ItemB";

Is that [sibling, parent]? What was it pre-2.27.0?

[hood]

Pre-2.27.0 imports were sorted by depth (as expected):

import ItemC from "../../ItemC";
import ItemB from "../ItemB";
import ItemA from "./ItemA";

Config excerpt:

// ...
"import/order": [
    "error",

    {
      "groups": [
        ["builtin", "external"],
        [
          "internal",
          "parent",
          "sibling",
          "index",
          "object",
          "type",
          "unknown"
        ]
      ],
      "newlines-between": "always",
      "alphabetize": {
        "order": "asc",
        "caseInsensitive": true
      }
    }
  ],
// ...

I also see discussion about this topic here: #2396 (comment)

Which led to this PR (which is yet to be merged): #2885

And other related issues: #2682

[yndajas]

@hood wow there are so many threads on this! It's difficult to synthesise all this and work out what's the original behaviour and what needs adding as a feature to avoid breaking the breaking changes 😅 seems like the documentation has confused others too

[hood]

The PR I linked fixed the issue at least partially. It'd be a good starting point to get that merged first, and then proceed to focus on restoring the remaining wrong behaviours IMHO. At its current state the plugin is not behaving in a predictable manner, and it's not backwards compatible, forcing pretty much everyone using relative imports on 2.26.0. I can help if needed.

[hood]

There is also another PR addressing this: #2721