order rule should support "node:" protocol for imports

[adp-psych]

The order rule should support the node: protocol for builtin node modules.

[adp-psych]

See also the eslint-plugin-unicorn rule prefer-node-protocol.

[ljharb]

Support it in what way? I released an update to is-core-module yesterday, so now node: URLs are resolved. The order rule sorts by type and specifier, so it should already work.

Separately, I think a rule like "prefer-node-protocol" is in fact insanely harmful, because package authors should never use it for the foreseeable future, due to backwards compatibility.

[adp-psych]

Sorry, I should have been clearer in my report. The specific problem I have with the order rule is that when I prepend node: to the modules, the order rule wants me to change the order of my imports from the previously correct order.

Here’s an example:

import {promises as fs} from 'node:fs';
import path from 'node:path';

import NeDB from 'nedb';
import {isNil} from 'ramda';

import makeDebug from './debug.js';

When I lint this, I get the following warning:

  4:1  warning  `nedb` import should occur before import of `node:fs`  import/order

The order rule wants me to import nedb before node:fs because, with the node: protocol, it alphabetically precedes it. However, I want the order rule to enforce that node modules are imported first, as in the documentation for the order rule.

Regarding backward compatibility, according to the documentation for node: imports, they are supported in v12.20.0, v14.13.1, and v16.0.0. LTS maintenance for v10.x ends 2021-04-30, after which every LTS version of node will support node: imports.

[ljharb]

Packages shouldn’t be dropping support for a node version just because it’s not LTS.

Thanks for the link tho; i didn’t realize it had been backported. I’ll have to update is-core-module to cover 12 and 14 for these kind of imports.

What version of node are you using eslint with? (note that if you’re using vscode, it bundles its own copy of node). Until i make the is-core-module change, only in node 16 will eslint properly recognize node: imports.

[adp-psych]

I’m using node v16.0.0 on Linux (Fedora 33). I’m not using VSCode.

[ljharb]

I double checked; require('node:fs') indeed fails everywhere but in node 16. Perhaps it's only in native ESM that they work?

[ljharb]

Can you confirm which version of is-core-module you have installed?

[nicolashenry]

From documentation node: imports also have been added in v14.13.1, v12.20.0 not only v16.0.0 :

[ljharb]

@nicolashenry yes, but that's the "imports" section of the docs. Try it yourself - require does not work with the node: prefix prior to node v16.

[nicolashenry]

I don't get your point, imports are working fine with 14.15.5:

import * as fs from 'node:fs';

console.log(fs !== undefined);

$ node --version
v14.15.5
$ node test.js
true

Who cares about require? Isn't this issue about import ordering?

[ljharb]

This plugin doesn't handle native ESM in node particularly; like virtually the entire JS ecosystem, it's built for transpiled ESM. Thus, require is in fact the primary thing that matters.

If you happen to be authoring native ESM, this plugin does not yet support that.

[adp-psych]

@ljharb @nicolashenry Node v16 supports the node: protocol for both import and require; node v14 and v12 only support the node: protocol for import, not for require, but there are plans to backport support.

@ljharb I don’t directly use is-core-module, but npm ls is-core-module shows that I have is-core-module@2.3.0 installed. Note that the package.json for eslint-plugin-import itself lists is-core-module@^1.0.2 as a dependency.

[ljharb]

@adp-psych aha, oops :-) #1926 isn't released yet, however, which means that resolve is actually what matters, and resolve is ^1.17.0, which should indeed transitively depend on is-core-module v2.

[adp-psych]

@ljharb eslint-plugin-import@2.22.1 depends on resolve@1.17.0, which does not depend on is-core-module.

resolve@1.18.0 depends on is-core-module@^2.0.0.

[ljharb]

@adp-psych yes, but it uses ^, which automatically includes the latest version of resolve v1. Thus, if you reinstall eslint-plugin-import you’ll get resolve v1 latest and is-core-module 2.

[adp-psych]

@ljharb I see what you’re saying; but after running rm -rf node_modules package-lock.json && npm install, I still get the import/order warnings I originally described.

