From dc56010c2511dcbb455c43557cc9c0947a6d61c7 Mon Sep 17 00:00:00 2001 From: async Date: Fri, 10 Jul 2020 17:12:39 -0700 Subject: [PATCH 1/3] feat: mapStateToProps-prefer-selectors --- README.md | 1 + .../rules/mapStateToProps-prefer-selectors.md | 52 +++++ index.js | 1 + lib/rules/mapStateToProps-prefer-selectors.js | 97 ++++++++++ .../rules/mapStateToProps-prefer-selectors.js | 181 ++++++++++++++++++ 5 files changed, 332 insertions(+) create mode 100644 docs/rules/mapStateToProps-prefer-selectors.md create mode 100644 lib/rules/mapStateToProps-prefer-selectors.js create mode 100644 tests/lib/rules/mapStateToProps-prefer-selectors.js diff --git a/README.md b/README.md index 71afbfb..7942446 100644 --- a/README.md +++ b/README.md @@ -54,5 +54,6 @@ To configure individual rules: * [react-redux/mapStateToProps-no-store](docs/rules/mapStateToProps-no-store.md) Prohibits binding a whole store object to a component. * [react-redux/mapStateToProps-prefer-hoisted](docs/rules/mapStateToProps-prefer-hoisted.md) Flags generation of copies of same-by-value but different-by-reference props. * [react-redux/mapStateToProps-prefer-parameters-names](docs/rules/mapStateToProps-prefer-parameters-names.md) Enforces that all mapStateToProps parameters have specific names. +* [react-redux/mapStateToProps-prefer-selectors](docs/rules/mapStateToProps-prefer-selectors.md) Enforces that all mapStateToProps properties use selector functions. * [react-redux/no-unused-prop-types](docs/rules/no-unused-prop-types.md) Extension of a react's no-unused-prop-types rule filtering out false positive used in redux context. * [react-redux/prefer-separate-component-file](docs/rules/prefer-separate-component-file.md) Enforces that all connected components are defined in a separate file. diff --git a/docs/rules/mapStateToProps-prefer-selectors.md b/docs/rules/mapStateToProps-prefer-selectors.md new file mode 100644 index 0000000..2efefcc --- /dev/null +++ b/docs/rules/mapStateToProps-prefer-selectors.md @@ -0,0 +1,52 @@ +# Enforces that all mapStateToProps properties use selector functions. (react-redux/mapStateToProps-prefer-selectors) + +Using selectors in `mapStateToProps` to pull data from the store or [compute derived data](https://redux.js.org/recipes/computing-derived-data#composing-selectors) allows you to uncouple your containers from the state architecture and more easily enable memoization. This rule will ensure that every prop utilizes a selector. + +## Rule details + +The following pattern is considered incorrect: + +```js +const mapStateToProps = (state) => { x: state.property } +``` + +```js +connect(function(state) { + return { + y: state.other.property + } +}, null)(App) +``` + +The following patterns are considered correct: + +```js +const propertySelector = (state) => state.property +const mapStateToProps = (state) => { x: propertySelector(state) } +``` + +```js +const getOtherProperty = (state) => state.other.property +connect(function(state) { + return { + y: getOtherProperty(state) + } +}, null)(App) +``` + +## Rule Options + +```js +... +"react-redux/mapStateToProps-prefer-selectors": [, { + "matching": + "validateParams": +}] +... +``` + +### `matching` +If provided, validates the name of the selector functions against the RegExp pattern provided. + +### `validateParams` +Boolean to determine if the selectors use the correct params (`selectorFunction(state, ownProps)`, where both params are optional). Defaults to true. \ No newline at end of file diff --git a/index.js b/index.js index dc451e2..62ee7be 100644 --- a/index.js +++ b/index.js @@ -7,6 +7,7 @@ const rules = { 'mapStateToProps-no-store': require('./lib/rules/mapStateToProps-no-store'), 'mapStateToProps-prefer-hoisted': require('./lib/rules/mapStateToProps-prefer-hoisted'), 'mapStateToProps-prefer-parameters-names': require('./lib/rules/mapStateToProps-prefer-parameters-names'), + 'mapStateToProps-prefer-selectors': require('./lib/rules/mapStateToProps-prefer-selectors'), 'no-unused-prop-types': require('./lib/rules/no-unused-prop-types'), 'prefer-separate-component-file': require('./lib/rules/prefer-separate-component-file'), }; diff --git a/lib/rules/mapStateToProps-prefer-selectors.js b/lib/rules/mapStateToProps-prefer-selectors.js new file mode 100644 index 0000000..995cf96 --- /dev/null +++ b/lib/rules/mapStateToProps-prefer-selectors.js @@ -0,0 +1,97 @@ +const isReactReduxConnect = require('../isReactReduxConnect'); +const utils = require('../utils'); + +const reportNoSelector = function (context, node, name) { + context.report({ + message: `mapStateToProps property "${name}" should use a selector function.`, + node, + }); +}; + +const reportWrongName = function (context, node, propName, functionName, matching) { + context.report({ + message: `mapStateToProps "${propName}"'s selector "${functionName}" does not match "${matching}".`, + node, + }); +}; + +const reportUnexpectedParam = function (context, node, propName, functionName, index) { + context.report({ + message: `mapStateToProps "${propName}"'s selector "${functionName}" parameter #${index} is not expected.`, + node, + }); +}; + +const reportInvalidParams = function (context, node, propName, functionName, params, index) { + context.report({ + message: `mapStateToProps "${propName}"'s selector "${functionName}" parameter #${index} should be "${params[index].name}".`, + node, + }); +}; + +const checkProperties = function (context, properties, matching, expectedParams) { + properties.forEach((prop) => { + if (prop.value.type !== 'CallExpression') { + reportNoSelector(context, prop, prop.key.name); + return; + } + if (matching && !prop.value.callee.name.match(new RegExp(matching))) { + reportWrongName(context, prop, prop.key.name, prop.value.callee.name, matching); + } + if (expectedParams) { + const actualParams = prop.value.arguments; + const propName = prop.key.name; + const functionName = prop.value.callee.name; + actualParams.forEach((param, i) => { + if (!expectedParams[i]) { + reportUnexpectedParam(context, prop, propName, functionName, i); + return; + } + if (param.name !== expectedParams[i].name) { + reportInvalidParams(context, prop, propName, functionName, expectedParams, i); + } + }); + } + }); +}; + +const check = function (context, node, matching, validateParams) { + const returnNode = utils.getReturnNode(node); + if (utils.isObject(returnNode)) { + checkProperties(context, returnNode.properties, matching, validateParams && node.params); + } +}; + +module.exports = function (context) { + const config = context.options[0] || {}; + return { + VariableDeclaration(node) { + node.declarations.forEach((decl) => { + if (decl.id && decl.id.name === 'mapStateToProps') { + if (decl.init && ( + decl.init.type === 'ArrowFunctionExpression' || + decl.init.type === 'FunctionExpression' + )) { + check(context, decl.init, config.matching, !(config.validateParams === false)); + } + } + }); + }, + FunctionDeclaration(node) { + if (node.id && node.id.name === 'mapStateToProps') { + check(context, node.body, config.matching, !(config.validateParams === false)); + } + }, + CallExpression(node) { + if (isReactReduxConnect(node)) { + const mapStateToProps = node.arguments && node.arguments[0]; + if (mapStateToProps && ( + mapStateToProps.type === 'ArrowFunctionExpression' || + mapStateToProps.type === 'FunctionExpression') + ) { + check(context, mapStateToProps, config.matching, !(config.validateParams === false)); + } + } + }, + }; +}; diff --git a/tests/lib/rules/mapStateToProps-prefer-selectors.js b/tests/lib/rules/mapStateToProps-prefer-selectors.js new file mode 100644 index 0000000..c558f3f --- /dev/null +++ b/tests/lib/rules/mapStateToProps-prefer-selectors.js @@ -0,0 +1,181 @@ +require('babel-eslint'); + +const rule = require('../../../lib/rules/mapStateToProps-prefer-selectors'); +const RuleTester = require('eslint').RuleTester; + +const parserOptions = { + ecmaVersion: 6, + sourceType: 'module', + ecmaFeatures: { + experimentalObjectRestSpread: true, + }, +}; + +const ruleTester = new RuleTester({ parserOptions }); + +ruleTester.run('mapStateToProps-prefer-selectors', rule, { + valid: [ + 'const mapStateToProps = (state) => 1', + 'const mapStateToProps = (state) => ({})', + 'const mapStateToProps = (state) => ({ x: xSelector(state) })', + 'const mapStateToProps = (state, ownProps) => ({ x: xSelector(state, ownProps) })', + 'const mapStateToProps = (state) => ({ x: xSelector(state), y: ySelector(state) })', + 'const mapStateToProps = (state) => { return { x: xSelector(state) }; }', + 'const mapStateToProps = (state) => { doSomethingElse(); return { x: xSelector(state) }; }', + 'const mapStateToProps = function(state) { return { x: xSelector(state) }; }', + 'function mapStateToProps(state) { doSomethingElse(); return { x: xSelector(state) }; }', + 'connect((state) => ({ x: xSelector(state) }), {})(Comp)', + 'const mapStateToProps = () => ({ x: xSelector() })', + 'const mapStateToProps = function(state) { return { x: getX() }; }', + 'const mapStateToProps = function(state) { return { x: getX(state) }; }', + 'connect((state, ownProps) => ({ x: selector() }), {})(Comp)', + 'connect((state, ownProps) => ({ x: selector(state) }), {})(Comp)', + 'connect((state, ownProps) => ({ x: selector(state, ownProps) }), {})(Comp)', + { + code: 'const mapStateToProps = (state) => ({ x: xSelector(state) })', + options: [{ + matching: '^.*Selector$', + }], + }, + { + code: 'const mapStateToProps = function(state) { return { x: getX(state) }; }', + options: [{ + matching: '^get.*$', + }], + }, + { + code: 'connect((state) => ({ x: selector(state) }), {})(Comp)', + options: [{ + matching: '^selector$', + }], + }, + { + code: 'const mapStateToProps = (state) => ({ x: xSelector(differentParam) })', + options: [{ + validateParams: false, + }], + }, + { + code: 'const mapStateToProps = function(state) { return { x: getX(state, ownProps2) }; }', + options: [{ + validateParams: false, + }], + }, + { + code: 'connect(() => ({ x: selector(state) }), {})(Comp)', + options: [{ + validateParams: false, + }], + }, + ], + invalid: [{ + code: 'const mapStateToProps = (state) => ({ x: state.b })', + errors: [ + { + message: 'mapStateToProps property "x" should use a selector function.', + }, + ], + }, { + code: 'const mapStateToProps = (state) => ({ x: state.x, y: state.y })', + errors: [ + { + message: 'mapStateToProps property "x" should use a selector function.', + }, + { + message: 'mapStateToProps property "y" should use a selector function.', + }, + ], + }, { + code: 'const mapStateToProps = (state) => ({ x: state.x, y: ySelector(state) })', + errors: [ + { + message: 'mapStateToProps property "x" should use a selector function.', + }, + ], + }, { + code: 'const mapStateToProps = (state) => { return { x: state.b }; }', + errors: [ + { + message: 'mapStateToProps property "x" should use a selector function.', + }, + ], + }, { + code: 'const mapStateToProps = (state) => { doSomethingElse(); return { x: state.b }; }', + errors: [ + { + message: 'mapStateToProps property "x" should use a selector function.', + }, + ], + }, { + code: 'const mapStateToProps = function(state) { return { x: state.x }; }', + errors: [ + { + message: 'mapStateToProps property "x" should use a selector function.', + }, + ], + }, { + code: 'function mapStateToProps(state) { doSomethingElse(); return { x: state.b }; }', + errors: [ + { + message: 'mapStateToProps property "x" should use a selector function.', + }, + ], + }, { + code: 'connect((state) => ({ x: state.x }), {})(Comp)', + errors: [ + { + message: 'mapStateToProps property "x" should use a selector function.', + }, + ], + }, { + code: 'const mapStateToProps = (state) => ({ x: xSelector(state) })', + options: [{ + matching: '^get.*$', + }], + errors: [{ + message: 'mapStateToProps "x"\'s selector "xSelector" does not match "^get.*$".', + }], + }, { + code: 'const mapStateToProps = function(state) { return { x: getX(state) }; }', + options: [{ + matching: '^.*Selector$', + }], + errors: [{ + message: 'mapStateToProps "x"\'s selector "getX" does not match "^.*Selector$".', + }], + }, { + code: 'connect((state) => ({ x: selectorr(state) }), {})(Comp)', + options: [{ + matching: '^selector$', + }], + errors: [{ + message: 'mapStateToProps "x"\'s selector "selectorr" does not match "^selector$".', + }], + }, { + code: 'const mapStateToProps = (state) => ({ x: xSelector(state, ownProps) })', + errors: [{ + message: 'mapStateToProps "x"\'s selector "xSelector" parameter #1 is not expected.', + }], + }, { + code: 'const mapStateToProps = function(state) { return { x: getX(notState) }; }', + errors: [{ + message: 'mapStateToProps "x"\'s selector "getX" parameter #0 should be "state".', + }], + }, { + code: 'connect((state, ownProps) => ({ x: getX(state, notOwnProps) }), {})(Comp)', + errors: [{ + message: 'mapStateToProps "x"\'s selector "getX" parameter #1 should be "ownProps".', + }], + }, { + code: 'connect((state2, ownProps) => ({ x: getX(state) }), {})(Comp)', + errors: [{ + message: 'mapStateToProps "x"\'s selector "getX" parameter #0 should be "state2".', + }], + }, { + code: 'connect((state, ownProps2) => ({ x: getX(state, ownProps) }), {})(Comp)', + errors: [{ + message: 'mapStateToProps "x"\'s selector "getX" parameter #1 should be "ownProps2".', + }], + }], + +}); From b6f08c9c711d6c820e45dac94bc9bf2263b17c5a Mon Sep 17 00:00:00 2001 From: async Date: Sun, 12 Jul 2020 20:29:08 -0700 Subject: [PATCH 2/3] docs: add option examples to prefer selector documentation --- .../rules/mapStateToProps-prefer-selectors.md | 44 ++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/docs/rules/mapStateToProps-prefer-selectors.md b/docs/rules/mapStateToProps-prefer-selectors.md index 2efefcc..15305bc 100644 --- a/docs/rules/mapStateToProps-prefer-selectors.md +++ b/docs/rules/mapStateToProps-prefer-selectors.md @@ -48,5 +48,47 @@ connect(function(state) { ### `matching` If provided, validates the name of the selector functions against the RegExp pattern provided. +```js + // .eslintrc + { + "react-redux/mapStateToProps-prefer-selectors": ["error", { matching: "^.*Selector$"}] + } + + // container.js + const mapStateToProps = (state) => { + x: xSelector(state), // success + y: selectY(state), // failure + } +``` + +```js + // .eslintrc + { + "react-redux/mapStateToProps-prefer-selectors": ["error", { matching: "^get.*FromState$"}] + } + + // container.js + const mapStateToProps = (state) => { + x: getXFromState(state), // success + y: getY(state), // failure + } +``` + ### `validateParams` -Boolean to determine if the selectors use the correct params (`selectorFunction(state, ownProps)`, where both params are optional). Defaults to true. \ No newline at end of file +Boolean to determine if the selectors use the correct params (`(state, ownProps)`, where both params are optional). Defaults to true. + +```js + // .eslintrc + { + "react-redux/mapStateToProps-prefer-selectors": ["error", { validateParams: true }] + } + + // container.js + const mapStateToProps = (state, ownProps) => { + x: xSelector(state), // success + y: ySelector(state, ownProps), // sucess + z: zSelector(), // success + a: aSelector(ownProps, state), // failure + b: bSelector(state, someOtherValue) // failure + } +``` \ No newline at end of file From 3274dbe5bb53ec54b5bd0aeb0ca97e0d71a4e36d Mon Sep 17 00:00:00 2001 From: async Date: Sun, 12 Jul 2020 20:29:33 -0700 Subject: [PATCH 3/3] test: add extra test case for parameter validation --- tests/lib/rules/mapStateToProps-prefer-selectors.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/lib/rules/mapStateToProps-prefer-selectors.js b/tests/lib/rules/mapStateToProps-prefer-selectors.js index c558f3f..1fbf90f 100644 --- a/tests/lib/rules/mapStateToProps-prefer-selectors.js +++ b/tests/lib/rules/mapStateToProps-prefer-selectors.js @@ -156,6 +156,11 @@ ruleTester.run('mapStateToProps-prefer-selectors', rule, { errors: [{ message: 'mapStateToProps "x"\'s selector "xSelector" parameter #1 is not expected.', }], + }, { + code: 'const mapStateToProps = (state, ownProps) => ({ x: xSelector(state, ownProps, someOtherValue) })', + errors: [{ + message: 'mapStateToProps "x"\'s selector "xSelector" parameter #2 is not expected.', + }], }, { code: 'const mapStateToProps = function(state) { return { x: getX(notState) }; }', errors: [{