Skip to content

Commit

Permalink
Add allow option to no-nodejs-modules rule (fixes #452) (#509)
Browse files Browse the repository at this point in the history
* Add `allow` option to `no-nodejs-modules` rule (fixes #452)

* Add test case

* Fix not working in Node v4
  • Loading branch information
jfmengels authored and benmosher committed Aug 23, 2016
1 parent 7995581 commit 387d333
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 5 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com).

## [Unreleased]

- Added an `allow` option to [`no-nodejs-modules`] to allow exceptions ([#452], [#509]).

## [1.14.0] - 2016-08-22
### Added
Expand Down Expand Up @@ -290,6 +290,7 @@ for info on changes for earlier releases.
[`prefer-default-export`]: ./docs/rules/prefer-default-export.md
[`no-restricted-paths`]: ./docs/rules/no-restricted-paths.md

[#509]: https://github.com/benmosher/eslint-plugin-import/pull/509
[#503]: https://github.com/benmosher/eslint-plugin-import/pull/503
[#499]: https://github.com/benmosher/eslint-plugin-import/pull/499
[#461]: https://github.com/benmosher/eslint-plugin-import/pull/461
Expand Down Expand Up @@ -331,6 +332,7 @@ for info on changes for earlier releases.
[#478]: https://github.com/benmosher/eslint-plugin-import/issues/478
[#456]: https://github.com/benmosher/eslint-plugin-import/issues/456
[#453]: https://github.com/benmosher/eslint-plugin-import/issues/453
[#452]: https://github.com/benmosher/eslint-plugin-import/issues/452
[#441]: https://github.com/benmosher/eslint-plugin-import/issues/441
[#423]: https://github.com/benmosher/eslint-plugin-import/issues/423
[#416]: https://github.com/benmosher/eslint-plugin-import/issues/416
Expand Down
9 changes: 9 additions & 0 deletions docs/rules/no-nodejs-modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

Forbid the use of Node.js builtin modules. Can be useful for client-side web projects that do not have access to those modules.

### Options

This rule supports the following options:

- `allow`: Array of names of allowed modules. Defaults to an empty array.

## Rule Details

### Fail
Expand All @@ -24,6 +30,9 @@ import foo from './foo';
var _ = require('lodash');
var foo = require('foo');
var foo = require('./foo');

/* eslint import/no-nodejs-modules: ["error", {"allow": ["path"]}] */
import path from 'path';
```

## When Not To Use It
Expand Down
11 changes: 7 additions & 4 deletions src/rules/no-nodejs-modules.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
import importType from '../core/importType'
import isStaticRequire from '../core/staticRequire'

function reportIfMissing(context, node, name) {
if (importType(name, context) === 'builtin') {
function reportIfMissing(context, node, allowed, name) {
if (allowed.indexOf(name) === -1 && importType(name, context) === 'builtin') {
context.report(node, 'Do not import Node.js builtin module "' + name + '"')
}
}

module.exports = function (context) {
const options = context.options[0] || {}
const allowed = options.allow || []

return {
ImportDeclaration: function handleImports(node) {
reportIfMissing(context, node, node.source.value)
reportIfMissing(context, node, allowed, node.source.value)
},
CallExpression: function handleRequires(node) {
if (isStaticRequire(node)) {
reportIfMissing(context, node, node.arguments[0].value)
reportIfMissing(context, node, allowed, node.arguments[0].value)
}
},
}
Expand Down
37 changes: 37 additions & 0 deletions tests/src/rules/no-nodejs-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,36 @@ ruleTester.run('no-nodejs-modules', rule, {
test({ code: 'var foo = require("foo")'}),
test({ code: 'var foo = require("./")'}),
test({ code: 'var foo = require("@scope/foo")'}),
test({
code: 'import events from "events"',
options: [{
allow: ['events'],
}],
}),
test({
code: 'import path from "path"',
options: [{
allow: ['path'],
}],
}),
test({
code: 'var events = require("events")',
options: [{
allow: ['events'],
}],
}),
test({
code: 'var path = require("path")',
options: [{
allow: ['path'],
}],
}),
test({
code: 'import path from "path";import events from "events"',
options: [{
allow: ['path', 'events'],
}],
}),
],
invalid: [
test({
Expand All @@ -44,5 +74,12 @@ ruleTester.run('no-nodejs-modules', rule, {
code: 'var fs = require("fs")',
errors: [error('Do not import Node.js builtin module "fs"')],
}),
test({
code: 'import fs from "fs"',
options: [{
allow: ['path'],
}],
errors: [error('Do not import Node.js builtin module "fs"')],
}),
],
})

0 comments on commit 387d333

Please sign in to comment.