Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[eslint-plugin-react-hooks] Disallow hooks in class components #18341

Merged
merged 1 commit into from
Mar 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -232,45 +232,6 @@ const tests = {
}
}
`,
`
// Currently valid because we found this to be a common pattern
// for feature flag checks in existing components.
// We *could* make it invalid but that produces quite a few false positives.
// Why does it make sense to ignore it? Firstly, because using
// hooks in a class would cause a runtime error anyway.
// But why don't we care about the same kind of false positive in a functional
// component? Because even if it was a false positive, it would be confusing
// anyway. So it might make sense to rename a feature flag check in that case.
class ClassComponentWithFeatureFlag extends React.Component {
render() {
if (foo) {
useFeatureFlag();
}
}
}
`,
`
// Currently valid because we don't check for hooks in classes.
// See ClassComponentWithFeatureFlag for rationale.
// We *could* make it invalid if we don't regress that false positive.
class ClassComponentWithHook extends React.Component {
render() {
React.useState();
}
}
`,
`
// Currently valid.
// These are variations capturing the current heuristic--
// we only allow hooks in PascalCase, useFoo functions,
// or classes (due to common false positives and because they error anyway).
// We *could* make some of these invalid.
// They probably don't matter much.
(class {useHook = () => { useState(); }});
(class {useHook() { useState(); }});
(class {h = () => { useState(); }});
(class {i() { useState(); }});
`,
`
// Valid because they're not matching use[A-Z].
fooState();
Expand Down Expand Up @@ -870,6 +831,52 @@ const tests = {
`,
errors: [topLevelError('useBasename')],
},
{
code: `
class ClassComponentWithFeatureFlag extends React.Component {
render() {
if (foo) {
useFeatureFlag();
}
}
}
`,
errors: [classError('useFeatureFlag')],
},
{
code: `
class ClassComponentWithHook extends React.Component {
render() {
React.useState();
}
}
`,
errors: [classError('React.useState')],
},
{
code: `
(class {useHook = () => { useState(); }});
`,
errors: [classError('useState')],
},
{
code: `
(class {useHook() { useState(); }});
`,
errors: [classError('useState')],
},
{
code: `
(class {h = () => { useState(); }});
`,
errors: [classError('useState')],
},
{
code: `
(class {i() { useState(); }});
`,
errors: [classError('useState')],
},
],
};

Expand Down Expand Up @@ -919,6 +926,15 @@ function topLevelError(hook) {
};
}

function classError(hook) {
return {
message:
`React Hook "${hook}" cannot be called in a class component. React Hooks ` +
'must be called in a React function component or a custom React ' +
'Hook function.',
};
}

// For easier local testing
if (!process.env.CI) {
let only = [];
Expand Down
11 changes: 6 additions & 5 deletions packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -461,11 +461,12 @@ export default {
codePathNode.parent.type === 'ClassProperty') &&
codePathNode.parent.value === codePathNode
) {
// Ignore class methods for now because they produce too many
// false positives due to feature flag checks. We're less
// sensitive to them in classes because hooks would produce
// runtime errors in classes anyway, and because a use*()
// call in a class, if it works, is unambiguously *not* a hook.
// Custom message for hooks inside a class
const message =
`React Hook "${context.getSource(hook)}" cannot be called ` +
'in a class component. React Hooks must be called in a ' +
'React function component or a custom React Hook function.';
context.report({node: hook, message});
} else if (codePathFunctionName) {
// Custom message if we found an invalid function name.
const message =
Expand Down