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

[kassens]

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]

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]

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]

Sounds good to me!

[gaearon]

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

[acdlite]

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.

[sizebot]

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

[benasher44]

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.

[SethDavenport]

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]

_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.