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(valid-describe): add check for concise body arrow functions #106

Merged

Conversation

Saberre
Copy link
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
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
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
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
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
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
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