Skip to content

Commit

Permalink
Merge pull request #880 from futpib/no-commonjs-allow-require
Browse files Browse the repository at this point in the history
Add `allow-require` option to `no-commonjs` rule
  • Loading branch information
ljharb authored Apr 1, 2018
2 parents 48d0a8a + c438d3e commit ee15fa4
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 6 deletions.
25 changes: 22 additions & 3 deletions docs/rules/no-commonjs.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,29 @@ module.exports = { a: "b" }
exports.c = "d"
```

If `allow-primitive-modules` is provided as an option, the following is valid:
### Allow require

If `allowRequire` option is set to `true`, `require` calls are valid:

```js
/*eslint no-commonjs: [2, { allowRequire: true }]*/

if (typeof window !== "undefined") {
require('that-ugly-thing');
}
```

but `module.exports` is reported as usual.

This is useful for conditional requires.
If you don't rely on synchronous module loading, check out [dynamic import](https://github.com/airbnb/babel-plugin-dynamic-import-node).

### Allow primitive modules

If `allowPrimitiveModules` option is set to `true`, the following is valid:

```js
/*eslint no-commonjs: [2, "allow-primitive-modules"]*/
/*eslint no-commonjs: [2, { allowPrimitiveModules: true }]*/

module.exports = "foo"
module.exports = function rule(context) { return { /* ... */ } }
Expand All @@ -33,7 +52,7 @@ module.exports = function rule(context) { return { /* ... */ } }
but this is still reported:

```js
/*eslint no-commonjs: [2, "allow-primitive-modules"]*/
/*eslint no-commonjs: [2, { allowPrimitiveModules: true }]*/

module.exports = { x: "y" }
exports.z = function boop() { /* ... */ }
Expand Down
44 changes: 41 additions & 3 deletions src/rules/no-commonjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,33 +8,69 @@ import docsUrl from '../docsUrl'
const EXPORT_MESSAGE = 'Expected "export" or "export default"'
, IMPORT_MESSAGE = 'Expected "import" instead of "require()"'

function allowPrimitive(node, context) {
if (context.options.indexOf('allow-primitive-modules') < 0) return false
function normalizeLegacyOptions(options) {
if (options.indexOf('allow-primitive-modules') >= 0) {
return { allowPrimitiveModules: true }
}
return options[0] || {}
}

function allowPrimitive(node, options) {
if (!options.allowPrimitiveModules) return false
if (node.parent.type !== 'AssignmentExpression') return false
return (node.parent.right.type !== 'ObjectExpression')
}

function allowRequire(node, options) {
return options.allowRequire
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

const schemaString = { enum: ['allow-primitive-modules'] }
const schemaObject = {
type: 'object',
properties: {
allowPrimitiveModules: { 'type': 'boolean' },
allowRequire: { 'type': 'boolean' },
},
additionalProperties: false,
}

module.exports = {
meta: {
docs: {
url: docsUrl('no-commonjs'),
},

schema: {
anyOf: [
{
type: 'array',
items: [schemaString],
additionalItems: false,
},
{
type: 'array',
items: [schemaObject],
additionalItems: false,
},
],
},
},

create: function (context) {
const options = normalizeLegacyOptions(context.options)

return {

'MemberExpression': function (node) {

// module.exports
if (node.object.name === 'module' && node.property.name === 'exports') {
if (allowPrimitive(node, context)) return
if (allowPrimitive(node, options)) return
context.report({ node, message: EXPORT_MESSAGE })
}

Expand Down Expand Up @@ -65,6 +101,8 @@ module.exports = {
if (module.type !== 'Literal') return
if (typeof module.value !== 'string') return

if (allowRequire(call, options)) return

// keeping it simple: all 1-string-arg `require` calls are reported
context.report({
node: call.callee,
Expand Down
3 changes: 3 additions & 0 deletions tests/src/rules/no-commonjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,12 @@ ruleTester.run('no-commonjs', require('rules/no-commonjs'), {
{ code: "var bar = proxyquire('./bar');" },
{ code: "var bar = require('./ba' + 'r');" },
{ code: 'var zero = require(0);' },
{ code: 'require("x")', options: [{ allowRequire: true }] },

{ code: 'module.exports = function () {}', options: ['allow-primitive-modules'] },
{ code: 'module.exports = function () {}', options: [{ allowPrimitiveModules: true }] },
{ code: 'module.exports = "foo"', options: ['allow-primitive-modules'] },
{ code: 'module.exports = "foo"', options: [{ allowPrimitiveModules: true }] },
],

invalid: [
Expand Down

0 comments on commit ee15fa4

Please sign in to comment.