-
Notifications
You must be signed in to change notification settings - Fork 234
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
[valid-describe] Support functions as 'name' argument #431
Comments
Thanks for reporting this. On the surface it seems easy, but it's actually an interesting problem, b/c it's possibly a lot of work to determine what's valid in that situation. I feel that this is the point where you'd be best to disable I'm not going to close this issue, as I think it's worth exploring what we can do, but my initial view is it's a possible can of worms. Here's a bit of a brain dump of my thinking:
The following are all valid:
This would be very complex and out of scope of an eslint plugin very quickly. It's also why I see it as a can of worms: it's very easy to "fix" the first one, the second and third are somewhat easy, but then it's reasonable to ask for support for the rest. The above is a bit of a brain dump, so it's possible I'm overthinking or have missed something, but for now, I don't know if there's a nice way we can solve this w/o crippling the rule or trying to write a mini-TS. |
Sometimes I'll use some weird things to name |
That's the can of worms I was thinking of :) What you're asking is for us to resolve the type of the parameter at runtime, which is static analysis, and best left to the likes of TypeScript. Again, not closing this just yet b/c I'm going to think on it for a bit for if there are any easy wins, and to get more opinions, ideas, input, etc, but I can say that whatever we come up w/, ultimately TypeScript will do it better in this case :) |
I would just say to give us an option on the rule, to disable checking the name field, since this rule also does other checks as well. |
+1 for a "validate name argument" option |
Maybe this helps: test(`${fn}`) We could ofc also consider specifically allowing |
FYI I'm going to be moving this into From everything I can tell, only |
🎉 This issue has been resolved in version 23.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Since Jest 22
describe()
also allows function references as "name" (instead of a string).See jestjs/jest#5154.
See also the TS typing: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/f79c08624659aea629bf0d1dc2f30a15e2c68383/types/jest/index.d.ts#L390
This is not supported by
valid-describe
:=> Emits "error First argument must be name jest/valid-describe".
The text was updated successfully, but these errors were encountered: