Skip to content

Commit 2329324

Browse files
add avoidObjects option & warning message
1 parent 036d68b commit 2329324

File tree

3 files changed

+39
-7
lines changed

3 files changed

+39
-7
lines changed

README.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
# `eslint-plugin-react-hooks-unreliable-deps`
2+
[![npm](https://img.shields.io/npm/v/eslint-plugin-react-hooks-unreliable-deps.svg?style=flat-square)](https://www.npmjs.com/package/eslint-plugin-react-hooks-unreliable-deps)
23

34
This is a companion ESLint plugin for [`eslint-plugin-react-hooks`](https://github.com/facebook/react/tree/main/packages/eslint-plugin-react-hooks) to warn about potential issues arising from reference equality in [React Hooks API](https://reactjs.org/docs/hooks-intro.html) dependency arrays.
45

@@ -37,7 +38,9 @@ If you want more fine-grained configuration, you can instead add a snippet like
3738
],
3839
"rules": {
3940
// ...
40-
"react-hooks-unreliable-deps/reference-deps": "warn",
41+
"react-hooks-unreliable-deps/reference-deps": ["warn", {
42+
"avoidObjects": true
43+
}]
4144
}
4245
}
4346
```
@@ -53,7 +56,8 @@ This option accepts a regex to match the names of custom Hooks that have depende
5356
"rules": {
5457
// ...
5558
"react-hooks-unreliable-deps/reference-deps": ["warn", {
56-
"additionalHooks": "(useMyCustomHook|useMyOtherCustomHook)"
59+
"additionalHooks": "(useMyCustomHook|useMyOtherCustomHook)",
60+
"avoidObjects": true
5761
}]
5862
}
5963
}

src/ReferenceDeps.js

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ export default {
2828
additionalHooks: {
2929
type: 'string',
3030
},
31+
avoidObjects: {
32+
type: 'boolean',
33+
},
3134
},
3235
},
3336
],
@@ -41,8 +44,16 @@ export default {
4144
? new RegExp(context.options[0].additionalHooks)
4245
: undefined;
4346

47+
const avoidObjects =
48+
context.options &&
49+
context.options[0] &&
50+
context.options[0].avoidObjects
51+
? true
52+
: false;
53+
4454
const options = {
4555
additionalHooks,
56+
avoidObjects,
4657
};
4758

4859
function reportProblem(problem) {
@@ -68,7 +79,7 @@ export default {
6879
reportProblem({
6980
node: declaredDependenciesNode,
7081
message:
71-
`React Hook ${context.getSource(reactiveHook)} was passed a ` +
82+
`React Hook ${reactiveHookName} was passed a ` +
7283
'dependency list that is not an array literal. This means we ' +
7384
"can't statically verify whether you've passed the correct " +
7485
'dependencies.',
@@ -79,9 +90,25 @@ export default {
7990
if (declaredDependencyNode === null) {
8091
return;
8192
}
93+
if (declaredDependencyNode.type === 'Identifier' && options && options.avoidObjects) {
94+
// If we see an object then add a special warning if avoidObjects option is true.
8295
reportProblem({
8396
node: declaredDependencyNode,
8497
message:
98+
`React Hook ${reactiveHookName} has an object in its ` +
99+
`dependency array: '${declaredDependencyNode.name}'. ` +
100+
// `Non-primitive dependencies can result in triggering the ${reactiveHookName} unnecessarily ` +
101+
"Non-primitive dependencies can result in triggering " +
102+
"the callback unnecessarily due to referential equality " + // via Object.is()
103+
"comparison. Consider destructuring the object outside " +
104+
`the ${reactiveHookName} call or using property accessors ` +
105+
"to refer to primitive values within the dependency array.",
106+
// `destructure the object outside of the ${reactiveHookName} call and ` +
107+
// "refer to those specific values inside ${ context.getSource(reactiveHook) }.`;
108+
//
109+
// "equality. Either use a property accessor to extract the primitive value " +
110+
// `or convert the ${context.getSource(reactiveHook)} to a ${context.getSource(reactiveHook).replace('use', 'useDeepCompare')}.`,
111+
// ^!! Don't recommend useDeepCompare per Dan Abramov: https://twitter.com/dan_abramov/status/1104414469629898754 !!
85112
});
86113
return;
87114
}
@@ -105,9 +132,8 @@ export default {
105132
const isEffect = /Effect($|[^a-z])/g.test(reactiveHookName);
106133

107134
// Check the declared dependencies for this reactive hook. If there is no
108-
// second argument then the reactive callback will re-run on every render.
109-
// So no need to check for dependency inclusion.
110-
if (!declaredDependenciesNode && !isEffect) {
135+
// second argument then there are no declared dependencies.
136+
if (!declaredDependenciesNode) return;
111137

112138
switch (callback.type) {
113139
case 'FunctionExpression':

src/index.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ export const configs = {
1313
recommended: {
1414
plugins: ['react-hooks-unreliable-deps'],
1515
rules: {
16-
'react-hooks-unreliable-deps/reference-deps': 'warn',
16+
'react-hooks-unreliable-deps/reference-deps': ['warn', {
17+
'avoidObjects': true
18+
}]
1719
},
1820
},
1921
};

0 commit comments

Comments
 (0)