Merge 8b1c71c into 7ffbf03

import-js · Aug 19, 2019 · 3168e48 · 3168e48
2 parents 7ffbf03 + 8b1c71c
commit 3168e48
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 8 deletions.
diff --git a/docs/rules/no-commonjs.md b/docs/rules/no-commonjs.md
@@ -27,15 +27,30 @@ If `allowRequire` option is set to `true`, `require` calls are valid:
 
 ```js
 /*eslint no-commonjs: [2, { allowRequire: true }]*/
+var mod = require('./mod');
+```
+
+but `module.exports` is reported as usual.
+
+### Allow conditional require
+
+By default, conditional requires are allowed:
+
+```js
+var a = b && require("c")
 
 if (typeof window !== "undefined") {
   require('that-ugly-thing');
 }
+
+var fs = null;
+try {
+  fs = require("fs")
+} catch (error) {}
 ```
 
-but `module.exports` is reported as usual.
+If the `allowConditionalRequire` option is set to `false`, they will be reported.
 
-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

diff --git a/src/rules/no-commonjs.js b/src/rules/no-commonjs.js
@@ -25,6 +25,27 @@ function allowRequire(node, options) {
   return options.allowRequire
 }
 
+function allowConditionalRequire(node, options) {
+  return options.allowConditionalRequire === undefined ? true : options.allowConditionalRequire
+}
+
+function validateScope(scope) {
+  if (scope.variableScope.type === 'module') return true
+  return false
+}
+
+// https://github.com/estree/estree/blob/master/es5.md
+function isConditional(node) {
+  if (
+    node.type === 'IfStatement'
+    || node.type === 'TryStatement'
+    || node.type === 'LogicalExpression'
+    || node.type === 'ConditionalExpression'
+  ) return true
+  if (node.parent) return isConditional(node.parent)
+  return false
+}
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -35,6 +56,7 @@ const schemaObject = {
   properties: {
     allowPrimitiveModules: { 'type': 'boolean' },
     allowRequire: { 'type': 'boolean' },
+    allowConditionalRequire: { 'type': 'boolean' },
   },
   additionalProperties: false,
 }
@@ -87,12 +109,7 @@ module.exports = {
 
       },
       'CallExpression': function (call) {
-        if (context.getScope().type !== 'module') return
-        if (
-          call.parent.type !== 'ExpressionStatement'
-          && call.parent.type !== 'VariableDeclarator'
-          && call.parent.type !== 'AssignmentExpression'
-        ) return
+        if (!validateScope(context.getScope())) return
 
         if (call.callee.type !== 'Identifier') return
         if (call.callee.name !== 'require') return
@@ -105,6 +122,8 @@ module.exports = {
 
         if (allowRequire(call, options)) return
 
+        if (allowConditionalRequire(call, options) && isConditional(call.parent)) 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
@@ -56,6 +56,13 @@ ruleTester.run('no-commonjs', require('rules/no-commonjs'), {
     { code: 'module.exports = function () {}', options: [{ allowPrimitiveModules: true }] },
     { code: 'module.exports = "foo"', options: ['allow-primitive-modules'] },
     { code: 'module.exports = "foo"', options: [{ allowPrimitiveModules: true }] },
+
+    { code: 'if (typeof window !== "undefined") require("x")', options: [{ allowRequire: true }] },
+    { code: 'if (typeof window !== "undefined") require("x")', options: [{ allowRequire: false }] },
+    { code: 'if (typeof window !== "undefined") { require("x") }', options: [{ allowRequire: true }] },
+    { code: 'if (typeof window !== "undefined") { require("x") }', options: [{ allowRequire: false }] },
+
+    { code: 'try { require("x") } catch (error) {}' },
   ],
 
   invalid: [
@@ -65,6 +72,19 @@ ruleTester.run('no-commonjs', require('rules/no-commonjs'), {
       { code: 'var x = require("x")', errors: [ { message: IMPORT_MESSAGE }] },
       { code: 'x = require("x")', errors: [ { message: IMPORT_MESSAGE }] },
       { code: 'require("x")', errors: [ { message: IMPORT_MESSAGE }] },
+
+      { code: 'if (typeof window !== "undefined") require("x")',
+        options: [{ allowConditionalRequire: false }],
+        errors: [ { message: IMPORT_MESSAGE }],
+      },
+      { code: 'if (typeof window !== "undefined") { require("x") }',
+        options: [{ allowConditionalRequire: false }],
+        errors: [ { message: IMPORT_MESSAGE }],
+      },
+      { code: 'try { require("x") } catch (error) {}',
+        options: [{ allowConditionalRequire: false }],
+        errors: [ { message: IMPORT_MESSAGE }],
+      },
     ]),
 
     // exports