[eslint-plugin-react-hooks] only allow capitalized component names #25162

kassens · 2022-08-31T16:59:13Z

Summary

A name like _useFoo was previously recognized as a component name ("starting with non-lowercase"). This would misleadingly identify this code as valid (see the first commit in this PR):

function Component(props) {
  if (cond) {
    _useHook();
  }
}
function _useHook() {
  useState(null);
}

The main change in this PR is that components can no longer start with 1 or more underscores followed by a capital letter. _component, $Component, _useFoo are no longer valid component names.

This means the above code would now be invalid because of the useState call in the non-hook/non-component function _useHook.

How did you test this change?

Updated unit tests.

kassens · 2022-08-31T17:02:26Z

packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js

@@ -432,6 +431,7 @@ const tests = {
      `,
      errors: [
        topLevelError('Hook.useState'),
+        topLevelError('Hook._useState'),


We could disallow this by only allowing the leading underscore for top level identifiers, but figured it might be nice to recognize Hook.__useSemiPrivateHook?

gaearon · 2022-08-31T17:47:09Z

Why not take a stronger stance and remove all support for the _ prefix?

gaearon

I think we should forbid the _ prefix completely instead. We never suggested the _Foo pattern in documentation, the only reason it was supported is because some www code used it, and we didn't want to touch that code. I suggest that we fix this forward on www and remove support for the pattern completely.

kassens · 2022-08-31T17:57:17Z

Sounds good to me!

gaearon

We need to remember not to do a release without tagging this as a major.

acdlite · 2022-09-01T00:41:42Z

We need to remember not to do a release without tagging this as a major.

You can update the version in ReactVersions:

react/ReactVersions.js

Line 28 in 8d1b057

'eslint-plugin-react-hooks': '4.7.0',

That will bump the version in the @next channel, too.

…ming

sizebot · 2022-09-01T14:10:42Z

Comparing: 36c908a...bd5763e

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	134.97 kB	134.97 kB	=	43.23 kB	43.23 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	141.64 kB	141.64 kB	=	45.20 kB	45.20 kB
facebook-www/ReactDOM-prod.classic.js	=	486.06 kB	486.06 kB	=	86.51 kB	86.51 kB
facebook-www/ReactDOM-prod.modern.js	=	471.34 kB	471.34 kB	=	84.29 kB	84.29 kB
facebook-www/ReactDOMForked-prod.classic.js	=	486.06 kB	486.06 kB	=	86.51 kB	86.51 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against bd5763e

Summary: This sync includes the following changes: - **[c28f313e6](facebook/react@c28f313e6 )**: experimental_use(promise) for SSR ([#25214](facebook/react#25214)) //<Andrew Clark>// - **[d6f9628a8](facebook/react@d6f9628a8 )**: Remove some RSC subset entry points that were removed in the main entry point ([#25209](facebook/react#25209)) //<Sebastian Markbåge>// - **[a473d08fc](facebook/react@a473d08fc )**: Update to Flow from 0.97 to 0.122 ([#25204](facebook/react#25204)) //<Jan Kassens>// - **[7028ce745](facebook/react@7028ce745 )**: experimental_use(promise) for Server Components ([#25207](facebook/react#25207)) //<Andrew Clark>// - **[bfb65681e](facebook/react@bfb65681e )**: experimental_use(context)([#25202](facebook/react#25202)) //<mofeiZ>// - **[f0efa1164](facebook/react@f0efa1164 )**: [flow] remove custom suppress comment config ([#25170](facebook/react#25170)) //<Jan Kassens>// - **[2e7f422fe](facebook/react@2e7f422fe )**: Refactor: its type is Container ([#25153](facebook/react#25153)) //<bubucuo>// - **[2c2d9a1df](facebook/react@2c2d9a1df )**: [eslint-plugin-react-hooks] only allow capitalized component names ([#25162](facebook/react#25162)) //<Jan Kassens>// - **[36c908a6c](facebook/react@36c908a6c )**: Don't use the Flight terminology in public error messages ([#25166](facebook/react#25166)) //<Sebastian Markbåge>// - **[8d1b057ec](facebook/react@8d1b057ec )**: [Flight] Minor error handling fixes ([#25151](facebook/react#25151)) //<Sebastian Markbåge>// - **[9ff738f53](facebook/react@9ff738f53 )**: [devtools][easy] Fix flow type ([#25147](facebook/react#25147)) //<Tianyu Yao>// - **[0de3ddf56](facebook/react@0de3ddf56 )**: Remove Symbol Polyfill (again) ([#25144](facebook/react#25144)) //<Ricky>// - **[b36f72235](facebook/react@b36f72235 )**: Remove ReactFiberFlags MountLayoutDev and MountPassiveDev ([#25091](facebook/react#25091)) //<Samuel Susla>// - **[b6978bc38](facebook/react@b6978bc38 )**: experimental_use(promise) ([#25084](facebook/react#25084)) //<Andrew Clark>// - **[11ed7010c](facebook/react@11ed7010c )**: [Transition Tracing] onMarkerIncomplete - Tracing Marker/Suspense Boundary Deletions ([#24885](facebook/react#24885)) //<Luna Ruan>// - **[b79894259](facebook/react@b79894259 )**: [Flight] Add support for Webpack Async Modules ([#25138](facebook/react#25138)) //<Sebastian Markbåge>// - **[c8b778b7f](facebook/react@c8b778b7f )**: Fix typo: supportsMicrotask -> supportsMicrotasks ([#25142](facebook/react#25142)) //<kwzr>// - **[d0f396651](facebook/react@d0f396651 )**: Allow functions to be used as module references ([#25137](facebook/react#25137)) //<Sebastian Markbåge>// - **[38c5d8a03](facebook/react@38c5d8a03 )**: Test the node-register hooks in unit tests ([#25132](facebook/react#25132)) //<Sebastian Markbåge>// - **[3f70e68ce](facebook/react@3f70e68ce )**: Return closestInstance in `getInspectorDataForViewAtPoint` ([#25118](facebook/react#25118)) //<Tianyu Yao>// - **[3d443cad7](facebook/react@3d443cad7 )**: Update fixtures/flight to webpack 5 ([#25115](facebook/react#25115)) //<Tim Neutkens>// - **[5d1ce6513](facebook/react@5d1ce6513 )**: Align StrictMode behaviour with production ([#25049](facebook/react#25049)) //<Samuel Susla>// - **[9e67e7a31](facebook/react@9e67e7a31 )**: Scaffolding for useMemoCache hook ([#25123](facebook/react#25123)) //<Joseph Savona>// - **[19e9a4c68](facebook/react@19e9a4c68 )**: Add missing createServerContext for experimental shared subset ([#25114](facebook/react#25114)) //<Jiachi Liu>// - **[6ef466c68](facebook/react@6ef466c68 )**: make preamble and postamble types explicit and fix typo ([#25102](facebook/react#25102)) //<Josh Story>// - **[796d31809](facebook/react@796d31809 )**: Implement basic stylesheet Resources for react-dom ([#25060](facebook/react#25060)) //<Josh Story>// - **[32baab38f](facebook/react@32baab38f )**: [Transition Tracing] Add Tag Field to Marker Instance ([#25085](facebook/react#25085)) //<Luna Ruan>// - **[8ef3a7c08](facebook/react@8ef3a7c08 )**: Resume immediately pinged fiber without unwinding ([#25074](facebook/react#25074)) //<Andrew Clark>// - **[7bcc68772](facebook/react@7bcc68772 )**: Remove argument committedLanes from reappearLayoutEffects and recursivelyTraverseReappearLayoutEffects ([#25080](facebook/react#25080)) //<Samuel Susla>// - **[ca990e9a7](facebook/react@ca990e9a7 )**: Add API to force Scheduler to yield for macrotask ([#25044](facebook/react#25044)) //<Andrew Clark>// - **[b4204ede6](facebook/react@b4204ede6 )**: Clean up unused Deletion flag ([#24992](facebook/react#24992)) //<Andrew Clark>// - **[e193be87e](facebook/react@e193be87e )**: [Transition Tracing] Add Offscreen Test ([#25035](facebook/react#25035)) //<Luna Ruan>// - **[9fcaf88d5](facebook/react@9fcaf88d5 )**: Remove rootContainerInstance from unnecessary places ([#25024](facebook/react#25024)) //<Sebastian Markbåge>// - **[80f3d8819](facebook/react@80f3d8819 )**: Mount/unmount passive effects when Offscreen visibility changes ([#24977](facebook/react#24977)) //<Andrew Clark>// Changelog: [General][Changed] - React Native sync for revisions 4ea064e...c28f313 Reviewed By: rickhanlonii Differential Revision: D39384898 fbshipit-source-id: 20b080a53851d6dd9d522c8468dd02aab9ba76db

…25162) - update naming rules to disallow _component - update eslint-plugin-react-hooks version

Summary: This sync includes the following changes: - **[c28f313e6](facebook/react@c28f313e6 )**: experimental_use(promise) for SSR ([facebook#25214](facebook/react#25214)) //<Andrew Clark>// - **[d6f9628a8](facebook/react@d6f9628a8 )**: Remove some RSC subset entry points that were removed in the main entry point ([facebook#25209](facebook/react#25209)) //<Sebastian Markbåge>// - **[a473d08fc](facebook/react@a473d08fc )**: Update to Flow from 0.97 to 0.122 ([facebook#25204](facebook/react#25204)) //<Jan Kassens>// - **[7028ce745](facebook/react@7028ce745 )**: experimental_use(promise) for Server Components ([facebook#25207](facebook/react#25207)) //<Andrew Clark>// - **[bfb65681e](facebook/react@bfb65681e )**: experimental_use(context)([facebook#25202](facebook/react#25202)) //<mofeiZ>// - **[f0efa1164](facebook/react@f0efa1164 )**: [flow] remove custom suppress comment config ([facebook#25170](facebook/react#25170)) //<Jan Kassens>// - **[2e7f422fe](facebook/react@2e7f422fe )**: Refactor: its type is Container ([facebook#25153](facebook/react#25153)) //<bubucuo>// - **[2c2d9a1df](facebook/react@2c2d9a1df )**: [eslint-plugin-react-hooks] only allow capitalized component names ([facebook#25162](facebook/react#25162)) //<Jan Kassens>// - **[36c908a6c](facebook/react@36c908a6c )**: Don't use the Flight terminology in public error messages ([facebook#25166](facebook/react#25166)) //<Sebastian Markbåge>// - **[8d1b057ec](facebook/react@8d1b057ec )**: [Flight] Minor error handling fixes ([facebook#25151](facebook/react#25151)) //<Sebastian Markbåge>// - **[9ff738f53](facebook/react@9ff738f53 )**: [devtools][easy] Fix flow type ([facebook#25147](facebook/react#25147)) //<Tianyu Yao>// - **[0de3ddf56](facebook/react@0de3ddf56 )**: Remove Symbol Polyfill (again) ([facebook#25144](facebook/react#25144)) //<Ricky>// - **[b36f72235](facebook/react@b36f72235 )**: Remove ReactFiberFlags MountLayoutDev and MountPassiveDev ([facebook#25091](facebook/react#25091)) //<Samuel Susla>// - **[b6978bc38](facebook/react@b6978bc38 )**: experimental_use(promise) ([facebook#25084](facebook/react#25084)) //<Andrew Clark>// - **[11ed7010c](facebook/react@11ed7010c )**: [Transition Tracing] onMarkerIncomplete - Tracing Marker/Suspense Boundary Deletions ([facebook#24885](facebook/react#24885)) //<Luna Ruan>// - **[b79894259](facebook/react@b79894259 )**: [Flight] Add support for Webpack Async Modules ([facebook#25138](facebook/react#25138)) //<Sebastian Markbåge>// - **[c8b778b7f](facebook/react@c8b778b7f )**: Fix typo: supportsMicrotask -> supportsMicrotasks ([facebook#25142](facebook/react#25142)) //<kwzr>// - **[d0f396651](facebook/react@d0f396651 )**: Allow functions to be used as module references ([facebook#25137](facebook/react#25137)) //<Sebastian Markbåge>// - **[38c5d8a03](facebook/react@38c5d8a03 )**: Test the node-register hooks in unit tests ([facebook#25132](facebook/react#25132)) //<Sebastian Markbåge>// - **[3f70e68ce](facebook/react@3f70e68ce )**: Return closestInstance in `getInspectorDataForViewAtPoint` ([facebook#25118](facebook/react#25118)) //<Tianyu Yao>// - **[3d443cad7](facebook/react@3d443cad7 )**: Update fixtures/flight to webpack 5 ([facebook#25115](facebook/react#25115)) //<Tim Neutkens>// - **[5d1ce6513](facebook/react@5d1ce6513 )**: Align StrictMode behaviour with production ([facebook#25049](facebook/react#25049)) //<Samuel Susla>// - **[9e67e7a31](facebook/react@9e67e7a31 )**: Scaffolding for useMemoCache hook ([facebook#25123](facebook/react#25123)) //<Joseph Savona>// - **[19e9a4c68](facebook/react@19e9a4c68 )**: Add missing createServerContext for experimental shared subset ([facebook#25114](facebook/react#25114)) //<Jiachi Liu>// - **[6ef466c68](facebook/react@6ef466c68 )**: make preamble and postamble types explicit and fix typo ([facebook#25102](facebook/react#25102)) //<Josh Story>// - **[796d31809](facebook/react@796d31809 )**: Implement basic stylesheet Resources for react-dom ([facebook#25060](facebook/react#25060)) //<Josh Story>// - **[32baab38f](facebook/react@32baab38f )**: [Transition Tracing] Add Tag Field to Marker Instance ([facebook#25085](facebook/react#25085)) //<Luna Ruan>// - **[8ef3a7c08](facebook/react@8ef3a7c08 )**: Resume immediately pinged fiber without unwinding ([facebook#25074](facebook/react#25074)) //<Andrew Clark>// - **[7bcc68772](facebook/react@7bcc68772 )**: Remove argument committedLanes from reappearLayoutEffects and recursivelyTraverseReappearLayoutEffects ([facebook#25080](facebook/react#25080)) //<Samuel Susla>// - **[ca990e9a7](facebook/react@ca990e9a7 )**: Add API to force Scheduler to yield for macrotask ([facebook#25044](facebook/react#25044)) //<Andrew Clark>// - **[b4204ede6](facebook/react@b4204ede6 )**: Clean up unused Deletion flag ([facebook#24992](facebook/react#24992)) //<Andrew Clark>// - **[e193be87e](facebook/react@e193be87e )**: [Transition Tracing] Add Offscreen Test ([facebook#25035](facebook/react#25035)) //<Luna Ruan>// - **[9fcaf88d5](facebook/react@9fcaf88d5 )**: Remove rootContainerInstance from unnecessary places ([facebook#25024](facebook/react#25024)) //<Sebastian Markbåge>// - **[80f3d8819](facebook/react@80f3d8819 )**: Mount/unmount passive effects when Offscreen visibility changes ([facebook#24977](facebook/react#24977)) //<Andrew Clark>// Changelog: [General][Changed] - React Native sync for revisions 4ea064e...c28f313 Reviewed By: rickhanlonii Differential Revision: D39384898 fbshipit-source-id: 20b080a53851d6dd9d522c8468dd02aab9ba76db

benasher44 · 2024-08-14T14:19:20Z

Since 5.0 isn't out yet, would it be possible to instead tweak this to allow _ again but add an additional stylistic rule to disallow the _ and provide a nice warning that says why it's discouraged? For us, this rule created a barrier to upgrading, such that we decided to just patch the plugin to allow _ again, while we deliberated how to fix our 100+ components that were using _.

IMO, the goal of this plugin is to make it easy for developers to catch bugs with hook usage, whereas this tweak inserts what appears to be a stylistic choice. Sure, our team will grow from this and pick better component names going forward, but if there is something truly wrong with _, why do this here and not ban it in react 19, an upcoming major version where the opportunity to truly ban it is there? If banning it in a react major version isn't palatable, then it seems to me the choice is truly a stylistic one and should not be put in the path of people getting on the latest eslint-plugin-react, whose main focus (IMO) really should be about preventing bugs.

Edit: also if it's not purely a style thing, then it seems a new rule would be great anyway, so people can adopt it, rather than it be a side-effect of this rule.

See facebook/react#25162

@kassens

##### [v5.0.0](https://github.com/facebook/react/blob/HEAD/packages/eslint-plugin-react-hooks/CHANGELOG.md#500) - **New Violations:** Component names now need to start with an uppercase letter instead of a non-lowercase letter. This means `_Button` or `_component` are no longer valid. ([@kassens](https://github.com/kassens)) in [#25162](facebook/react#25162)  - Consider dispatch from `useActionState` stable. ([@eps1lon](https://github.com/eps1lon) in [#29665](facebook/react#29665)) - Add support for ESLint v9. ([@eps1lon](https://github.com/eps1lon) in [#28773](facebook/react#28773)) - Accept `as` expression in callback. ([@StyleShit](https://github.com/StyleShit) in [#28202](facebook/react#28202)) - Accept `as` expressions in deps array. ([@StyleShit](https://github.com/StyleShit) in [#28189](facebook/react#28189)) - Treat `React.use()` the same as `use()`. ([@kassens](https://github.com/kassens) in [#27769](facebook/react#27769)) - Move `use()` lint to non-experimental. ([@kassens](https://github.com/kassens) in [#27768](facebook/react#27768)) - Support Flow `as` expressions. ([@cpojer](https://github.com/cpojer) in [#27590](facebook/react#27590)) - Allow `useEffect(fn, undefined)`. ([@kassens](https://github.com/kassens) in [#27525](facebook/react#27525)) - Disallow hooks in async functions. ([@acdlite](https://github.com/acdlite) in [#27045](facebook/react#27045)) - Rename experimental `useEvent` to `useEffectEvent`. ([@sebmarkbage](https://github.com/sebmarkbage) in [#25881](facebook/react#25881)) - Lint for presence of `useEvent` functions in dependency lists. ([@poteto](https://github.com/poteto) in [#25512](facebook/react#25512)) - Check `useEvent` references instead. ([@poteto](https://github.com/poteto) in [#25319](facebook/react#25319)) - Update `RulesOfHooks` with `useEvent` rules. ([@poteto](https://github.com/poteto) in [#25285](facebook/react#25285))

See facebook/react#25162

SethDavenport · 2024-11-29T15:35:57Z

For what it's worth, I agree with @benasher44.

Our codebase has 916 places where component names are prefixed by _ and this presents a barrier to upgrade for something that feels stylistic.

At one point this was a fairly common pattern in many codebases, not just ours:

export function MyComponent() {
  // logic
  ...
  
  // render
  return <_MyComponent .. />;
}

The fact that this was not explicitly suggested in the React documentation doesn't seem terribly relevant if I'm honest.

It's not a pattern we pursue much any more, but remediating 2MM lines of legacy code is not something I'm excited about, especially for a choice that doesn't seem to have much behind it except aesthetics.

Would you consider configuration for this rule to opt into _ + capital letter, even if it's not the default?

depressedX · 2024-12-19T09:56:09Z

_Component case is useful when using HOC pattern or when refactoring code.

function _PageComposer() {
}

export const PageComposer = withProviders(
  withProps(RecordProvider, { vendor: "composer" })
)(_PageComposer);

Hope it could be added to default rules, or provide configuarable options at least.

kassens requested a review from gaearon August 31, 2022 16:59

facebook-github-bot added the CLA Signed label Aug 31, 2022

kassens force-pushed the hook-naming branch from 203a6a2 to 1299697 Compare August 31, 2022 17:00

kassens commented Aug 31, 2022

View reviewed changes

gaearon requested changes Aug 31, 2022

View reviewed changes

kassens added 2 commits August 31, 2022 14:50

[eslint-plugin-react-hooks] add test showing broken behavior

cfee9e6

[eslint-plugin-react-hooks] update naming rules to disallow _component

f95ad03

kassens force-pushed the hook-naming branch from 1299697 to f95ad03 Compare August 31, 2022 18:54

kassens changed the title ~~Hook naming~~ [eslint-plugin-react-hooks] only allow capitalized component names Aug 31, 2022

kassens requested a review from gaearon August 31, 2022 18:56

clarify changelog

2c54215

gaearon approved these changes Aug 31, 2022

View reviewed changes

update CHANGELOG to remind us the next release is a major

a29812e

kassens added 2 commits September 1, 2022 10:05

update eslint-plugin-react-hooks version

f426964

Merge branch 'main' of https://github.com/facebook/react into hook-na…

bd5763e

…ming

kassens merged commit 2c2d9a1 into facebook:main Sep 1, 2022

kassens deleted the hook-naming branch September 1, 2022 14:07

rickhanlonii pushed a commit that referenced this pull request Oct 5, 2022

[eslint-plugin-react-hooks] only allow capitalized component names (#…

7517748

…25162) - update naming rules to disallow _component - update eslint-plugin-react-hooks version

shvlv added a commit to shvlv/gutenberg that referenced this pull request Oct 9, 2024

ESLint: suppress react-hooks/rules-of-hooks for experimental components

f6a166b

See facebook/react#25162

shvlv mentioned this pull request Oct 9, 2024

Upgrade ESLint to v9 WordPress/gutenberg#65648

Draft

shvlv added a commit to shvlv/gutenberg that referenced this pull request Oct 15, 2024

ESLint: suppress react-hooks/rules-of-hooks for experimental components

f9195af

See facebook/react#25162

KenAJoh mentioned this pull request Oct 17, 2024

[Linting] Eslint v9 dependency upgrades navikt/aksel#3248

Merged

11 tasks

DavidZidar mentioned this pull request Nov 7, 2024

Bug: eslint-plugin-react-hooks@5.0.0 only detects english component names #31446

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[eslint-plugin-react-hooks] only allow capitalized component names #25162

[eslint-plugin-react-hooks] only allow capitalized component names #25162

[eslint-plugin-react-hooks] only allow capitalized component names #25162

[eslint-plugin-react-hooks] only allow capitalized component names #25162

Conversation

Summary

How did you test this change?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Critical size changes

Significant size changes