eslint-plugin-testing-library
eslint-plugin-testing-library copied to clipboard
Make sure `no-unnecessary-act` detects unnecessary `act` called before the `render` method
What rule do you want to change?
no-unnecessary-act
Does this change cause the rule to produce more or fewer warnings?
More warnings
How will the change be implemented?
I don't have a plan to implement the change yet.
The idea is to improve the rule no-unnecessary-act
so it can detect unnecessary act
which are called before a render.
https://github.com/testing-library/eslint-plugin-testing-library/blob/main/docs/rules/no-unnecessary-act.md
Those unnecessary make the codebase noisy.
Example code
act(() => {
someSetupMethod();
});
render(<Example />);
How does the current rule affect the code?
This unnecessary act
is not detected
How will the new rule affect the code?
This unnecessary act
is detected
Anything else?
No response
Do you want to submit a pull request to change the rule?
Yes, but need help
Hey @fabiendem! Thanks for the suggestion, it's an interesting one. It would be great if you finally make some time and implement it yourself. I'll try to help as much as I can!
@Belco90 cheers. I use a lot of ESLint rules but I have never written one.
- Do you have a list of good resources to start with?
- Can you think of a similar constraint in the existing rules ("do not do X before Y"), so I can see examples?
- Any other hint is welcomed 🙏
Thanks
I see. Some resources to help you with this:
- The Code Transformation and Linting with ASTs course is the one I used to learn about how to work with AST to build this plugin
- I think there is no similar constraint as the one you want to apply. There are others about "do not do X inside Y callback" but they won't work in the same way. I can suggest you check
no-render-in-setup
andrender-result-naming-convention
which are related to therender
method so you can see how to work around that - You can find some contributing guidelines in this project that will lead you through some parts and point to a few utils. We have tons of utils you can take advantage of, like the
isRenderUtil
or theisRenderVariableDeclarator
, so you don't need to manually look forrender
methods but rely on those utils.
Thank you 👍
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Feature is still relevant, I didn't have a chance to tackle it yet.
This may be tricky to check tho. What if the developer is rendering the component in the beforeEach
hook? This rule won't find any valid render within the tests so all acts could be potentially reported.
We have this rule in place: https://github.com/testing-library/eslint-plugin-testing-library/blob/main/docs/rules/no-render-in-setup.md So this is not an issue for us. But surely not everyone will have this rule enabled.
But even, we would want to detect any act
before a render
call.
We can start by detecting this in the same blocks. Then detect acts
in the before*
blocks as well.
Rules must not depend on each other, so we have to assume this scenario is possible.
We can start by detecting this in the same blocks. Then detect acts in the before* blocks as well.
LGTM! So this rule would report acts for the current criteria OR for acts called before a render within the test block (ignoring them if no render is found).
Rules must not depend on each other, so we have to assume this scenario is possible.
yeah definitely
LGTM! So this rule would report acts for the current criteria OR for acts called before a render within the test block (ignoring them if no render is found).
yeah seems legit