[eslint-plugin-react-hooks] Disallow hooks in class components (#18341)
Per discussion at Facebook, we think hooks have reached a tipping point where it is more valuable to lint against potential hooks in classes than to worry about false positives. Test plan: ``` # run from repo root yarn test --watch RuleOfHooks ```
Ian Obermiller committed
Mar 18, 2020 at 12:55 UTC
d3368beeecb2b51f51814f95f977e8c11900f54e
2 files changed
+61
-44
packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
+55
-39
@@ -232,45 +232,6 @@ const tests = {
232
}
233
}
234
`,
235
- `
236
- // Currently valid because we found this to be a common pattern
237
- // for feature flag checks in existing components.
238
- // We *could* make it invalid but that produces quite a few false positives.
239
- // Why does it make sense to ignore it? Firstly, because using
240
- // hooks in a class would cause a runtime error anyway.
241
- // But why don't we care about the same kind of false positive in a functional
242
- // component? Because even if it was a false positive, it would be confusing
243
- // anyway. So it might make sense to rename a feature flag check in that case.
244
- class ClassComponentWithFeatureFlag extends React.Component {
245
- render() {
246
- if (foo) {
247
- useFeatureFlag();
248
- }
249
- }
250
- }
251
- `,
252
- `
253
- // Currently valid because we don't check for hooks in classes.
254
- // See ClassComponentWithFeatureFlag for rationale.
255
- // We *could* make it invalid if we don't regress that false positive.
256
- class ClassComponentWithHook extends React.Component {
257
- render() {
258
- React.useState();
259
- }
260
- }
261
- `,
262
- `
263
- // Currently valid.
264
- // These are variations capturing the current heuristic--
265
- // we only allow hooks in PascalCase, useFoo functions,
266
- // or classes (due to common false positives and because they error anyway).
267
- // We *could* make some of these invalid.
268
- // They probably don't matter much.
269
- (class {useHook = () => { useState(); }});
270
- (class {useHook() { useState(); }});
271
- (class {h = () => { useState(); }});
272
- (class {i() { useState(); }});
273
- `,
235
`
236
// Valid because they're not matching use[A-Z].
237
fooState();
@@ -870,6 +831,52 @@ const tests = {
831
`,
832
errors: [topLevelError('useBasename')],
833
},
834
+ {
835
+ code: `
836
+ class ClassComponentWithFeatureFlag extends React.Component {
837
+ render() {
838
+ if (foo) {
839
+ useFeatureFlag();
840
+ }
841
+ }
842
+ }
843
+ `,
844
+ errors: [classError('useFeatureFlag')],
845
+ },
846
+ {
847
+ code: `
848
+ class ClassComponentWithHook extends React.Component {
849
+ render() {
850
+ React.useState();
851
+ }
852
+ }
853
+ `,
854
+ errors: [classError('React.useState')],
855
+ },
856
+ {
857
+ code: `
858
+ (class {useHook = () => { useState(); }});
859
+ `,
860
+ errors: [classError('useState')],
861
+ },
862
+ {
863
+ code: `
864
+ (class {useHook() { useState(); }});
865
+ `,
866
+ errors: [classError('useState')],
867
+ },
868
+ {
869
+ code: `
870
+ (class {h = () => { useState(); }});
871
+ `,
872
+ errors: [classError('useState')],
873
+ },
874
+ {
875
+ code: `
876
+ (class {i() { useState(); }});
877
+ `,
878
+ errors: [classError('useState')],
879
+ },
880
],
881
};
882
@@ -919,6 +926,15 @@ function topLevelError(hook) {
926
};
927
}
928
929
+function classError(hook) {
930
+ return {
931
+ message:
932
+ `React Hook "${hook}" cannot be called in a class component. React Hooks ` +
933
+ 'must be called in a React function component or a custom React ' +
934
+ 'Hook function.',
935
+ };
936
+}
937
+
938
// For easier local testing
939
if (!process.env.CI) {
940
let only = [];
packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
+6
-5
@@ -461,11 +461,12 @@ export default {
461
codePathNode.parent.type === 'ClassProperty') &&
462
codePathNode.parent.value === codePathNode
463
) {
464
- // Ignore class methods for now because they produce too many
465
- // false positives due to feature flag checks. We're less
466
- // sensitive to them in classes because hooks would produce
467
- // runtime errors in classes anyway, and because a use*()
468
- // call in a class, if it works, is unambiguously *not* a hook.
464
+ // Custom message for hooks inside a class
465
+ const message =
466
+ `React Hook "${context.getSource(hook)}" cannot be called ` +
467
+ 'in a class component. React Hooks must be called in a ' +
468
+ 'React function component or a custom React Hook function.';
469
+ context.report({node: hook, message});
470
} else if (codePathFunctionName) {
471
// Custom message if we found an invalid function name.
472
const message =