[ljharb]

@adp-psych and just to reiterate, this is with node 16, with eslint, on the command line, and npm ls is-core-module prints out v2, and npm ls resolve prints out what?

[adp-psych]

@ljharb Yes, I think so:

$ cat test.js
/* eslint-disable no-unused-vars, jsdoc/require-file-overview, notice/notice --
 * Minimal example. */

import fs from 'node:fs';

import NeDB from 'nedb';
$ eslint test.js

/home/user/.projects/anthonys-psychology-phd/experiments/2/test.js
  6:1  warning  `nedb` import should occur before import of `node:fs`  import/order

✖ 1 problem (0 errors, 1 warning)
  0 errors and 1 warning potentially fixable with the `--fix` option.

$ node --version
v16.0.0
$ npm --version
7.11.1
$ eslint --version
v7.25.0
$ npm ls is-core-module
@adp-psych/experiment-2@0.1.0 /home/user/.projects/anthonys-psychology-phd/experiments/2
└─┬ kss@3.0.1
  └─┬ resolve@1.20.0
    └── is-core-module@2.3.0

$ npm ls resolve
@adp-psych/experiment-2@0.1.0 /home/user/.projects/anthonys-psychology-phd/experiments/2
├─┬ @adp-psych/babel-config@2.0.4
│ ├─┬ @babel/preset-env@7.13.15
│ │ └─┬ babel-plugin-polyfill-corejs2@0.2.0
│ │   └─┬ @babel/helper-define-polyfill-provider@0.2.0
│ │     └── resolve@1.20.0 deduped
│ ├─┬ babel-plugin-macros@3.0.1
│ │ └── resolve@1.20.0 deduped
│ └─┬ babel-plugin-module-resolver@4.1.0
│   └── resolve@1.20.0 deduped
├─┬ @adp-psych/eslint-config@24.1.4
│ ├─┬ eslint-import-resolver-babel-module@5.3.1
│ │ └── resolve@1.20.0 deduped
│ ├─┬ eslint-plugin-import@2.22.1
│ │ ├─┬ eslint-import-resolver-node@0.3.4
│ │ │ └── resolve@1.20.0 deduped
│ │ ├─┬ read-pkg-up@2.0.0
│ │ │ └─┬ read-pkg@2.0.0
│ │ │   └─┬ normalize-package-data@2.5.0
│ │ │     └── resolve@1.20.0 deduped
│ │ └── resolve@1.20.0 deduped
│ └─┬ eslint-plugin-node@11.1.0
│   └── resolve@1.20.0 deduped
├─┬ @adp-psych/npm-package-json-lint-config@7.1.2
│ └─┬ npm-package-json-lint@5.1.0
│   └─┬ meow@6.1.1
│     └─┬ normalize-package-data@2.5.0
│       └── resolve@1.20.0 deduped
├─┬ @adp-psych/stylelint-config@9.0.1
│ └─┬ stylelint-no-browser-hacks@1.2.1
│   └─┬ stylelint@9.10.1
│     └─┬ meow@5.0.0
│       └─┬ normalize-package-data@2.5.0
│         └── resolve@1.20.0 deduped
├─┬ @adp-psych/webpack-config@0.4.0
│ └─┬ image-webpack-loader@7.0.1
│   └─┬ imagemin-gifsicle@7.0.0
│     └─┬ gifsicle@5.2.0
│       └─┬ logalot@2.1.0
│         └─┬ squeak@1.3.0
│           └─┬ lpad-align@1.1.2
│             └─┬ meow@3.7.0
│               └─┬ normalize-package-data@2.5.0
│                 └── resolve@1.20.0 deduped
├─┬ ava@3.15.0
│ └─┬ read-pkg@5.2.0
│   └─┬ normalize-package-data@2.5.0
│     └── resolve@1.20.0 deduped
├─┬ check-licenses@1.0.0
│ └─┬ meow@8.1.2
│   └─┬ normalize-package-data@3.0.2
│     └── resolve@1.20.0 deduped
├─┬ kss@3.0.1
│ └── resolve@1.20.0
├─┬ npm-check@5.9.2
│ ├─┬ depcheck@0.8.3
│ │ └── resolve@1.20.0 deduped
│ └─┬ meow@3.7.0
│   └─┬ normalize-package-data@2.5.0
│     └── resolve@1.20.0 deduped
├─┬ npm-run-all@4.1.5
│ └─┬ read-pkg@3.0.0
│   └─┬ normalize-package-data@2.5.0
│     └── resolve@1.20.0 deduped
└─┬ webpack-cli@4.6.0
  └─┬ rechoir@0.7.0
    └── resolve@1.20.0 deduped

