Make `no-wait-for-side-effects` report render() inside `waitFor`

[zaicevas]

Consider this: await waitFor(() => render(<Component />)). AFAIC it doesn't seem like a legit use case 🤔 Should we report every usage of render* wrapped in waitFor ?

[Belco90]

Hey @zaicevas! This is an interesting proposal. I think you are right and no-wait-for-side-effects should report render util calls inside waitFor indeed, and I guess rerender too by extension.

It would be nice to include this report in no-wait-for-side-effects but this can't be enabled by default. Following our Semantic Versioning Policy, "A new option to an existing rule that results in ESLint reporting more linting errors by default" would be a major version. So we need to aim for "A new option to an existing rule that does not result in ESLint reporting more linting errors by default" change which would result in a minor (and then make it a default option in the next major).

Following this workflow, we would need to include a new option (e.g shouldReportRender) accepting a boolean which defaults to false, and report this new case when it's enabled so we don't force a major release. What do you think?

[zaicevas]

Hey @zaicevas! This is an interesting proposal. I think you are right and no-wait-for-side-effects should report render util calls inside waitFor indeed, and I guess rerender too by extension.

It would be nice to include this report in no-wait-for-side-effects but this can't be enabled by default. Following our Semantic Versioning Policy, "A new option to an existing rule that results in ESLint reporting more linting errors by default" would be a major version. So we need to aim for "A new option to an existing rule that does not result in ESLint reporting more linting errors by default" change which would result in a minor (and then make it a default option in the next major).

Following this workflow, we would need to include a new option (e.g shouldReportRender) accepting a boolean which defaults to false, and report this new case when it's enabled so we don't force a major release. What do you think?

What you're saying makes total sense. Do you think we could enable shouldReportRender by default in v15?

An alternative would be to create a new rule and then merge it into one in v15. What do you think about that?

By the way, I'd like to try to implement it 😄

[Belco90]

We are still debating about our SemVer Policy, so I think it would be better if we report that error by default in the rule without any option, and then we see if we release it as a minor or we need to opt it out with some option. Are you happy with that?

[zaicevas]

We are still debating about our SemVer Policy, so I think it would be better if we report that error by default in the rule without any option, and then we see if we release it as a minor or we need to opt it out with some option. Are you happy with that?

Yeah. I'll keep an eye on the discussion you linked 👍