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

[Fix] prop-types: handle anonymous functions #2730

Merged
merged 1 commit into from Jul 28, 2020
Merged

[Fix] prop-types: handle anonymous functions #2730

merged 1 commit into from Jul 28, 2020

Conversation

odinho
Copy link
Contributor

@odinho odinho commented Jul 27, 2020

Fixes #2728.

Fixes #2728.

Co-authored-by: Odin Hørthe Omdal <odin.omdal@gmail.com>
Co-authored-by: Dmitriy Lazarev <w@kich.dev>
Co-authored-by: Jordan Harband <ljharb@gmail.com>
Co-authored-by: Johnny Zabala <jzabala.s@gmail.com>
@sluukkonen
Copy link

I'm no expert in eslint, but is the check actually correct? I think an anonymous function could also be considered to be a stateless component.

I was thinking of something like

      if (
        node.type === 'FunctionDeclaration'
        && (node.id == null || isFirstLetterCapitalized(node.id.name))
        && utils.isReturningJSXOrNull(node)
      ) {

@odinho
Copy link
Contributor Author

odinho commented Jul 27, 2020

I'm no expert in eslint, but is the check actually correct? I think an anonymous function could also be considered to be a stateless component.

I was thinking of something like

      if (
        node.type === 'FunctionDeclaration'
        && (node.id == null || isFirstLetterCapitalized(node.id.name))
        && utils.isReturningJSXOrNull(node)
      ) {

As I read it, the code underneath will handle that case. It checks the variable that the anonymous function is being stored into.

@ljharb
Copy link
Member

ljharb commented Jul 28, 2020

Thanks! This does need a regression test first.

@ljharb
Copy link
Member

ljharb commented Jul 28, 2020

@odinho Since you made this PR from a fork that you don't own (whereby instead of odinho) I can't merge this yet. Can you please add me to https://github.com/whereby/eslint-plugin-react so I can force push to the PR branch?

@ljharb ljharb changed the title [Fix]: handle anonymous functions [Fix] prop-types: handle anonymous functions Jul 28, 2020
@odinho
Copy link
Contributor Author

odinho commented Jul 28, 2020

@odinho Since you made this PR from a fork that you don't own (whereby instead of odinho) I can't merge this yet. Can you please add me to https://github.com/whereby/eslint-plugin-react so I can force push to the PR branch?

Oup. Why do I keep forgetting that this restriction exists. I've added you now 🤦

@jzabala
Copy link
Contributor

jzabala commented Jul 28, 2020

This indeed will solve #2728 but then components defined this way won't be recognized:

export default function () {
  return <h1>Hello World</div>
}

I think a better check would be:

if (
        node.type === 'FunctionDeclaration'
        && (!node.id || isFirstLetterCapitalized(node.id.name)) // skip first letter check if the node.id doesn't exists
        && utils.isReturningJSXOrNull(node)
) {

@ljharb
Copy link
Member

ljharb commented Jul 28, 2020

@jzabala thanks, i've added a test case and your suggested fix.

@ljharb ljharb merged commit e8d2ce9 into jsx-eslint:master Jul 28, 2020
@ljharb ljharb deleted the odin/fix_typeerror_crash branch July 28, 2020 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: Cannot read property 'name' of null
4 participants