From ec03d0c631ed9026d32fd96ccdb554b3a66c3a10 Mon Sep 17 00:00:00 2001
From: futpib <futpib@gmail.com>
Date: Fri, 23 Jun 2017 18:26:34 +0300
Subject: [PATCH 1/4] Add `allow-require` option to `no-commonjs` rule

This fixes #548
---
 docs/rules/no-commonjs.md      | 18 ++++++++++++++++++
 src/rules/no-commonjs.js       |  7 +++++++
 tests/src/rules/no-commonjs.js |  1 +
 3 files changed, 26 insertions(+)

diff --git a/docs/rules/no-commonjs.md b/docs/rules/no-commonjs.md
index 4e823502b..8de04d837 100644
--- a/docs/rules/no-commonjs.md
+++ b/docs/rules/no-commonjs.md
@@ -21,6 +21,24 @@ module.exports = { a: "b" }
 exports.c = "d"
 ```
 
+### Allow require
+
+If `allow-require` is provided as an option, `require` calls are valid:
+
+```js
+/*eslint no-commonjs: [2, "allow-require"]*/
+
+if (typeof window !== "undefined") {
+  require('that-ugly-thing');
+}
+```
+
+but `module.exports` is reported as usual.
+
+This is useful for conditional requires.
+
+### Allow primitive modules
+
 If `allow-primitive-modules` is provided as an option, the following is valid:
 
 ```js
diff --git a/src/rules/no-commonjs.js b/src/rules/no-commonjs.js
index dfa9b0a5c..d891f4965 100644
--- a/src/rules/no-commonjs.js
+++ b/src/rules/no-commonjs.js
@@ -14,6 +14,11 @@ function allowPrimitive(node, context) {
   return (node.parent.right.type !== 'ObjectExpression')
 }
 
+function allowRequire(node, context) {
+  if (context.options.indexOf('allow-require') < 0) return false
+  return true
+}
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -65,6 +70,8 @@ module.exports = {
         if (module.type !== 'Literal') return
         if (typeof module.value !== 'string') return
 
+        if (allowRequire(call, context)) return
+
         // keeping it simple: all 1-string-arg `require` calls are reported
         context.report({
           node: call.callee,
diff --git a/tests/src/rules/no-commonjs.js b/tests/src/rules/no-commonjs.js
index 649d0022f..2f1d69634 100644
--- a/tests/src/rules/no-commonjs.js
+++ b/tests/src/rules/no-commonjs.js
@@ -37,6 +37,7 @@ 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: ['allow-require'] },
 
     { code: 'module.exports = function () {}', options: ['allow-primitive-modules'] },
     { code: 'module.exports = "foo"', options: ['allow-primitive-modules'] },

From d038a0c48aa5102b63a434171afbd9e28b5436a7 Mon Sep 17 00:00:00 2001
From: futpib <futpib@gmail.com>
Date: Fri, 23 Jun 2017 22:02:11 +0300
Subject: [PATCH 2/4] Switch `no-commonjs` to object options

---
 docs/rules/no-commonjs.md      | 10 +++++-----
 src/rules/no-commonjs.js       | 21 ++++++++++++++-------
 tests/src/rules/no-commonjs.js |  4 +++-
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/docs/rules/no-commonjs.md b/docs/rules/no-commonjs.md
index 8de04d837..5e771a256 100644
--- a/docs/rules/no-commonjs.md
+++ b/docs/rules/no-commonjs.md
@@ -23,10 +23,10 @@ exports.c = "d"
 
 ### Allow require
 
-If `allow-require` is provided as an option, `require` calls are valid:
+If `allowRequire` option is set to `true`, `require` calls are valid:
 
 ```js
-/*eslint no-commonjs: [2, "allow-require"]*/
+/*eslint no-commonjs: [2, { allowRequire: true }]*/
 
 if (typeof window !== "undefined") {
   require('that-ugly-thing');
@@ -39,10 +39,10 @@ This is useful for conditional requires.
 
 ### Allow primitive modules
 
-If `allow-primitive-modules` is provided as an option, the following is valid:
+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 { /* ... */ } }
@@ -51,7 +51,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() { /* ... */ }
diff --git a/src/rules/no-commonjs.js b/src/rules/no-commonjs.js
index d891f4965..5ba73e80f 100644
--- a/src/rules/no-commonjs.js
+++ b/src/rules/no-commonjs.js
@@ -8,15 +8,21 @@ 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, context) {
-  if (context.options.indexOf('allow-require') < 0) return false
-  return true
+function allowRequire(node, options) {
+  return options.allowRequire
 }
 
 //------------------------------------------------------------------------------
@@ -32,6 +38,7 @@ module.exports = {
   },
 
   create: function (context) {
+    const options = normalizeLegacyOptions(context.options)
 
     return {
 
@@ -39,7 +46,7 @@ module.exports = {
 
         // 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 })
         }
 
@@ -70,7 +77,7 @@ module.exports = {
         if (module.type !== 'Literal') return
         if (typeof module.value !== 'string') return
 
-        if (allowRequire(call, context)) return
+        if (allowRequire(call, options)) return
 
         // keeping it simple: all 1-string-arg `require` calls are reported
         context.report({
diff --git a/tests/src/rules/no-commonjs.js b/tests/src/rules/no-commonjs.js
index 2f1d69634..b2985203d 100644
--- a/tests/src/rules/no-commonjs.js
+++ b/tests/src/rules/no-commonjs.js
@@ -37,10 +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: ['allow-require'] },
+    { 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: [

From 1c9f61c8a06379ec6d5e7adf722a0f590c126164 Mon Sep 17 00:00:00 2001
From: futpib <futpib@gmail.com>
Date: Fri, 23 Jun 2017 22:02:37 +0300
Subject: [PATCH 3/4] Add note on dynamic `require` vs dynamic `import`

---
 docs/rules/no-commonjs.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/rules/no-commonjs.md b/docs/rules/no-commonjs.md
index 5e771a256..4353886bf 100644
--- a/docs/rules/no-commonjs.md
+++ b/docs/rules/no-commonjs.md
@@ -36,6 +36,7 @@ if (typeof window !== "undefined") {
 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
 

From c438d3e6765c6331eca2c1cf2df75e085cc666ac Mon Sep 17 00:00:00 2001
From: futpib <futpib@gmail.com>
Date: Fri, 23 Jun 2017 23:37:15 +0300
Subject: [PATCH 4/4] Add options schema to `no-commonjs` rule

---
 src/rules/no-commonjs.js | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/src/rules/no-commonjs.js b/src/rules/no-commonjs.js
index 5ba73e80f..22939aa7b 100644
--- a/src/rules/no-commonjs.js
+++ b/src/rules/no-commonjs.js
@@ -29,12 +29,36 @@ function allowRequire(node, options) {
 // 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) {