Skip to content

Commit 6a312ab

Browse files
authored
feat: mapStateToProps-prefer-selectors (#50)
1 parent 60da288 commit 6a312ab

File tree

5 files changed

+379
-0
lines changed

5 files changed

+379
-0
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,5 +54,6 @@ To configure individual rules:
5454
* [react-redux/mapStateToProps-no-store](docs/rules/mapStateToProps-no-store.md) Prohibits binding a whole store object to a component.
5555
* [react-redux/mapStateToProps-prefer-hoisted](docs/rules/mapStateToProps-prefer-hoisted.md) Flags generation of copies of same-by-value but different-by-reference props.
5656
* [react-redux/mapStateToProps-prefer-parameters-names](docs/rules/mapStateToProps-prefer-parameters-names.md) Enforces that all mapStateToProps parameters have specific names.
57+
* [react-redux/mapStateToProps-prefer-selectors](docs/rules/mapStateToProps-prefer-selectors.md) Enforces that all mapStateToProps properties use selector functions.
5758
* [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.
5859
* [react-redux/prefer-separate-component-file](docs/rules/prefer-separate-component-file.md) Enforces that all connected components are defined in a separate file.
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
# Enforces that all mapStateToProps properties use selector functions. (react-redux/mapStateToProps-prefer-selectors)
2+
3+
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.
4+
5+
## Rule details
6+
7+
The following pattern is considered incorrect:
8+
9+
```js
10+
const mapStateToProps = (state) => { x: state.property }
11+
```
12+
13+
```js
14+
connect(function(state) {
15+
return {
16+
y: state.other.property
17+
}
18+
}, null)(App)
19+
```
20+
21+
The following patterns are considered correct:
22+
23+
```js
24+
const propertySelector = (state) => state.property
25+
const mapStateToProps = (state) => { x: propertySelector(state) }
26+
```
27+
28+
```js
29+
const getOtherProperty = (state) => state.other.property
30+
connect(function(state) {
31+
return {
32+
y: getOtherProperty(state)
33+
}
34+
}, null)(App)
35+
```
36+
37+
## Rule Options
38+
39+
```js
40+
...
41+
"react-redux/mapStateToProps-prefer-selectors": [<enabled>, {
42+
"matching": <string>
43+
"validateParams": <boolean>
44+
}]
45+
...
46+
```
47+
48+
### `matching`
49+
If provided, validates the name of the selector functions against the RegExp pattern provided.
50+
51+
```js
52+
// .eslintrc
53+
{
54+
"react-redux/mapStateToProps-prefer-selectors": ["error", { matching: "^.*Selector$"}]
55+
}
56+
57+
// container.js
58+
const mapStateToProps = (state) => {
59+
x: xSelector(state), // success
60+
y: selectY(state), // failure
61+
}
62+
```
63+
64+
```js
65+
// .eslintrc
66+
{
67+
"react-redux/mapStateToProps-prefer-selectors": ["error", { matching: "^get.*FromState$"}]
68+
}
69+
70+
// container.js
71+
const mapStateToProps = (state) => {
72+
x: getXFromState(state), // success
73+
y: getY(state), // failure
74+
}
75+
```
76+
77+
### `validateParams`
78+
Boolean to determine if the selectors use the correct params (`<selectorFunction>(state, ownProps)`, where both params are optional). Defaults to true.
79+
80+
```js
81+
// .eslintrc
82+
{
83+
"react-redux/mapStateToProps-prefer-selectors": ["error", { validateParams: true }]
84+
}
85+
86+
// container.js
87+
const mapStateToProps = (state, ownProps) => {
88+
x: xSelector(state), // success
89+
y: ySelector(state, ownProps), // sucess
90+
z: zSelector(), // success
91+
a: aSelector(ownProps, state), // failure
92+
b: bSelector(state, someOtherValue) // failure
93+
}
94+
```

index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const rules = {
77
'mapStateToProps-no-store': require('./lib/rules/mapStateToProps-no-store'),
88
'mapStateToProps-prefer-hoisted': require('./lib/rules/mapStateToProps-prefer-hoisted'),
99
'mapStateToProps-prefer-parameters-names': require('./lib/rules/mapStateToProps-prefer-parameters-names'),
10+
'mapStateToProps-prefer-selectors': require('./lib/rules/mapStateToProps-prefer-selectors'),
1011
'no-unused-prop-types': require('./lib/rules/no-unused-prop-types'),
1112
'prefer-separate-component-file': require('./lib/rules/prefer-separate-component-file'),
1213
};
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
const isReactReduxConnect = require('../isReactReduxConnect');
2+
const utils = require('../utils');
3+
4+
const reportNoSelector = function (context, node, name) {
5+
context.report({
6+
message: `mapStateToProps property "${name}" should use a selector function.`,
7+
node,
8+
});
9+
};
10+
11+
const reportWrongName = function (context, node, propName, functionName, matching) {
12+
context.report({
13+
message: `mapStateToProps "${propName}"'s selector "${functionName}" does not match "${matching}".`,
14+
node,
15+
});
16+
};
17+
18+
const reportUnexpectedParam = function (context, node, propName, functionName, index) {
19+
context.report({
20+
message: `mapStateToProps "${propName}"'s selector "${functionName}" parameter #${index} is not expected.`,
21+
node,
22+
});
23+
};
24+
25+
const reportInvalidParams = function (context, node, propName, functionName, params, index) {
26+
context.report({
27+
message: `mapStateToProps "${propName}"'s selector "${functionName}" parameter #${index} should be "${params[index].name}".`,
28+
node,
29+
});
30+
};
31+
32+
const checkProperties = function (context, properties, matching, expectedParams) {
33+
properties.forEach((prop) => {
34+
if (prop.value.type !== 'CallExpression') {
35+
reportNoSelector(context, prop, prop.key.name);
36+
return;
37+
}
38+
if (matching && !prop.value.callee.name.match(new RegExp(matching))) {
39+
reportWrongName(context, prop, prop.key.name, prop.value.callee.name, matching);
40+
}
41+
if (expectedParams) {
42+
const actualParams = prop.value.arguments;
43+
const propName = prop.key.name;
44+
const functionName = prop.value.callee.name;
45+
actualParams.forEach((param, i) => {
46+
if (!expectedParams[i]) {
47+
reportUnexpectedParam(context, prop, propName, functionName, i);
48+
return;
49+
}
50+
if (param.name !== expectedParams[i].name) {
51+
reportInvalidParams(context, prop, propName, functionName, expectedParams, i);
52+
}
53+
});
54+
}
55+
});
56+
};
57+
58+
const check = function (context, node, matching, validateParams) {
59+
const returnNode = utils.getReturnNode(node);
60+
if (utils.isObject(returnNode)) {
61+
checkProperties(context, returnNode.properties, matching, validateParams && node.params);
62+
}
63+
};
64+
65+
module.exports = function (context) {
66+
const config = context.options[0] || {};
67+
return {
68+
VariableDeclaration(node) {
69+
node.declarations.forEach((decl) => {
70+
if (decl.id && decl.id.name === 'mapStateToProps') {
71+
if (decl.init && (
72+
decl.init.type === 'ArrowFunctionExpression' ||
73+
decl.init.type === 'FunctionExpression'
74+
)) {
75+
check(context, decl.init, config.matching, !(config.validateParams === false));
76+
}
77+
}
78+
});
79+
},
80+
FunctionDeclaration(node) {
81+
if (node.id && node.id.name === 'mapStateToProps') {
82+
check(context, node.body, config.matching, !(config.validateParams === false));
83+
}
84+
},
85+
CallExpression(node) {
86+
if (isReactReduxConnect(node)) {
87+
const mapStateToProps = node.arguments && node.arguments[0];
88+
if (mapStateToProps && (
89+
mapStateToProps.type === 'ArrowFunctionExpression' ||
90+
mapStateToProps.type === 'FunctionExpression')
91+
) {
92+
check(context, mapStateToProps, config.matching, !(config.validateParams === false));
93+
}
94+
}
95+
},
96+
};
97+
};
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
require('babel-eslint');
2+
3+
const rule = require('../../../lib/rules/mapStateToProps-prefer-selectors');
4+
const RuleTester = require('eslint').RuleTester;
5+
6+
const parserOptions = {
7+
ecmaVersion: 6,
8+
sourceType: 'module',
9+
ecmaFeatures: {
10+
experimentalObjectRestSpread: true,
11+
},
12+
};
13+
14+
const ruleTester = new RuleTester({ parserOptions });
15+
16+
ruleTester.run('mapStateToProps-prefer-selectors', rule, {
17+
valid: [
18+
'const mapStateToProps = (state) => 1',
19+
'const mapStateToProps = (state) => ({})',
20+
'const mapStateToProps = (state) => ({ x: xSelector(state) })',
21+
'const mapStateToProps = (state, ownProps) => ({ x: xSelector(state, ownProps) })',
22+
'const mapStateToProps = (state) => ({ x: xSelector(state), y: ySelector(state) })',
23+
'const mapStateToProps = (state) => { return { x: xSelector(state) }; }',
24+
'const mapStateToProps = (state) => { doSomethingElse(); return { x: xSelector(state) }; }',
25+
'const mapStateToProps = function(state) { return { x: xSelector(state) }; }',
26+
'function mapStateToProps(state) { doSomethingElse(); return { x: xSelector(state) }; }',
27+
'connect((state) => ({ x: xSelector(state) }), {})(Comp)',
28+
'const mapStateToProps = () => ({ x: xSelector() })',
29+
'const mapStateToProps = function(state) { return { x: getX() }; }',
30+
'const mapStateToProps = function(state) { return { x: getX(state) }; }',
31+
'connect((state, ownProps) => ({ x: selector() }), {})(Comp)',
32+
'connect((state, ownProps) => ({ x: selector(state) }), {})(Comp)',
33+
'connect((state, ownProps) => ({ x: selector(state, ownProps) }), {})(Comp)',
34+
{
35+
code: 'const mapStateToProps = (state) => ({ x: xSelector(state) })',
36+
options: [{
37+
matching: '^.*Selector$',
38+
}],
39+
},
40+
{
41+
code: 'const mapStateToProps = function(state) { return { x: getX(state) }; }',
42+
options: [{
43+
matching: '^get.*$',
44+
}],
45+
},
46+
{
47+
code: 'connect((state) => ({ x: selector(state) }), {})(Comp)',
48+
options: [{
49+
matching: '^selector$',
50+
}],
51+
},
52+
{
53+
code: 'const mapStateToProps = (state) => ({ x: xSelector(differentParam) })',
54+
options: [{
55+
validateParams: false,
56+
}],
57+
},
58+
{
59+
code: 'const mapStateToProps = function(state) { return { x: getX(state, ownProps2) }; }',
60+
options: [{
61+
validateParams: false,
62+
}],
63+
},
64+
{
65+
code: 'connect(() => ({ x: selector(state) }), {})(Comp)',
66+
options: [{
67+
validateParams: false,
68+
}],
69+
},
70+
],
71+
invalid: [{
72+
code: 'const mapStateToProps = (state) => ({ x: state.b })',
73+
errors: [
74+
{
75+
message: 'mapStateToProps property "x" should use a selector function.',
76+
},
77+
],
78+
}, {
79+
code: 'const mapStateToProps = (state) => ({ x: state.x, y: state.y })',
80+
errors: [
81+
{
82+
message: 'mapStateToProps property "x" should use a selector function.',
83+
},
84+
{
85+
message: 'mapStateToProps property "y" should use a selector function.',
86+
},
87+
],
88+
}, {
89+
code: 'const mapStateToProps = (state) => ({ x: state.x, y: ySelector(state) })',
90+
errors: [
91+
{
92+
message: 'mapStateToProps property "x" should use a selector function.',
93+
},
94+
],
95+
}, {
96+
code: 'const mapStateToProps = (state) => { return { x: state.b }; }',
97+
errors: [
98+
{
99+
message: 'mapStateToProps property "x" should use a selector function.',
100+
},
101+
],
102+
}, {
103+
code: 'const mapStateToProps = (state) => { doSomethingElse(); return { x: state.b }; }',
104+
errors: [
105+
{
106+
message: 'mapStateToProps property "x" should use a selector function.',
107+
},
108+
],
109+
}, {
110+
code: 'const mapStateToProps = function(state) { return { x: state.x }; }',
111+
errors: [
112+
{
113+
message: 'mapStateToProps property "x" should use a selector function.',
114+
},
115+
],
116+
}, {
117+
code: 'function mapStateToProps(state) { doSomethingElse(); return { x: state.b }; }',
118+
errors: [
119+
{
120+
message: 'mapStateToProps property "x" should use a selector function.',
121+
},
122+
],
123+
}, {
124+
code: 'connect((state) => ({ x: state.x }), {})(Comp)',
125+
errors: [
126+
{
127+
message: 'mapStateToProps property "x" should use a selector function.',
128+
},
129+
],
130+
}, {
131+
code: 'const mapStateToProps = (state) => ({ x: xSelector(state) })',
132+
options: [{
133+
matching: '^get.*$',
134+
}],
135+
errors: [{
136+
message: 'mapStateToProps "x"\'s selector "xSelector" does not match "^get.*$".',
137+
}],
138+
}, {
139+
code: 'const mapStateToProps = function(state) { return { x: getX(state) }; }',
140+
options: [{
141+
matching: '^.*Selector$',
142+
}],
143+
errors: [{
144+
message: 'mapStateToProps "x"\'s selector "getX" does not match "^.*Selector$".',
145+
}],
146+
}, {
147+
code: 'connect((state) => ({ x: selectorr(state) }), {})(Comp)',
148+
options: [{
149+
matching: '^selector$',
150+
}],
151+
errors: [{
152+
message: 'mapStateToProps "x"\'s selector "selectorr" does not match "^selector$".',
153+
}],
154+
}, {
155+
code: 'const mapStateToProps = (state) => ({ x: xSelector(state, ownProps) })',
156+
errors: [{
157+
message: 'mapStateToProps "x"\'s selector "xSelector" parameter #1 is not expected.',
158+
}],
159+
}, {
160+
code: 'const mapStateToProps = (state, ownProps) => ({ x: xSelector(state, ownProps, someOtherValue) })',
161+
errors: [{
162+
message: 'mapStateToProps "x"\'s selector "xSelector" parameter #2 is not expected.',
163+
}],
164+
}, {
165+
code: 'const mapStateToProps = function(state) { return { x: getX(notState) }; }',
166+
errors: [{
167+
message: 'mapStateToProps "x"\'s selector "getX" parameter #0 should be "state".',
168+
}],
169+
}, {
170+
code: 'connect((state, ownProps) => ({ x: getX(state, notOwnProps) }), {})(Comp)',
171+
errors: [{
172+
message: 'mapStateToProps "x"\'s selector "getX" parameter #1 should be "ownProps".',
173+
}],
174+
}, {
175+
code: 'connect((state2, ownProps) => ({ x: getX(state) }), {})(Comp)',
176+
errors: [{
177+
message: 'mapStateToProps "x"\'s selector "getX" parameter #0 should be "state2".',
178+
}],
179+
}, {
180+
code: 'connect((state, ownProps2) => ({ x: getX(state, ownProps) }), {})(Comp)',
181+
errors: [{
182+
message: 'mapStateToProps "x"\'s selector "getX" parameter #1 should be "ownProps2".',
183+
}],
184+
}],
185+
186+
});

0 commit comments

Comments
 (0)