-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: added prefer-node-builtin-imports rule #3024
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
# import/prefer-node-builtin-imports | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m not interested in a rule that solely pushes using the protocol - if anything, it’d be a rule that can be configured to require, or forbid, the protocol. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ljharb What config option should be from user's perspective? Just a boolean to toggle this? If yes then I don't see how is it different from simply have it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "off" is the current state - where you can have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ljharb Ok, got it. |
||
|
||
🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). | ||
|
||
<!-- end auto-generated rule header --> | ||
|
||
Reports when there is no `node:` protocol for builtin modules. | ||
|
||
```ts | ||
import path from "node:path"; | ||
``` | ||
|
||
## Rule Details | ||
|
||
This rule enforces that builtins node imports are using `node:` protocol. It resolved the conflict of a module (npm-installed) in `node_modules` overriding the built-in module. Besides that, it is also clear that a built-in Node.js module is imported. | ||
|
||
## Examples | ||
|
||
❌ Invalid | ||
|
||
```ts | ||
import fs from "fs"; | ||
export { promises } from "fs"; | ||
// require | ||
const fs = require("fs/promises"); | ||
``` | ||
|
||
✅ Valid | ||
|
||
```ts | ||
import fs from "node:fs"; | ||
export { promises } from "node:fs"; | ||
// require | ||
const fs = require("node:fs/promises"); | ||
``` | ||
|
||
## When Not To Use It | ||
|
||
If you are using browser or Bun or Deno since this rule doesn't do anything with them. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
'use strict'; | ||
|
||
const { builtinModules } = require('module'); | ||
const { default: docsUrl } = require('../docsUrl'); | ||
|
||
const MESSAGE_ID = 'prefer-node-builtin-imports'; | ||
const messages = { | ||
[MESSAGE_ID]: 'Prefer `node:{{moduleName}}` over `{{moduleName}}`.', | ||
}; | ||
|
||
function replaceStringLiteral( | ||
fixer, | ||
node, | ||
text, | ||
relativeRangeStart, | ||
relativeRangeEnd, | ||
) { | ||
const firstCharacterIndex = node.range[0] + 1; | ||
const start = Number.isInteger(relativeRangeEnd) | ||
? relativeRangeStart + firstCharacterIndex | ||
: firstCharacterIndex; | ||
const end = Number.isInteger(relativeRangeEnd) | ||
? relativeRangeEnd + firstCharacterIndex | ||
: node.range[1] - 1; | ||
|
||
return fixer.replaceTextRange([start, end], text); | ||
} | ||
|
||
const isStringLiteral = (node) => node.type === 'Literal' && typeof node.value === 'string'; | ||
|
||
const isStaticRequireWith1Param = (node) => !node.optional | ||
&& node.callee.type === 'Identifier' | ||
&& node.callee.name === 'require' | ||
&& node.arguments[0] | ||
// check for only 1 argument | ||
&& !node.arguments[1]; | ||
|
||
function checkAndReport(src, ctx) { | ||
const { value } = src; | ||
|
||
if (!builtinModules.includes(value)) { return; } | ||
|
||
if (value.startsWith('node:')) { return; } | ||
|
||
ctx.report({ | ||
node: src, | ||
messageId: MESSAGE_ID, | ||
data: { moduleName: value }, | ||
/** @param {import('eslint').Rule.RuleFixer} fixer */ | ||
fix(fixer) { | ||
return replaceStringLiteral(fixer, src, 'node:', 0, 0); | ||
}, | ||
}); | ||
} | ||
|
||
/** @type {import('eslint').Rule.RuleModule} */ | ||
module.exports = { | ||
meta: { | ||
type: 'suggestion', | ||
docs: { | ||
description: | ||
'Prefer using the `node:` protocol when importing Node.js builtin modules.', | ||
recommended: true, | ||
category: 'Best Practices', | ||
url: docsUrl('prefer-node-builin-imports'), | ||
}, | ||
fixable: 'code', | ||
schema: [], | ||
messages, | ||
}, | ||
create(ctx) { | ||
return { | ||
CallExpression(node) { | ||
if (!isStaticRequireWith1Param(node)) { | ||
return; | ||
} | ||
|
||
if (!isStringLiteral(node.arguments[0])) { | ||
return; | ||
} | ||
|
||
return checkAndReport(node.arguments[0], ctx); | ||
}, | ||
ExportNamedDeclaration(node) { | ||
if (!isStringLiteral) { return; } | ||
|
||
return checkAndReport(node.source, ctx); | ||
}, | ||
ImportDeclaration(node) { | ||
if (!isStringLiteral) { return; } | ||
|
||
return checkAndReport(node.source, ctx); | ||
}, | ||
ImportExpression(node) { | ||
if (!isStringLiteral) { return; } | ||
|
||
return checkAndReport(node.source, ctx); | ||
}, | ||
}; | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
import { test } from '../utils'; | ||
|
||
import { RuleTester } from 'eslint'; | ||
|
||
const ruleTester = new RuleTester(); | ||
const rule = require('rules/prefer-node-builtin-imports'); | ||
|
||
ruleTester.run('prefer-node-builtin-imports', rule, { | ||
valid: [ | ||
test({ code: 'import unicorn from "unicorn";' }), | ||
test({ code: 'import fs from "./fs";' }), | ||
test({ code: 'import fs from "unknown-builtin-module";' }), | ||
test({ code: 'import fs from "node:fs";' }), | ||
test({ | ||
code: ` | ||
async function foo() { | ||
const fs = await import(fs); | ||
}`, | ||
}), | ||
test({ | ||
code: ` | ||
async function foo() { | ||
const fs = await import(0); | ||
}`, | ||
}), | ||
test({ | ||
code: ` | ||
async function foo() { | ||
const fs = await import(\`fs\`); | ||
}`, | ||
}), | ||
test({ code: 'import "punycode/";' }), | ||
test({ code: 'const fs = require("node:fs");' }), | ||
test({ code: 'const fs = require("node:fs/promises");' }), | ||
test({ code: 'const fs = require(fs);' }), | ||
test({ code: 'const fs = notRequire("fs");' }), | ||
test({ code: 'const fs = foo.require("fs");' }), | ||
test({ code: 'const fs = require.resolve("fs");' }), | ||
test({ code: 'const fs = require(`fs`);' }), | ||
test({ code: 'const fs = require?.("fs");' }), | ||
test({ code: 'const fs = require("fs", extra);' }), | ||
test({ code: 'const fs = require();' }), | ||
test({ code: 'const fs = require(...["fs"]);' }), | ||
test({ code: 'const fs = require("unicorn");' }), | ||
], | ||
invalid: [ | ||
test({ code: 'import fs from "fs";' }), | ||
test({ code: 'export {promises} from "fs";' }), | ||
test({ | ||
code: ` | ||
async function foo() { | ||
const fs = await import('fs'); | ||
}`, | ||
}), | ||
test({ code: 'import fs from "fs/promises";' }), | ||
test({ code: 'export {default} from "fs/promises";' }), | ||
test({ | ||
code: ` | ||
async function foo() { | ||
const fs = await import('fs/promises'); | ||
}`, | ||
}), | ||
test({ code: 'import {promises} from "fs";' }), | ||
test({ code: 'export {default as promises} from "fs";' }), | ||
test({ | ||
code: ` | ||
async function foo() { | ||
const fs = await import("fs/promises"); | ||
}`, | ||
}), | ||
test({ | ||
code: ` | ||
async function foo() { | ||
const fs = await import(/* escaped */"\\u{66}s/promises"); | ||
`, | ||
}), | ||
test({ code: 'import "buffer";' }), | ||
test({ code: 'import "child_process";' }), | ||
test({ code: 'import "timers/promises";' }), | ||
test({ code: 'const {promises} = require("fs")' }), | ||
test({ code: 'const fs = require("fs/promises")' }), | ||
], | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding things to recommended is a breaking change