$ npm ls eslint-plugin-import
@adp-psych/experiment-2@0.1.0 /home/user/.projects/anthonys-psychology-phd/experiments/2
└─┬ @adp-psych/eslint-config@24.1.4
  └── eslint-plugin-import@2.22.1

$ grep version node_modules/eslint-plugin-import/node_modules/is-core-module/package.json
grep: node_modules/eslint-plugin-import/node_modules/is-core-module/package.json: No such file or directory
$ grep version node_modules/eslint-plugin-import/node_modules/resolve/package.json
grep: node_modules/eslint-plugin-import/node_modules/resolve/package.json: No such file or directory
$ grep version node_modules/is-core-module/package.json
    "version": "2.3.0",
        "version": "auto-changelog && git add CHANGELOG.md",
        "postversion": "auto-changelog && git add CHANGELOG.md && git commit --no-edit --amend && git tag -f \"v$(node -e \"console.log(require('./package.json').version)\")\""
$ grep version node_modules/resolve/package.json
    "version": "1.20.0",
$ 

[ljharb]

Thanks, assuming that changing node:fs to fs gives you no warnings, this does seem like a bug.

[adp-psych]

@ljharb I’ve looked into this a bit and it seems that you can mostly fix the problem in master by updating is-core-module:

diff --git a/package.json b/package.json
index 8926092..0186270 100644
--- a/package.json
+++ b/package.json
@@ -106,7 +106,7 @@
     "eslint-module-utils": "^2.6.0",
     "find-up": "^2.0.0",
     "has": "^1.0.3",
-    "is-core-module": "^1.0.2",
+    "is-core-module": "^2.3.0",
     "minimatch": "^3.0.4",
     "object.values": "^1.1.1",
     "pkg-up": "^1.0.0",

I also wrote some tests for node: imports:

diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js
index 42f99c2..dcb89f6 100644
--- a/tests/src/rules/order.js
+++ b/tests/src/rules/order.js
@@ -706,6 +706,42 @@ ruleTester.run('order', rule, {
         },
       ],
     }),
