feat(prefer-find-by): make the rule fixable #153
Conversation
|
So I think that scenario should be fixed too. |
| context.report({ | ||
| node, | ||
| messageId: "preferFindBy", | ||
| data: { queryVariant, queryMethod, fullQuery }, | ||
| fix(fixer) { | ||
| const newCode = `${caller ? `${caller}.` : ''}${queryVariant}${queryMethod}(${callArguments.map((node) => sourceCode.getText(node)).join(', ')})` |
There was a problem hiding this comment.
Damn, even checking the test I can't figure out how this replaces waitFor + getBy with findBy
There was a problem hiding this comment.
does that mean is it too obscure? 🤣
There was a problem hiding this comment.
Ah sorry, I just realized queryVariant is the corresponding findBy from getFindByQueryVariant, rather than the original query variant found in the code as I thought. Could you add a simple jsdoc to reportInvalidUsage to clarify what args are expected? Probably would be a good idea to add more jsdoc around the plugin.
There was a problem hiding this comment.
Do you want me to write a suggestion for this jsdoc? We need something simple, not elaborated description necessary here.
There was a problem hiding this comment.
As you want. I can do it, but I will probably do it in the weekend as I have some work to do 😄
There was a problem hiding this comment.
No problem, there is no rush around this!
There was a problem hiding this comment.
Done!
Hovering the method shows the docs:
There was a problem hiding this comment.
Awesome! It wasn't even necessary to re-add the type, but thanks!
yeah, I felt the same. The PR fixes that scenario as well 🎉 |
| context.report({ | ||
| node, | ||
| messageId: "preferFindBy", | ||
| data: { queryVariant, queryMethod, fullQuery }, | ||
| fix(fixer) { | ||
| const newCode = `${caller ? `${caller}.` : ''}${queryVariant}${queryMethod}(${callArguments.map((node) => sourceCode.getText(node)).join(', ')})` |
There was a problem hiding this comment.
Ah sorry, I just realized queryVariant is the corresponding findBy from getFindByQueryVariant, rather than the original query variant found in the code as I thought. Could you add a simple jsdoc to reportInvalidUsage to clarify what args are expected? Probably would be a good idea to add more jsdoc around the plugin.
gndelia
force-pushed
the
feature/prefer-find-by-automatic-fix
branch
from
bee49db to
f8050b9
Compare
| context.report({ | ||
| node, | ||
| messageId: "preferFindBy", | ||
| data: { queryVariant, queryMethod, fullQuery }, | ||
| fix(fixer) { | ||
| const newCode = `${caller ? `${caller}.` : ''}${queryVariant}${queryMethod}(${callArguments.map((node) => sourceCode.getText(node)).join(', ')})` |
There was a problem hiding this comment.
Awesome! It wasn't even necessary to re-add the type, but thanks!
Belco90
changed the title
|
🎉 This PR is included in version 3.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This feature extends prefer-find-by rule to make it fixable
One doubt I had was whether to fix or not this scenario
as
queryBy*methods do not throw if not found, butfindBy*do. At first I thought of not changing the behavior, but then I consider that if null was always returned, eventuallywaitForwould timeout, throwing as well (just like thefindBy*method would)