ESLint plugin Testing Library v4

[Belco90]

I'd like to keep everything planned for v4 update here, so it's easier for everyone involved to organize pending work. This was started in #165.

Changes planned for v4:

• new rule: no-container - new rule: no-container #171, feat: add no-container rule #177
• new rule: render-result-naming-convention - new rule: render-result-naming-convention #170, feat: add render-result-naming-convention rule #200
• new rule: prefer-user-event - New rule: prefer-user-event #162, [new-rule] prefer-user-event #192
• new rule: no-promise-in-fire-event - fireEvent with Promises #166, feat(no-promise-in-fire-event): add new no-promise-in-fire-event rule #180
• new rule: no-wait-for-side-effects - new rule: no-side-effects-wait-for #134, feat: add rule no-side-effects-wait-for #196
• new rule: no-node-access - feat: add no-node-access rule #190
• new rule: no-wait-for-multiple-assertions - new rule: no-multiple-assertions-wait-for #133, feat: add rule no-multiple-assertions-wait-for #189
• BREAKING CHANGE: enable no-wait-for-empty-callback in config presets - feat: recommend no-wait-for-empty-callback #168
• BREAKING CHANGE: enable prefer-screen-queries in config presets - feat: recommend prefer-screen-queries #169
• BREAKING CHANGE: rename recommended config to dom - Rename recommended config #182, refactor: rename recommended config to dom #184
• BREAKING CHANGE: move rules settings to ESLint shared config - Move rules settings to ESLint shared config #198
• ci: migrate CI to GitHub Actions

Extra:

• Cleanup after all rules are migrated: refactor: cleanup after migrating all rules to v4 #303
• Update README with shared settings: docs: update README for v4 #317
• Update README to explain "aggressive mode", its motivation and how to restrict its scope: docs: add guide for migrating to v4 #312
• Update rules docs to delete references to removed options
• Update CONTRIBUTING to mention new rules creation + detection helper + realistic test cases: docs: update contributing guidelines to v4 #321
• Write migrating to v4 guide: docs: add guide for migrating to v4 #312
• Prepare release notes

Future Improvements:

• Allow multiple utils modules on Shared Settings: Allow multiple utils modules on Shared Settings #315
• New rule: no-unnecessary-act - add no-unnecessary-act rule #259

Thanks to @gndelia @timdeschryver @nickmccurdy @thebinaryfelix @renatoagds for your work on this one!

[Belco90]

Do you think we need to wait for those 4 new rules for releasing v4? I think render-result-naming-convention and prefer-user-event are necessary, but we could release prefer-user-event and no-multiple-assertions-wait-for in a later step?

[nickserv]

We could also assign issues from the milestone.

[Belco90]

Yeah I just felt I needed a place to see everything together but also be able to discuss what's needed and coordinate this better.

[nickserv]

Do you think we need to wait for those 4 new rules for releasing v4? I think render-result-naming-convention and prefer-user-event are necessary, but we could release prefer-user-event and no-multiple-assertions-wait-for in a later step?

I guess that depends on if we want to make them recommended, and I think they probably should be. As long as it doesn't take too longer to work on them, waiting to release them would reduce the number of breaking releases and potentially make upgrading easier. Alternatively we could make the migration more gradual and maybe release rules in 4.x but only recommend them in 5.x.

[Belco90]

Mmm I wasn't thinking about releasing new rules included in config presets as major. I see including rules in preset as a major only when they existed before, but not if they are gonna be completely new rules. Again, I don't know how we should address this.

[nickserv]

Both adding an existing rule and adding a new rule to a config change the config to make more use cases fail, which I see as a breaking change either way. So I think we'd either need to wait for these to be in v4 or have them in a future major release if we don't enable them in v4, but I was asking more about how we'd release the rules themselves and change the configs.

[nickserv]

Maybe we should make suggestions about new features warn rather than error to make it easier for users to migrate.

Relevant discussions:

• [new-rule] prefer-user-event #192 (comment)
• feat(eslint-config-react-app): Add jest & testing-library rules facebook/create-react-app#8963 (comment)

[Belco90]

Maybe we should make suggestions about new features warn rather than error to make it easier for users to migrate.

Relevant discussions:

* [#192 (comment)](/~https://github.com/testing-library/eslint-plugin-testing-library/pull/192#issuecomment-652529480)

* [facebook/create-react-app#8963 (comment)](/~https://github.com/facebook/create-react-app/pull/8963#issuecomment-652398748)

I guess the candidates for that would be:

• prefer-user-event
• no-side-effects-wait-for (when implemented)
• no-multiple-assertions-wait-for

Any other rule I'm missing?

[renatoagds]

@Belco90 Maybe no-container too? Looks like something good to have as default in CRA.

[Belco90]

@Belco90 Maybe no-container too? Looks like something good to have as default in CRA.

We don't really need to worry about if a rule should be enabled for CRA or not, as CRA will enable them manually. In our shareable configs is more about "should we enable a rule as error for a brand new Testing Library feature"? Probably better a warning for them as @nickmccurdy pointed so we can help users migrating their Testing Library framework codebase for future updates without harming them.

So regarding no-container, I'd set that one as error in our shareable configs.

[Belco90]

Something else I would like to discuss: I'm close to finish render-result-naming-convention rule and I had to use again the rule option for custom render functions. I'm gonna leave it like that, but it's the 3rd or 4th time we need to do this, so I think we should use v4 to introduce the breaking change to move that to ESLint global config.

I have some ideas about how to implement it, I don't think it would be complicated. As we have some utils functions already checking if a node is a render method, we can check directly on that one if there are some custom functions set in the plugin. Then we just need to remove the options from each rule, update the set up in the tests and all of them should pass.

Thoughts?

[gndelia]

I think it'd be really useful to have that generic configuration/functionality for the v4.

Either way, I think it's not a breaking change, but rather an opt-in (Extend the functionality to allow custom functions to be the imported ones), so in case we finish everything else but this, the worst case we can go out with v4 without it, if we can't delay it.

but I would love to have it as part of this release, and I think it will be very helpful for all those who use a custom render function (that I believe it's a lot of devs)

[thebinaryfelix]

I think it would be useful too. Would also avoid repetitions by the user in the rules configs.

Either way, I think it's not a breaking change, but rather an opt-in

I think if we remove the config option from each rule to read it just from a global config, the user would have to rewrite their configs. That's what makes it a breaking change, right?

[Belco90]

I think if we remove the config option from each rule to read it just from a global config, the user would have to rewrite their configs. That's what makes it a breaking change, right?

Exactly, this is why it would be a breaking change

[gndelia]

I See. The rules I've worked with did not have that config, so I wasn't sure it was implemented and released in any. In that case, yeah, we should consider it as part of the v4, or we will have to release the v5 really soon

[renatoagds]

Guys, looking at the v4 milestone, I would suggest something more. It's not required for v4 release, but probably we could start to ship it in v4 branch.

We could start to map fixable actions for the current rules that we support. We have a few ones that implements, but I think that's a path to improve.

What do you guys think about?

[nickserv]

Any specific suggestions?

[Belco90]

Guys, looking at the v4 milestone, I would suggest something more. It's not required for v4 release, but probably we could start to ship it in v4 branch.

We could start to map fixable actions for the current rules that we support. We have a few ones that implements, but I think that's a path to improve.

What do you guys think about?

Sounds good. I think we can do this even after v4 release. It seems it can be related to that mechanism @nickmccurdy wanted to add to autogenerate the shareable configs columns?

[nickserv]

I think @renatoagds was referring to implementing fix support in more rules. According to the readme, three rules have it so far. We could map the fix flag in each rule to the readme though.

[Belco90]

Ah sorry, I understood something else. Add fix code to current rules can be done at any time actually.

I'm sorry I'm not really active lately. I'm swamped at work and dealing with some other personal projects. I hope I have some spare time next week to push v4 of the plugin!

[renatoagds]

Add fix code to current rules can be done at any time actually.

Yes! The point was about mapping this in some place (including a few rules that is included only in v4)

Which is the best place to map ? A new issue ?

[Belco90]

@renatoagds yeah, I think a new issue would be better so we can organize corresponding work there. Thanks!

[Belco90]

I've created a new issue for discussing and addressing the ESLint shared config feature #198

[Belco90]

Almost there! I hope we can release v4 soon 🤞

[Belco90]

I just realized @typescript-eslint/experimental-utils exports an ASTUtils module which already includes:

• utils to find predicates. We have rewritten some of them in the plugin like isIdentifier or isAwaitExpression
• eslint-utils package with corresponding TS types. This will be really useful for stuff I want to implement in Move rules settings to ESLint shared config #198

I feel so stupid now... I'll go back to #198 reusing this utils and try to remove the duplicated predicated ones we might have.

[MichaelDeBoey]

@Belco90 This means eslint-utils should become a direct dependency too
Unfortunately it's not ESLint 7 compatible (yet) 😢

[Belco90]

Hey people! Just in case you haven't checked #198 we are doing some progress finally on last task! Mechanism to make plugin rules and detect Testing Library utils is implemented so from there we can start refactoring rules. There are few more things to add to the Testing Library utils detection, but the difficult part is done already ✨

[Belco90]

We are really close to release v4, only 5 rules to go! After that I need to write proper documentation and do some cleanups, but I'm planning to release a beta version in case some of you can give it a try while I do those extra steps.

[Belco90]

It seemed like this day would never come, but finally, all rules are migrated to the new rule creator + helpers mechanism. I'm creating a small PR with some cleanup, and I'll create some small additional PRs to re-update some of the first rules migrated here to reuse some helpers added after they were migrated.

The next steps are:

• move v4 branch to beta branch so I can get the proper @next version published to NPM with Semantic Release
• add pending documentation while waiting for some beta feedback: migrate to v4 guide, include new config on README, small document about Aggressive Reporting, and get release changelog ready
• fix issues or feedback from the beta version

After this, we should be ready to release v4 officially.

[Belco90]

Ok, so 4.0.0-beta.1 has been released successfully. You can install it just doing npm install -D eslint-plugin-testing-library@beta or yarn add -D eslint-plugin-testing-library@beta. This is gonna be tricky if you are not aware of the breaking changes etc. Then the first thing I'm doing now is writing the migration guide so people can adapt their codebase properly for using this v4 beta. I'll leave a comment here when this guide is available!

[Belco90]

I did a small check overall rules within a demo project and everything is working pretty smooth in general 🎉 . I found some weird node locations for some errors. I'll create a ticket for those issues and fix them in a proper PR.

[Belco90]

I just published 4.0.0-beta.3 with all the issues found so far fixed. It looks fine within the demo project I checked it. I'll do more deep testing in other projects, and keep writing documentation tomorrow.

[Belco90]

New beta 4.0.0-beta.4 was published today and the first draft for "migrating to v4 guide" written in #312

[Belco90]

Latest 4.0.0-beta.6 seems pretty stable. I just updated the contributing guidelines and I'm writing the release notes, so v4 should be officially released tomorrow! 🚀

[github-actions]

🎉 This issue has been resolved in version 4.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