+    // Handle 'node:' imports correctly (#2035)
+    test({
+      code: `
+        import { mkdir } from 'node:fs';
+        import path from 'node:path';
+
+        import foxtrot from 'foxtrot';
+        import { golf } from 'golf';
+
+        import alfa from './alfa';
+        import { bravo } from './bravo';
+      `,
+      options: [{
+        'newlines-between': 'always-and-inside-groups',
+        groups: ['builtin', 'external', 'parent', 'sibling', 'index'],
+        alphabetize: { order: 'asc' },
+      }],
+    }),
+    // Handle 'node:' requires correctly (#2035)
+    test({
+      code: `
+        var { mkdir } = require('node:fs');
+        var path = require('node:path');
+
+        var foxtrot = require('foxtrot');
+        var { golf } = require('golf');
+
+        var alfa = require('./alfa');
+        var { bravo } = require('./bravo');
+      `,
+      options: [{
+        'newlines-between': 'always-and-inside-groups',
+        groups: ['builtin', 'external', 'parent', 'sibling', 'index'],
+        alphabetize: { order: 'asc' },
+      }],
+    }),
     ...flatMap(getTSParsers, parser => [
       // Order of the `import ... = require(...)` syntax
       test({
@@ -2161,6 +2197,104 @@ ruleTester.run('order', rule, {
         }],
       }),
     ],
+    // Handle 'node:' imports correctly (#2035)
+    test({
+      code: `
+        import alfa from './alfa';
+        import { bravo } from './bravo';
+        import whiskey from './whiskey';
+        import foxtrot from 'foxtrot';
+        import { golf } from 'golf';
+        import victor from 'victor';
+        import { mkdir } from 'node:fs';
+        import path from 'node:path';
+        import util from 'node:util';
+      `,
+      options: [{
+        'newlines-between': 'always-and-inside-groups',
+        groups: ['builtin', 'external', 'parent', 'sibling', 'index'],
+        alphabetize: { order: 'asc' },
+      }],
+      errors: [{
+        message: 'There should be at least one empty line between import groups',
+      }, {
+        message: '`foxtrot` import should occur before import of `./alfa`',
+      }, {
+        message: '`golf` import should occur before import of `./alfa`',
+      }, {
+        message: 'There should be at least one empty line between import groups',
+      }, {
+        message: '`victor` import should occur before import of `./alfa`',
+      }, {
+        message: '`node:fs` import should occur before import of `./alfa`',
+      }, {
+        message: '`node:path` import should occur before import of `./alfa`',
+      }, {
+        message: '`node:util` import should occur before import of `./alfa`',
+      }],
+      output: `
+        import { mkdir } from 'node:fs';
+        import path from 'node:path';
+        import util from 'node:util';
+
+        import foxtrot from 'foxtrot';
+        import { golf } from 'golf';
+        import victor from 'victor';
+
+        import alfa from './alfa';
+        import { bravo } from './bravo';
+        import whiskey from './whiskey';
+      `,
+    }),
+    // Handle 'node:' requires correctly (#2035)
+    test({
+      code: `
+        var alfa = require('./alfa');
+        var { bravo } = require('./bravo');
+        var whiskey = require('./whiskey');
+        var foxtrot = require('foxtrot');
+        var { golf } = require('golf');
+        var victor = require('victor');
+        var { mkdir } = require('node:fs');
+        var path = require('node:path');
+        var util = require('node:util');
+      `,
+      options: [{
+        'newlines-between': 'always-and-inside-groups',
+        groups: ['builtin', 'external', 'parent', 'sibling', 'index'],
+        alphabetize: { order: 'asc' },
+      }],
+      errors: [{
+        message: 'There should be at least one empty line between import groups',
+      }, {
+        message: '`foxtrot` import should occur before import of `./alfa`',
+      }, {
+        message: '`golf` import should occur before import of `./alfa`',
+      }, {
+        message: 'There should be at least one empty line between import groups',
+      }, {
+        message: '`victor` import should occur before import of `./alfa`',
+      }, {
+        message: '`node:fs` import should occur before import of `./alfa`',
+      }, {
+        message: '`node:path` import should occur before import of `./alfa`',
+      }, {
+        message: '`node:util` import should occur before import of `./alfa`',
+      }],
+      output: `
+        var { mkdir } = require('node:fs');
+        var path = require('node:path');
+        var util = require('node:util');
+
+        var foxtrot = require('foxtrot');
+        var { golf } = require('golf');
+        var victor = require('victor');
+
+        var alfa = require('./alfa');
+        var { bravo } = require('./bravo');
+        var whiskey = require('./whiskey');
+      `,
+    }),
   ].filter((t) => !!t),
 });

