fix: including chnages for MDTP-prefer-object to MDTP-returns-object

DianaSuvorova · DianaSuvorova · commit 6b447b18bcb5 · 2018-02-11T14:42:34.000-08:00
diff --git a/docs/rules/mapDispatchToProps-prefer-object.md b/docs/rules/mapDispatchToProps-prefer-object.md
diff --git a/docs/rules/mapDispatchToProps-returns-object.md b/docs/rules/mapDispatchToProps-returns-object.md
@@ -0,0 +1,99 @@
+#  Enforces that mapDispatchToProps returns an object. (react-redux/mapDispatchToProps-returns-object)
+
+Enforces that the mapDispatchToProps is an object or a function returning an object.
+
+*Note: All of the caught cases would have caused a runtime [warning](https://github.com/reactjs/react-redux/blob/master/src/utils/verifyPlainObject.js) by react-redux *  
+
+## Rule details
+
+The following pattern is considered incorrect:
+
+```js
+const mapDispatchToProps = (dispatch) => dispatch(action())
+```
+
+```js
+connect((state) => state, (dispatch) => dispatch(action()))(App)
+```
+
+```js
+const mapDispatchToProps = () => {}
+```
+
+The following patterns are considered correct:
+
+
+```js
+const mapDispatchToProps = {}
+```
+
+```js
+const mapDispatchToProps = null;
+```
+
+```js
+const mapDispatchToProps = {anAction: anAction}
+```
+
+```js
+const mapDispatchToProps = (dispatch) => ({anAction: dispatch(anAction())})
+```
+
+## Not supported use cases.
+
+#### mapDispatchToProps is a function but actions are not bound to dispatch
+
+>If a function is passed, it will be given dispatch as the first parameter. It’s up to you to return an object that somehow uses dispatch to bind action creators in your own way.
+
+Below use case is likely not what you want but will not be enforced by this rule nor runtime react-redux check:
+
+```js
+const mapDispatchToProps = () => ({
+  action
+});
+```
+
+In this scenario action wouldn't be wrapped in dispatch and thus wouldn't be triggered.
+
+Most likely it needs to be rewritten as:
+
+```js
+const mapDispatchToProps = {
+  action
+};
+```
+or
+
+```js
+const mapDispatchToProps = (dispatch) => ({
+  action: () => dispatch(action())
+});
+```
+
+or
+
+```js
+const mapDispatchToProps = (dispatch) => ({
+  action: () => bindActionCreators(action, dispatch)
+});
+```
+
+#### mapDispatchToProps is equal to or returns a variable
+
+Note that if mapDispatchToProps is assigned a value of a variable there is no way for lint to know if the variable resolves to an object.
+
+So both of below use cases will be considered correct by the rule event though the second one is technically incorrect.
+
+This one would be caught by a react-redux check.
+
+```js
+const actionsMap = {
+  action1,
+  action2,
+};
+const mapDispatchToProps = actionsMap;
+```
+
+```js
+const mapDispatchToProps = aSingleAction;
+```
diff --git a/index.js b/index.js
@@ -1,7 +1,7 @@
 const rules = {
   'connect-prefer-minimum-two-arguments': require('./lib/rules/connect-prefer-minimum-two-arguments'),
   'connect-prefer-named-arguments': require('./lib/rules/connect-prefer-named-arguments'),
-  'mapDispatchToProps-prefer-object': require('./lib/rules/mapDispatchToProps-prefer-object'),
+  'mapDispatchToProps-returns-object': require('./lib/rules/mapDispatchToProps-returns-object'),
   'mapDispatchToProps-prefer-parameters-names': require('./lib/rules/mapDispatchToProps-prefer-parameters-names'),
   'mapStateToProps-no-store': require('./lib/rules/mapStateToProps-no-store'),
   'mapStateToProps-prefer-parameters-names': require('./lib/rules/mapStateToProps-prefer-parameters-names'),
@@ -27,7 +27,7 @@ module.exports = {
         'react-redux/connect-prefer-minimum-two-arguments': 0,
         'react-redux/connect-prefer-named-arguments': 2,
         'react-redux/mapDispatchToProps-prefer-parameters-names': 2,
-        'react-redux/mapDispatchToProps-prefer-object': 2,
+        'react-redux/mapDispatchToProps-returns-object': 2,
         'react-redux/mapStateToProps-no-store': 2,
         'react-redux/mapStateToProps-prefer-parameters-names': 2,
         'react-redux/prefer-separate-component-file': 1,
diff --git a/lib/rules/mapDispatchToProps-returns-object.js b/lib/rules/mapDispatchToProps-returns-object.js
@@ -1,4 +1,5 @@
 const isReactReduxConnect = require('../isReactReduxConnect');
+const utils = require('../utils');
 
 const report = function (context, node) {
   context.report({
@@ -17,14 +18,20 @@ module.exports = function (context) {
             decl.init.type === 'ArrowFunctionExpression' ||
             decl.init.type === 'FunctionExpression'
           )) {
-            report(context, decl);
+            const returnNode = utils.getReturnNode(decl.init);
+            if (!utils.isObject(returnNode)) {
+              report(context, node);
+            }
           }
         }
       });
     },
     FunctionDeclaration(node) {
       if (node.id && node.id.name === 'mapDispatchToProps') {
-        report(context, node.body);
+        const returnNode = utils.getReturnNode(node.body);
+        if (!utils.isObject(returnNode)) {
+          report(context, node);
+        }
       }
     },
     CallExpression(node) {
@@ -34,7 +41,10 @@ module.exports = function (context) {
           mapDispatchToProps.type === 'ArrowFunctionExpression' ||
           mapDispatchToProps.type === 'FunctionExpression')
         ) {
-          report(context, mapDispatchToProps);
+          const returnNode = utils.getReturnNode(mapDispatchToProps);
+          if (!utils.isObject(returnNode)) {
+            report(context, node);
+          }
         }
       }
     },
