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
React/declarations for function components #2414
React/declarations for function components #2414
Conversation
…pe for function components Fixes jsx-eslint#690.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is a great start!
It'd be really nice if this could be autofixable. Things to watch out for in the fixers include:
- any usage of
this
directly inside the component, or inside an arrow function (ie, not wrapped by another normal function), should prevent the autofix - if the component is an arrow function, and has an inferred name, that should be the name of the function expression. if it doesn't, then it should become an anonymous function expression
- if the component is a function declaration, it should not be autofixed into an arrow expression
- if the component is a function expression, the explicit name should match whatever the inferred name of the arrow function will eventually be - or else it should not run an autofix.
Does autofix take typescript into account at all? I imagine autofixing a functional component that is defined like so:
would lead to unintentional side effects. Should eslint-plugin-react care about this? @ljharb |
That’s a fair point; if the types can be fixed, great, if not, best to skip auto fixing entirely when there’s type annotations. |
I will start writing the documentation once I have some certainty that it now works the way you are okay with @ljharb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks great. Let's get that documentation written.
@Stefanwullems tests are failing. |
50ed041
to
6f07af0
Compare
@ljharb If everything is allright, let's get this merged?? |
@Stefanwullems this needs to be rebased on latest master, ideally squashed down to one commit, and all conflict markers removed (look for |
@ljharb I have no idea how to squash so it would be perfect if you or someone else can take is off my hands before I explode something. |
I’m talking about git rebase; i can certainly try to do it for you but you have more context on the change than i do. |
@ljharb I think I rebased it but I have no idea if I did it right. |
ce1d96a
to
d7423f1
Compare
@ljharb I think it's ready |
6151282
to
cf1cf2e
Compare
cf1cf2e
to
b8151af
Compare
b8151af
to
99cea84
Compare
This is just draft of how I would like it to work. Any feedback would be much appreciated.
This pull request was inspired by #690
Tasks: