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

More clear hooks warning for why a function is not considered as a component #17736

Open
aditya81070 opened this issue Dec 29, 2019 · 13 comments
Open

Comments

@aditya81070
Copy link

@aditya81070 aditya81070 commented Dec 29, 2019

Do you want to request a feature or report a bug?
bug
What is the current behavior?
In current behaviour, I can create a functional component that starts with lowercase and I can use it by importing it with the uppercase name. But if I start using hooks in this lowercase named component, React will give error React Hook "useState" is called in function "login" which is neither a React function component or a custom React Hook function react-hooks/rules-of-hooks.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:
Reproduce bug here
What is the expected behavior?
The expected behaviour is that I should get a warning at the first point when I am creating a component which starts with lowercase and if I am using hooks into a functional component that starts with lowercase then it should give useful warning something like start component with uppercase.

I know as a developer that I should always start my component with Uppercase letter. But if by mistake someone creates a component with a lowercase letter and tries to use hooks, he will have no clue what is wrong with his code.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
"react": "^16.12.0",
"react-dom": "^16.12.0",
"react-scripts": "3.3.0"
Chrome: 79.0.3945.88 (Official Build) (64-bit)
OS: Ubuntu 18.04.3 LTS x86_64

@aditya81070
Copy link
Author

@aditya81070 aditya81070 commented Dec 29, 2019

I also found that when writing the demo in the codesandbox, it gives me a warning that I am using hooks into a normal function(because I am using the lowercase letter to start my component name) and it is wrong.

@bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Dec 29, 2019

I believe your issue is just requesting a wording change to the hooks lint rule, right? I don't think you're actually suggesting React should "enforce component name to start with Capital Letter" - just that the hooks warning should make it more clearer why it thinks your function isn't a React component?

If so, maybe you could re-word the issue title?

@aditya81070
Copy link
Author

@aditya81070 aditya81070 commented Dec 29, 2019

@bvaughn yeah, I want to get more specific warning. I will rename the issue.

@aditya81070 aditya81070 changed the title Enforce component name to start with Capital Letter More clear hooks warning for why a function is not considered as a component Dec 29, 2019
@yadejo
Copy link

@yadejo yadejo commented Dec 30, 2019

I'd like to give this a shot, if thats ok 🙂

@bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Dec 30, 2019

@yadedjo It's yours. Let us know if you change your mind so I can change the tag back to being up for grabs :)

@Nilomiranda
Copy link

@Nilomiranda Nilomiranda commented Dec 30, 2019

Oh, I forgot to mention I was working on it, sorry

@yadejo
Copy link

@yadejo yadejo commented Dec 30, 2019

@Nilomiranda No worries

@albseb511
Copy link

@albseb511 albseb511 commented Feb 5, 2020

can I work on the issue?
Its been ~30+ days.

@yadejo
Also to be clear:

  • This looks to me like I need to change the RulesOfHooks.js and the corresponding test for that.

I have made some changes already in that case. ran the test cases as well.

@bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Feb 5, 2020

@albseb511 There's an open PR for this (#17744) it just hasn't been reviewed.

@albseb511
Copy link

@albseb511 albseb511 commented Feb 5, 2020

@bvaughn
thanks. yea I think mine looks pretty much the same. :P

@stale
Copy link

@stale stale bot commented May 6, 2020

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the Resolution: Stale label May 6, 2020
@roheat
Copy link
Contributor

@roheat roheat commented May 9, 2020

@bvaughn the PR hasn't encorporated the requested changes (been over 30 days). Can I raise another updated PR for this issue?

@stale stale bot removed the Resolution: Stale label May 9, 2020
@gaearon
Copy link
Member

@gaearon gaearon commented May 9, 2020

Sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.