docs(valid-describe): add suggestion to disable for dynamic tests

[chrisdothtml]

Fixes #203

[chrisdothtml]

Think CI just needs to be restarted

[jeysal]

@chrisdothtml Thanks for taking time to fix this!
I wonder though: If we allow x.name, why not e.g. x.getName() (another kind of expression)? Both could be incorrect just as well as both could be correct. And if we allow both (in fact, all kinds of expressions that we cannot infer the type of), does the plugin provide a big benefit over Jest checking what type the name really is at runtime?
I guess you could error only if it's clearly something invalid like an object expression. Is that better than allowing only clearly valid things (the by far most common case) and recommending eslint-disable / template string-ish workarounds in special cases though? Just something that came to my mind looking at the issue thread :)
cc @macklinu @SimenB

[chrisdothtml]

If we allow x.name, why not e.g. x.getName()

Ah, good point; I was just covering the cases mentioned in #203

Both could be incorrect just as well as both could be correct

Even further than that, x.name could also be incorrect if using a Proxy or getter; but that's Javascript for ya 🤷‍♂. I think the benefit this plugin provides is more for syntactical/signature checking rather than type checking.

In that light, perhaps the best way to resolve this is to just remove the First argument must be name error and rely on Describe requires name and callback arguments to check the amount of arguments

EDIT: Although, the same points could be made for the Second argument must be function error checking for an inline function. Maybe since dynamic test suites are more of an edge case, the recommendation for this issue is just disabling the rule

[SimenB]

If we could extract some type info that it's a string it might make sense, but at that point it's not really a lint rule's responsibility.

Thoughts on making docs and the error say something like "must be string literal" or something, and recommending disabling the rule if you have dynamic names (like you mention)?

[chrisdothtml]

Thoughts on making docs and the error say something like "must be string literal" or something, and recommending disabling the rule if you have dynamic names (like you mention)?

Yeah I think that's the best solution. Updated my PR to just a docs update

[SimenB]

Look at the code - this should keep the old message. Can we differentiate between missing first arg/wrong type and a variable?

[chrisdothtml]

I don't think I follow. Could you explain what exactly you're looking for?

[jeysal]

I think the point is that the new message suggests that the mistake was to pass a function instead of a string, when the mistake is really that the name is just missing before the function. The old message was neutral in this regard.
And I guess @SimenB's suggestion (second sentence) was to print something like "First argument must be name" in this case because we can infer that the user probably just forgot the name in before the function, and only if both args are there but the first is not a string literal print the new one :)

[SimenB]

Yes, that's what I meant. Thanks! 😀

[thisissami]

Seeing as how this hasn't been merged in yet, I'm just gonna chime in with a different opinion: the linter should not enforce a particular form of strings, period. The first argument for a describe should be a string, and any valid way of generating a string there should be valid. If there is no way to do this, then this should not be the linter's responsibility, and this rule should be completely removed.

If people want typechecking, they can use typescript, which jest fully supports; it will complain if you don't pass in a string.

If the community insists on this being a rule for the linter, then the message that it expresses should make it really clear that this might not actually be an issue. E.g. instead of

First argument must be name

it could be

First argument must be name. Also if you are using a string variable or function that returns a string, then please ignore this message.

^if something like that is needed, then... it's a code smell and probably shouldn't be something the linter handles.

My $.02.

EDIT: the "catching async callbacks" aspect of this rule, however, is great.

[pedro-mass]

What @thisissami suggested, or provide a config object that allows ignoring the name check

[chrisdothtml]

Closing as I don't plan on finishing this PR, so anyone can free to pick up the change. Doesn't seem like there's a solid verdict on how this should be handled though