Make sure `no-unnecessary-act` detects unnecessary `act` called before the `render` method

[fabiendem]

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

[Belco90]

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!

[fabiendem]

@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

[Belco90]

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 and render-result-naming-convention which are related to the render 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 the isRenderVariableDeclarator, so you don't need to manually look for render methods but rely on those utils.

[fabiendem]

Thank you 👍

[stale]

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.

[fabiendem]

Feature is still relevant, I didn't have a chance to tackle it yet.

[Belco90]

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.

[fabiendem]

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.

[Belco90]

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).

[fabiendem]

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