Merge pull request #880 from futpib/no-commonjs-allow-require

Add `allow-require` option to `no-commonjs` rule

docs/rules/no-commonjs.md

@@ -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 { /* ... */ } }
@@ -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() { /* ... */ }

src/rules/no-commonjs.js

@@ -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 })
         }
 
@@ -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,

tests/src/rules/no-commonjs.js

@@ -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: [