diff --git a/lib/utils.js b/lib/utils.js
@@ -1,18 +1,31 @@
 'use strict';
 
+const isObject = node => node && (
+  node.type === 'ObjectExpression' || node.type === 'Identifier'
+);
+
 const getReturnNode = (node) => {
   const body = node.body;
-  if (!body || !body.length) {
+  if (!body) {
     return node;
+  } else if (isObject(body)) {
+    return body;
+  } else if (body.type === 'BlockStatement') {
+    return getReturnNode(body);
   }
   for (let i = body.length - 1; i >= 0; i -= 1) {
     if (body[i].type === 'ReturnStatement') {
-      return body[i].argument;
+      const arg = body[i].argument;
+      if (arg.type === 'ArrowFunctionExpression' || arg.type === 'FunctionExpression') {
+        return getReturnNode(arg);
+      }
+      return arg;
     }
   }
   return null;
 };
 
 module.exports = {
   getReturnNode,
+  isObject,
 };
diff --git a/tests/lib/rules/mapDispatchToProps-returns-object.js b/tests/lib/rules/mapDispatchToProps-returns-object.js
@@ -1,6 +1,6 @@
 require('babel-eslint');
 
-const rule = require('../../../lib/rules/mapDispatchToProps-prefer-object');
+const rule = require('../../../lib/rules/mapDispatchToProps-returns-object');
 const RuleTester = require('eslint').RuleTester;
 
 const parserOptions = {
@@ -13,8 +13,11 @@ const parserOptions = {
 
 const ruleTester = new RuleTester({ parserOptions });
 
-ruleTester.run('mapDispatchToProps-prefer-object', rule, {
+ruleTester.run('mapDispatchToProps-returns-object', rule, {
   valid: [
+    'const mapDispatchToProps = {}',
+    'const mapDispatchToProps = null',
+    'const mapDispatchToProps = actionsMap',
     'const mapDispatchToProps = {...actions}',
     'const mapDispatchToProps = {anAction: anAction}',
     `export default connect(
@@ -24,54 +27,39 @@ ruleTester.run('mapDispatchToProps-prefer-object', rule, {
       { fetchProducts }
     )(Products);
     `,
-    'connect(null, null)(App)',
-  ],
-  invalid: [{
-    code: 'function mapDispatchToProps () {}',
-    errors: [
-      {
-        message: 'mapDispatchToProps should return object',
-      },
-    ],
-  }, {
-    code: 'const mapDispatchToProps = () => {}',
-    errors: [
-      {
-        message: 'mapDispatchToProps should return object',
-      },
-    ],
-  }, {
-    code: `const mapDispatchToProps = (dispatch) => (
+    'function mapDispatchToProps () {return {action}}',
+    `const mapDispatchToProps = (dispatch) => (
             {
                 requestFilteredItems: (client, keyword) => {
                     dispatch(requestFilteredItems(client, keyword));
                 }
             }
-        )`,
-    errors: [
-      {
-        message: 'mapDispatchToProps should return object',
-      },
-    ],
-  }, {
-    code: `function mapDispatchToProps(dispatch) {
-              return { requestFilteredItems: (client, keyword) => {
-                dispatch(requestFilteredItems(client, keyword));
-              }
-            }
-        }`,
+        )
+    `,
+    `const mapDispatchToProps = dispatch => ({
+      onDoSomething: () => dispatch(toSomethingElse())
+    });`,
+    `const mapDispatchToProps = function(dispatch) {
+          return { requestFilteredItems: (client, keyword) => {
+            dispatch(requestFilteredItems(client, keyword));
+          }
+        }
+    }`,
+    'connect(null, null)(App)',
+    'function mapDispatchToProps () {return aThing}',
+    `function mapDispatchToProps(dispatch) {
+      return { actions: bindActionCreators(actionCreators, dispatch) }
+    }`,
+  ],
+  invalid: [{
+    code: 'function mapDispatchToProps () {}',
     errors: [
       {
         message: 'mapDispatchToProps should return object',
       },
     ],
   }, {
-    code: `const mapDispatchToProps = function(dispatch) {
-              return { requestFilteredItems: (client, keyword) => {
-                dispatch(requestFilteredItems(client, keyword));
-              }
-            }
-        }`,
+    code: 'const mapDispatchToProps = () => {}',
     errors: [
       {
         message: 'mapDispatchToProps should return object',
