Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Bug]: Prop-types rules ignored for lowercase functional components #3806

Open
2 tasks done
burnhamrobertp opened this issue Aug 22, 2024 · 7 comments
Open
2 tasks done
Labels

Comments

@burnhamrobertp
Copy link

burnhamrobertp commented Aug 22, 2024

Is there an existing issue for this?

  • I have searched the existing issues and my issue is unique
  • My issue appears in the command-line and not only in the text editor

Description Overview

Functional components that start with a lower-case letter (const someComponent = ...) are ignored when checking the prop-types rule. This behavior is not observed with similarly named class components.

Several of our codebases lean heavily into React's HoC pattern, resulting in component files that look like:

const someComponent = ({ userId }) => {
 // some code
}

someComponent.propTypes = {
  userId: PropTypes.string.isRequired,
}

const SomeComponent = compose(
  withGlobalContext(['userId']),
  React.memo,
)(someComponent);

export default SomeComponent

This worked for years on a much older version of the plugin/eslint, but has since stopped reporting errors on new functional components that follow this pattern. In the above example, changing someComponent to Test (or anything that starts with a capital letter) fixes the propTypes linting.

Expected Behavior

prop-types rule is enforced even for lowercase functional components; even if it requires a rule configuration to opt into.

eslint-plugin-react version

v7.35.0

eslint version

v8.57.0

node version

v18.20.3

@ljharb
Copy link
Member

ljharb commented Aug 22, 2024

A component, by definition, MUST have a capital letter as the beginning of its name, including with the HoC pattern. I think you just need to stick to that.

@burnhamrobertp
Copy link
Author

burnhamrobertp commented Aug 22, 2024

The usage of a component MUST have a capital letter, but not the definition. Moreover, even if this were strictly true it wouldn't explain the discrepancy in the behavior of the plugin between class and functional components.

There is nothing in React that precludes (as bad a pattern as it may be)

const someComponent = () => {
  // ...
}

export default someComponent
import SomeComponent from './some-component';

...

<SomeComponent />

@ljharb
Copy link
Member

ljharb commented Aug 22, 2024

You're entirely correct, but given the limits of static analysis, we've always enforced the universal practice that everyone follows. For classes, we have extends React.Component to help; SFCs have no such syntactic marker.

In my HOC experience, i'd name them SomeComponentInner and SomeComponent, for example.

@burnhamrobertp
Copy link
Author

burnhamrobertp commented Aug 22, 2024

You're entirely correct, but given the limits of static analysis, we've always enforced the universal practice that everyone follows. For classes, we have extends React.Component to help; SFCs have no such syntactic marker.

In my HOC experience, i'd name them SomeComponentInner and SomeComponent, for example.

We did this for quite some time; its an awful convention. It also doesn't do anything to address the plugin being internally inconsistent in its behavior between class components and functional components based on casing (an arbitrary way to determine behavior, regardless of React's conventions). Of course, this entire exchange has also failed to provide any explanation as to why the plugin doesn't actually work in the way the documentation specifies it should.

In other words: if this behavior is intended, why is it necessary? Removing this restriction for the enforcement of prop-types warnings/errors during linting wouldn't negatively affect any other user.

@ljharb
Copy link
Member

ljharb commented Aug 22, 2024

Fair point. It's necessary because without this distinction, we get too many false positives on non-component functions - making the entire plugin not useful.

@burnhamrobertp
Copy link
Author

Would it be considered as an opt-in behavior available via a new option on the prop-types rule, or new value for an existing option?

@ljharb
Copy link
Member

ljharb commented Aug 30, 2024

Sorry if I'm not being clear. Without this heuristic, you'll get react-related warnings on thousands of non-component functions in your codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants