Fix ESLint rule crash (#18455)
Dan Abramov committed
Apr 1, 2020 at 20:54 UTC
2bf60d9f9ccf64625caf298450b92ec283a9020d
2 files changed
+65
-25
packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
+31
@@ -357,6 +357,14 @@ const tests = {
357
}
358
`,
359
},
360
+ {
361
+ // Valid because has no deps.
362
+ code: normalizeIndent`
363
+ function MyComponent({myEffect}) {
364
+ useEffect(myEffect);
365
+ }
366
+ `,
367
+ },
368
{
369
code: normalizeIndent`
370
function MyComponent(props) {
@@ -1273,6 +1281,29 @@ const tests = {
1281
},
1282
],
1283
},
1284
+ {
1285
+ // Invalid because they don't have a meaning without deps.
1286
+ code: normalizeIndent`
1287
+ function MyComponent({ fn1, fn2 }) {
1288
+ const value = useMemo(fn1);
1289
+ const fn = useCallback(fn2);
1290
+ }
1291
+ `,
1292
+ errors: [
1293
+ {
1294
+ message:
1295
+ 'React Hook useMemo does nothing when called with only one argument. ' +
1296
+ 'Did you forget to pass an array of dependencies?',
1297
+ suggestions: undefined,
1298
+ },
1299
+ {
1300
+ message:
1301
+ 'React Hook useCallback does nothing when called with only one argument. ' +
1302
+ 'Did you forget to pass an array of dependencies?',
1303
+ suggestions: undefined,
1304
+ },
1305
+ ],
1306
+ },
1307
{
1308
// Regression test
1309
code: normalizeIndent`
packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
+34
-25
@@ -93,6 +93,28 @@ export default {
93
const reactiveHook = node.callee;
94
const reactiveHookName = getNodeWithoutReactNamespace(reactiveHook).name;
95
const declaredDependenciesNode = node.arguments[callbackIndex + 1];
96
+ const isEffect = /Effect($|[^a-z])/g.test(reactiveHookName);
97
+
98
+ // Check the declared dependencies for this reactive hook. If there is no
99
+ // second argument then the reactive callback will re-run on every render.
100
+ // So no need to check for dependency inclusion.
101
+ if (!declaredDependenciesNode && !isEffect) {
102
+ // These are only used for optimization.
103
+ if (
104
+ reactiveHookName === 'useMemo' ||
105
+ reactiveHookName === 'useCallback'
106
+ ) {
107
+ // TODO: Can this have a suggestion?
108
+ reportProblem({
109
+ node: reactiveHook,
110
+ message:
111
+ `React Hook ${reactiveHookName} does nothing when called with ` +
112
+ `only one argument. Did you forget to pass an array of ` +
113
+ `dependencies?`,
114
+ });
115
+ }
116
+ return;
117
+ }
118
119
switch (callback.type) {
120
case 'FunctionExpression':
@@ -101,13 +123,18 @@ export default {
123
callback,
124
declaredDependenciesNode,
125
reactiveHook,
126
+ reactiveHookName,
127
+ isEffect,
128
);
129
return; // Handled
130
case 'Identifier':
131
+ if (!declaredDependenciesNode) {
132
+ // No deps, no problems.
133
+ return; // Handled
134
+ }
135
// The function passed as a callback is not written inline.
136
// But perhaps it's in the dependencies array?
137
if (
110
- declaredDependenciesNode &&
138
declaredDependenciesNode.elements &&
139
declaredDependenciesNode.elements.some(
140
el => el.type === 'Identifier' && el.name === callback.name,
@@ -141,6 +168,8 @@ export default {
168
def.node,
169
declaredDependenciesNode,
170
reactiveHook,
171
+ reactiveHookName,
172
+ isEffect,
173
);
174
return; // Handled
175
case 'VariableDeclarator':
@@ -158,6 +187,8 @@ export default {
187
init,
188
declaredDependenciesNode,
189
reactiveHook,
190
+ reactiveHookName,
191
+ isEffect,
192
);
193
return; // Handled
194
}
@@ -202,31 +233,9 @@ export default {
233
node,
234
declaredDependenciesNode,
235
reactiveHook,
236
+ reactiveHookName,
237
+ isEffect,
238
) {
206
- const reactiveHookName = getNodeWithoutReactNamespace(reactiveHook).name;
207
- const isEffect = /Effect($|[^a-z])/g.test(reactiveHookName);
208
-
209
- // Check the declared dependencies for this reactive hook. If there is no
210
- // second argument then the reactive callback will re-run on every render.
211
- // So no need to check for dependency inclusion.
212
- if (!declaredDependenciesNode && !isEffect) {
213
- // These are only used for optimization.
214
- if (
215
- reactiveHookName === 'useMemo' ||
216
- reactiveHookName === 'useCallback'
217
- ) {
218
- // TODO: Can this have a suggestion?
219
- reportProblem({
220
- node: reactiveHook,
221
- message:
222
- `React Hook ${reactiveHookName} does nothing when called with ` +
223
- `only one argument. Did you forget to pass an array of ` +
224
- `dependencies?`,
225
- });
226
- }
227
- return;
228
- }
229
-
239
if (isEffect && node.async) {
240
reportProblem({
241
node: node,