import/order not respecting builtin

[thernstig]

Using "eslint-plugin-import": "2.27.5"

// .eslintrc.json
{
  "root": true,
  "reportUnusedDisableDirectives": true,
  "parserOptions": {
    "ecmaVersion": 2023,
    "sourceType": "module"
  },
  "env": {
    "node": true,
    "es2022": true
  },
  "plugins": ["import"],
  "rules": {
    "import/order": [
      "error",
      {
        "groups": [
          [
            "builtin",
            "external",
            "internal",
            "parent",
            "sibling",
            "index",
            "object",
            "type"
          ]
        ]
      }
    ]
  }
}

// test.js
import chpro from 'child_process';
import log4js from 'log4js';
import util from 'util';

const execFile = util.promisify(chpro.execFile);
const logger = log4js.getLogger('server');

I would expect util to be sorted before log4js but this is not the case. Neither is node:util.

[shawnmcknight]

I ran into this with a different built-in import (path). If I changed the import to use the new node protocol semantics (i.e. 'node:path') then the import order was correct. This made me believe that this detection might be due to a conflicting node core module and a module installed in node_modules. Sure enough, if I look at my node_modules there is a path dependency in there (also a util dependency, which might match @thernstig 's scenario).

If I use a core module like fs where I don't have a conflicting dependency in node_modules then the import order is correct (actually wanted to precede node:path which is what I would have expected).

Taking a guess here that import/order might be getting confused because of the duplicate names in core and node_modules, but that's just a shot in the dark guess.

[thernstig]

@shawnmcknight that is not my experience. The above config does not complain about this ordering:

import express from 'express';
import log4js from 'log4js';
import chpro from 'node:child_process';
import fsp from 'node:fs/promises';

I would expect all node: to end up first.

[ljharb]

@thernstig why do you have the nested array in groups?

[ljharb]

I was able to partially fix this in 1fa2971 - now the same error shows up with a nested group or a flat one - but the ordering still isn't fully correct.

[thernstig]

@ljharb I got that from eslint-config-airbnb-base which I used as a base for creating this issue, so maybe you can answer that. I saw though that https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/order.md does not have the nested array.

But I am using eslint-config-airbnb-base so in the end I get my config from there.

[ljharb]

@thernstig the base config has { groups: [['builtin', 'external', 'internal']] } in it, so something else must be providing that list.

[thernstig]

@ljharb I took it from eslint-config-airbnb-base, but then just added all items from here to test them all:

eslint-plugin-import/docs/rules/order.md

Lines 105 to 121 in 683d3a5

	```ts
	"import/order": [
	"error",
	{
	"groups": [
	"index",
	"sibling",
	"parent",
	"internal",
	"external",
	"builtin",
	"object",
	"type"
	]
	}
	]
	```

Is eslint-config-airbnb-base wrong?

[ljharb]

No, it’s not wrong, i was just curious where the config comes from.

[yndajas]

👋🏻 is the (partial) fix for this correct, particularly this conditional?

After updating to 2.28.0 we're seeing "groups": [["builtin", "external"]] (one group) lead to builtin and external being separated, when I believe these should be grouped together with everything else in a second group. This was the behaviour before 2.28.0 and seemingly what the docs suggest: ['sibling', 'parent'], // Then sibling and parent types. They can be mingled together ("They can be mingled together"). We've had to amend the config to keep the previous behaviour

I might not be fully understanding the original issue in this thread, but I would've thought it would be solved by de-nesting? It looks like the Airbnb config example is: groups: [['builtin', 'external', 'internal']], which would group builtin, external, and internal and then follow with everything else in a second group, so if you wanted builtin then external then internal then parent then sibling then index then object then type it would be groups: ["builtin", "external", "internal", "parent", "sibling", "index", "object", "type"] (de-nested)

I note that there's a TODO in the conditional and this is a described as a partial fix, so maybe the new behaviour is intended to be reverted eventually?

[ljharb]

@yndajas that sounds right to me. If the behavior has changed in v2.28.0 then that's a bug that needs fixing.

[yndajas]

@ljharb glad I'm not confused! Shall I open a new issue or leave it with you?

[ljharb]

@yndajas I think this issue probably covers it; a PR would be most helpful :-)

[thernstig]

@yndajas @ljharb I am not sure how the problem reported there solves this issue? This is about it not respecting builtin at all.

[ljharb]

@thernstig when it's nested like that, all of those groups can be intermingled. The config needs to de-nest the groups in order for their order to be enforced.

[yndajas]

@thernstig per @ljharb, I believe the original issue in this thread would be solved by denesting the groups:

// .eslintrc.json
{
  "root": true,
  "reportUnusedDisableDirectives": true,
  "parserOptions": {
    "ecmaVersion": 2023,
    "sourceType": "module"
  },
  "env": {
    "node": true,
    "es2022": true
  },
  "plugins": ["import"],
  "rules": {
    "import/order": [
      "error",
      {
        "groups": [
          "builtin",
          "external",
          "internal",
          "parent",
          "sibling",
          "index",
          "object",
          "type"
        ]
      }
    ]
  }
}

More detailed explanation in my earlier comment

Hope that clarifies!

[ljharb]

Per that comment, I'll close this, but can reopen if it doesn't work out.