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

Incorrect validTitle error flag #312

Closed
toduyemi opened this issue Aug 22, 2024 · 4 comments
Closed

Incorrect validTitle error flag #312

toduyemi opened this issue Aug 22, 2024 · 4 comments
Labels

Comments

@toduyemi
Copy link

Example =>
Screenshot 2024-08-22 at 10 52 32 AM

Title is a string yet an error is thrown.

@mskelton
Copy link
Member

The rule doesn't do semantic analysis, and doesn't have type information, so this is expected.

@mskelton mskelton closed this as not planned Won't fix, can't repro, duplicate, stale Aug 24, 2024
@toduyemi
Copy link
Author

toduyemi commented Aug 26, 2024

This doesn't make sense. The rule says title must be a string... the title is a string... and it's calling an error. That is by every definition a bug. What's the intention behind this rule?

If the rule only sees string-literals as valid, that's poor coding practices. Why is a linter advocating for it? Even still, if this is expected behaviour, it should say that in the rule/documentation.

https://github.com/playwright-community/eslint-plugin-playwright/blob/main/docs/rules/valid-title.md => Nowhere does it indicate developers storing strings in variables for titles is invalid. This is an extremely common practice whereas using string literals generally frowned upon.

"Title must be a string literal."

@mskelton
Copy link
Member

mskelton commented Sep 3, 2024

@toduyemi It's not a bug, this plugin does not use type information and thus cannot in all circumstances determine the type of the variable. We could add some amount of logic to make that work in the current file, but it's such a rare thing its not worth the effort.

The rule was ported from the Jest ESLint plugin as part of our effort to provide compatibility and feature completeness between the two. I do think it's a fine rule, test titles with a variable instead of a string literal is a super rare case, so I don't really care to handle most of the rare exceptions to this rule.

mskelton added a commit that referenced this issue Oct 19, 2024
Fixes #295
Fixes #243
Fixes #312
Fixes #320

I give in, I've got enough reports of this from users I'm tired of
dealing with it and decided to just support basic semantic analysis. No
doubt at some point someone is going to expect it to work with full type
information cause they'll have some weird use case where they import
some title string function from a util or something dumb like that, but
at least this will quell the nonsense for most simple cases.
Copy link

🎉 This issue has been resolved in version 1.8.0 🎉

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
Projects
None yet
Development

No branches or pull requests

2 participants