Skip to content

Conversation

@baevm
Copy link
Contributor

@baevm baevm commented Jan 14, 2026

this PR adds eslint-plugin-import/no-nodejs-modules rule, passes all tests + some new, like TS import equals or export declarations

issue #1117, also #15208

Copilot AI review requested due to automatic review settings January 14, 2026 19:01
@baevm baevm requested a review from camc314 as a code owner January 14, 2026 19:01
@github-actions github-actions bot added A-linter Area - Linter C-enhancement Category - New feature or request labels Jan 14, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements the eslint-plugin-import/no-nodejs-modules rule to forbid the use of Node.js builtin modules in client-side projects. The rule detects imports of Node.js builtin modules through various syntax patterns including ES6 imports, CommonJS require calls, dynamic imports, TypeScript import equals declarations, and export declarations.

Changes:

  • Adds a new lint rule no-nodejs-modules with configurable allow list for exceptions
  • Supports detection across multiple import patterns: import, require(), dynamic import(), TS import = require(), and export declarations
  • Handles both bare module names ("fs") and Node.js protocol syntax ("node:fs")

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
crates/oxc_linter/src/rules/import/no_nodejs_modules.rs Core implementation of the rule with comprehensive test coverage
crates/oxc_linter/src/rules.rs Registers the new rule in the linter
crates/oxc_linter/src/generated/rule_runner_impls.rs Auto-generated rule runner implementation specifying AST node types to check
crates/oxc_linter/src/snapshots/import_no_nodejs_modules.snap Test snapshots showing expected diagnostic output

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 91 to 94
AstKind::ImportExpression(import) => match &import.source {
Expression::StringLiteral(str_lit) => Some(str_lit.value),
_ => None,
},
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ImportExpression handler only checks for StringLiteral sources. Template literals without substitutions (e.g., import(\fs`)) should also be checked for Node.js modules, similar to how no_dynamic_requirehandles them. Consider adding support forExpression::TemplateLiteralwhenis_no_substitution_template()` returns true.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 17098a4

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 14, 2026

CodSpeed Performance Report

Merging this PR will not alter performance

Comparing baevm:import/no_nodejs_modules (ca10a37) with main (78fa1ba)

Summary

✅ 4 untouched benchmarks
⏩ 41 skipped benchmarks1

Footnotes

  1. 41 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant