@samitouri / QOS-React / commits / 1960131f11

Add opt-in support for dangerous autofix (#18437)

Dan Abramov committed Mar 31, 2020 at 11:43 UTC 1960131f11196325ff47458355d25071049aaa7a
2 files changed +69 -15
packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
+28
@@ -6336,6 +6336,34 @@ const tests = {
6336 },
6337 ],
6338 },
6339 + {
6340 + code: normalizeIndent`
6341 + function MyComponent() {
6342 + const local = {};
6343 + useEffect(() => {
6344 + console.log(local);
6345 + }, []);
6346 + }
6347 + `,
6348 + // Dangerous autofix is enabled due to the option:
6349 + output: normalizeIndent`
6350 + function MyComponent() {
6351 + const local = {};
6352 + useEffect(() => {
6353 + console.log(local);
6354 + }, [local]);
6355 + }
6356 + `,
6357 + errors: [
6358 + {
6359 + message:
6360 + "React Hook useEffect has a missing dependency: 'local'. " +
6361 + 'Either include it or remove the dependency array.',
6362 + },
6363 + ],
6364 + // Keep this until major IDEs and VS Code FB ESLint plugin support Suggestions API.
6365 + options: [{enableDangerousAutofixThisMayCauseInfiniteLoops: true}],
6366 + },
6367 ],
6368 };
6369
packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
+41 -15
@@ -11,14 +11,19 @@
11
12 export default {
13 meta: {
14 + fixable: 'code',
15 schema: [
16 {
17 type: 'object',
18 additionalProperties: false,
19 + enableDangerousAutofixThisMayCauseInfiniteLoops: false,
20 properties: {
21 additionalHooks: {
22 type: 'string',
23 },
24 + enableDangerousAutofixThisMayCauseInfiniteLoops: {
25 + type: 'boolean',
26 + },
27 },
28 },
29 ],
@@ -31,7 +36,28 @@ export default {
36 context.options[0].additionalHooks
37 ? new RegExp(context.options[0].additionalHooks)
38 : undefined;
34 - const options = {additionalHooks};
39 +
40 + const enableDangerousAutofixThisMayCauseInfiniteLoops =
41 + (context.options &&
42 + context.options[0] &&
43 + context.options[0].enableDangerousAutofixThisMayCauseInfiniteLoops) ||
44 + false;
45 +
46 + const options = {
47 + additionalHooks,
48 + enableDangerousAutofixThisMayCauseInfiniteLoops,
49 + };
50 +
51 + function reportProblem(problem) {
52 + if (enableDangerousAutofixThisMayCauseInfiniteLoops) {
53 + // Used to enable legacy behavior. Dangerous.
54 + // Keep this as an option until major IDEs upgrade (including VSCode FB ESLint extension).
55 + if (Array.isArray(problem.suggest) && problem.suggest.length > 0) {
56 + problem.fix = problem.suggest[0].fix;
57 + }
58 + }
59 + context.report(problem);
60 + }
61
62 const scopeManager = context.getSourceCode().scopeManager;
63
@@ -140,7 +166,7 @@ export default {
166 break; // Unhandled
167 default:
168 // useEffect(generateEffectBody(), []);
143 - context.report({
169 + reportProblem({
170 node: reactiveHook,
171 message:
172 `React Hook ${reactiveHookName} received a function whose dependencies ` +
@@ -150,7 +176,7 @@ export default {
176 }
177
178 // Something unusual. Fall back to suggesting to add the body itself as a dep.
153 - context.report({
179 + reportProblem({
180 node: reactiveHook,
181 message:
182 `React Hook ${reactiveHookName} has a missing dependency: '${callback.name}'. ` +
@@ -190,7 +216,7 @@ export default {
216 reactiveHookName === 'useCallback'
217 ) {
218 // TODO: Can this have a suggestion?
193 - context.report({
219 + reportProblem({
220 node: reactiveHook,
221 message:
222 `React Hook ${reactiveHookName} does nothing when called with ` +
@@ -202,7 +228,7 @@ export default {
228 }
229
230 if (isEffect && node.async) {
205 - context.report({
231 + reportProblem({
232 node: node,
233 message:
234 `Effect callbacks are synchronous to prevent race conditions. ` +
@@ -557,7 +583,7 @@ export default {
583 if (foundCurrentAssignment) {
584 return;
585 }
560 - context.report({
586 + reportProblem({
587 node: dependencyNode.parent.property,
588 message:
589 `The ref value '${dependency}.current' will likely have ` +
@@ -577,7 +603,7 @@ export default {
603 return;
604 }
605 staleAssignments.add(key);
580 - context.report({
606 + reportProblem({
607 node: writeExpr,
608 message:
609 `Assignments to the '${key}' variable from inside React Hook ` +
@@ -645,7 +671,7 @@ export default {
671 externalDependencies: new Set(),
672 isEffect: true,
673 });
648 - context.report({
674 + reportProblem({
675 node: reactiveHook,
676 message:
677 `React Hook ${reactiveHookName} contains a call to '${setStateInsideEffectWithoutDeps}'. ` +
@@ -677,7 +703,7 @@ export default {
703 // If the declared dependencies are not an array expression then we
704 // can't verify that the user provided the correct dependencies. Tell
705 // the user this in an error.
680 - context.report({
706 + reportProblem({
707 node: declaredDependenciesNode,
708 message:
709 `React Hook ${context.getSource(reactiveHook)} was passed a ` +
@@ -693,7 +719,7 @@ export default {
719 }
720 // If we see a spread element then add a special warning.
721 if (declaredDependencyNode.type === 'SpreadElement') {
696 - context.report({
722 + reportProblem({
723 node: declaredDependencyNode,
724 message:
725 `React Hook ${context.getSource(reactiveHook)} has a spread ` +
@@ -712,7 +738,7 @@ export default {
738 if (/Unsupported node type/.test(error.message)) {
739 if (declaredDependencyNode.type === 'Literal') {
740 if (dependencies.has(declaredDependencyNode.value)) {
715 - context.report({
741 + reportProblem({
742 node: declaredDependencyNode,
743 message:
744 `The ${declaredDependencyNode.raw} literal is not a valid dependency ` +
@@ -720,7 +746,7 @@ export default {
746 `Did you mean to include ${declaredDependencyNode.value} in the array instead?`,
747 });
748 } else {
723 - context.report({
749 + reportProblem({
750 node: declaredDependencyNode,
751 message:
752 `The ${declaredDependencyNode.raw} literal is not a valid dependency ` +
@@ -728,7 +754,7 @@ export default {
754 });
755 }
756 } else {
731 - context.report({
757 + reportProblem({
758 node: declaredDependencyNode,
759 message:
760 `React Hook ${context.getSource(reactiveHook)} has a ` +
@@ -828,7 +854,7 @@ export default {
854 }
855 // TODO: What if the function needs to change on every render anyway?
856 // Should we suggest removing effect deps as an appropriate fix too?
831 - context.report({
857 + reportProblem({
858 // TODO: Why not report this at the dependency site?
859 node: fn.node,
860 message,
@@ -1099,7 +1125,7 @@ export default {
1125 }
1126 }
1127
1102 - context.report({
1128 + reportProblem({
1129 node: declaredDependenciesNode,
1130 message:
1131 `React Hook ${context.getSource(reactiveHook)} has ` +