Skip to content

fix(valid-describe): add check for concise body arrow functions#106

Merged
SimenB merged 4 commits intojest-community:masterfrom
Saberre:fix/valid-describe-short-arrow-func
May 23, 2018
Merged

fix(valid-describe): add check for concise body arrow functions#106
SimenB merged 4 commits intojest-community:masterfrom
Saberre:fix/valid-describe-short-arrow-func

Conversation

@Saberre
Copy link
Copy Markdown
Contributor

@Saberre Saberre commented Apr 18, 2018

Fixes #105.

I've added a new condition for an arrow function with concise body. I'm not good at writing English, so the report message may not be look good to you. If so, please let me know and I'll fix it!

@SimenB
Copy link
Copy Markdown
Member

SimenB commented Apr 19, 2018

I wonder if we should allow it? I don't think it hurts.

@thymikee thoughts?

(CI seems confused, I'm not sure what's up with it...)

@macklinu
Copy link
Copy Markdown
Collaborator

I think we should allow this ArrowFunctionExpression syntax. I don't normally see tests written with a body-less arrow function, which is mentioned in #105, but I think that's an edge case we should check and allow. If the user wants to only include one test() or it() function per describe(), that's their choice. Also, it's technically valid to write tests this way in Jest (if I'm not mistaken), so I don't think the valid-describe rule should prevent something that is technically valid. That's my opinion, but definitely open to hearing others' thoughts!

@Saberre
Copy link
Copy Markdown
Contributor Author

Saberre commented Apr 20, 2018

I first thought that a concise body arrow function violates the existing lint rule because it implicitly returns the value of its body. But I also think that it doesn't harm the describe block by returning a value. Please let me know which one is better and I'll reflect it to the code!

@SimenB
Copy link
Copy Markdown
Member

SimenB commented Apr 20, 2018

Brace-less arrows should be supported even though returning in a describe is weird in the normal case

@SimenB
Copy link
Copy Markdown
Member

SimenB commented May 23, 2018

Woah, I never got an email that you fixed this @Saberre. So sorry! 🙏

@SimenB SimenB merged commit 4586bdc into jest-community:master May 23, 2018
@SimenB
Copy link
Copy Markdown
Member

SimenB commented May 23, 2018

🎉 This PR is included in version 21.15.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug with valid-describe

3 participants