The only remaining problem is that fixing doesn’t work correctly:

  130 passing (5s)
  2 failing

  1) order invalid
        import alfa from './alfa';
        import { bravo } from './bravo';
        import whiskey from './whiskey';
        import foxtrot from 'foxtrot';
        import { golf } from 'golf';
        import victor from 'victor';
        import { mkdir } from 'node:fs';
        import path from 'node:path';
        import util from 'node:util';
      :

      AssertionError [ERR_ASSERTION] [ERR_ASSERTION]: Output is incorrect.
      + expected - actual

      +        import { mkdir } from 'node:fs';
      +        import path from 'node:path';
      +        import util from 'node:util';
      +
               import foxtrot from 'foxtrot';
      +        import { golf } from 'golf';
      +        import victor from 'victor';
      +
               import alfa from './alfa';
               import { bravo } from './bravo';
               import whiskey from './whiskey';
      -        import { golf } from 'golf';
      -        import victor from 'victor';
      -
      -        import { mkdir } from 'node:fs';
      -        import path from 'node:path';
      -        import util from 'node:util';

      at testInvalidTemplate (node_modules/eslint/lib/rule-tester/rule-tester.js:854:28)
      at Context.<anonymous> (node_modules/eslint/lib/rule-tester/rule-tester.js:893:25)
      at processImmediate (node:internal/timers:464:21)

  2) order invalid
        var alfa = require('./alfa');
        var { bravo } = require('./bravo');
        var whiskey = require('./whiskey');
        var foxtrot = require('foxtrot');
        var { golf } = require('golf');
        var victor = require('victor');
        var { mkdir } = require('node:fs');
        var path = require('node:path');
        var util = require('node:util');
      :

      AssertionError [ERR_ASSERTION] [ERR_ASSERTION]: Output is incorrect.
      + expected - actual

      +        var { mkdir } = require('node:fs');
      +        var path = require('node:path');
      +        var util = require('node:util');
      +
               var foxtrot = require('foxtrot');
      +        var { golf } = require('golf');
      +        var victor = require('victor');
      +
               var alfa = require('./alfa');
               var { bravo } = require('./bravo');
               var whiskey = require('./whiskey');
      -        var { golf } = require('golf');
      -        var victor = require('victor');
      -
      -        var { mkdir } = require('node:fs');
      -        var path = require('node:path');
      -        var util = require('node:util');

      at testInvalidTemplate (node_modules/eslint/lib/rule-tester/rule-tester.js:854:28)
      at Context.<anonymous> (node_modules/eslint/lib/rule-tester/rule-tester.js:893:25)
      at processImmediate (node:internal/timers:464:21)

I don’t really understand the code, but it seems that it calculates the rank values correctly; the issue is just with the actual fixing.

[iamnapo]

Hi, I don’t know if it helps, but in my case, with a similar setup (node v16, is-core-module v2.3.0, resolve v1.20.0), import order is fine, but if I have a newline between the two import statements, I get: There should be no empty line within import group import/order. If I replace node:fs with fs, the error goes away. The rule’s config is: "import/order": ["error", { "newlines-between": "always" }]

[muuvmuuv]

Same here with uvicorn wanting me to use "node:x" syntax for built-in modules. As for me it is just for scripts and tools so not the project itself. Would be nice to have module support for it.

As a workaround I disabled it in eslint with: "unicorn/prefer-node-protocol": "off"

[ljharb]

There's not a good reason to prefer the node: protocol - there's really only downsides - so I'd recommend disabling that rule regardless.

[nicolashenry]

What downsides ? If you talk about node compatibility, it is only important for library developers, not for application developers.

For advantages, there are some explained in this proposal https://github.com/nodejs/node/issues/38343 which is about to use it by default in node documentation examples.

So if you disagree, I suggest you to give your counter arguments to the Node.js team before it is done (for now there are almost only positive reactions).

[iamnapo]

There's not a good reason to prefer the node: protocol - there's really only downsides - so I'd recommend disabling that rule regardless.

I mostly agree, but as @nicolashenry said, it shouldn’t be a problem for applications. Anyhow, updating is-core-module seems to fix the issue, so I guess it should be fine on the next release.

[adp-psych]

@iamnapo @ljharb Note that fixing still doesn’t work correctly, as I detailed above.

[iamnapo]

@adp-psych I ran the tests without node: and they still fail 🤷🏼‍♂️

[iamnapo]

Hi, I don’t know if it helps, but in my case, with a similar setup (node v16, is-core-module v2.3.0, resolve v1.20.0), import order is fine, but if I have a newline between the two import statements, I get: There should be no empty line within import group import/order. If I replace node:fs with fs, the error goes away. The rule’s config is: "import/order": ["error", { "newlines-between": "always" }]

@ljharb fyi: This is indeed fixed in v2.23.0

[ljharb]

Please file a new issue if it's not working!