Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking changes for next v5 #478

Closed
11 tasks done
Belco90 opened this issue Sep 20, 2021 · 6 comments · Fixed by #471
Closed
11 tasks done

Breaking changes for next v5 #478

Belco90 opened this issue Sep 20, 2021 · 6 comments · Fixed by #471
Assignees
Labels
BREAKING CHANGE This change will require a major version bump enhancement New feature or request released on @alpha released
Milestone

Comments

@Belco90
Copy link
Member

Belco90 commented Sep 20, 2021

I have a couple of breaking changes I think we should introduce in the new major version taking advantage of having to support ESLint 8

This project is maintained by volunteers. Support will be added as time permits.

If you're not willing or able to help with adding support:

  • Please do not raise issues.
  • Please do not comment on threads.
  • Please be patient and subscribe to this issue for updates.

This update will require breaking changes on our side as well, so it will take some time to support it.
There is no ETA on this work. Again please be patient.


✅ = done
🕐 = in progress
🔲 = not yet started

✅ Enable more rules in the shared configs

no-render-in-setup

Enable for angular, react & vue configs (dom doesn't have a render method)

no-unnecessary-act

Enable for react configs

no-wait-for-multiple-assertions

Enable for all configs

no-wait-for-side-effects

Enable for all configs

no-wait-for-snapshot

Enable for all configs

prefer-presence-queries

Enable for all configs

prefer-query-by-disappearance

Enable for all configs

✅ Make isStrict option true by default for no-unnecessary-act

We have enabled this rule with isStrict to true in the shared configs, this option is still false by default. We should change it so it's true by default.

✅ Rules rename

We have some inconsistencies in naming some of our rules regarding using singular/plural.

await-async-query -> await-async-queries

no-await-sync-query -> no-await-sync-queries

no-debug -> no-debugging-utils

@Belco90 Belco90 added the enhancement New feature or request label Sep 20, 2021
@Belco90 Belco90 added this to the 5.0.0 milestone Sep 20, 2021
@Belco90 Belco90 added the BREAKING CHANGE This change will require a major version bump label Sep 20, 2021
@Belco90 Belco90 self-assigned this Sep 20, 2021
@MichaelDeBoey
Copy link
Member

Enable more rules in the shared configs

I think we should enable these rules too:

  • testing-library/no-render-in-setup
  • testing-library/no-unnecessary-act (✅ done already)
  • testing-library/no-wait-for-multiple-assertions
  • testing-library/no-wait-for-side-effects
  • testing-library/no-wait-for-snapshot
  • testing-library/prefer-presence-queries
  • testing-library/prefer-query-by-disappearance

Do we enable them in all configs or only in specific framework configs?

[no-unnecessary-act] Set isStrict to true by default

We have enabled this rule with isStrict to true in the shared configs, this option is still false by default. We should change it so it's true by default.

This means we can just make no-unnecessary-act an error in the React config

Rules renaming for consistency

We have some inconsistencies in naming some of our rules regarding using singular/plural.

At the moment we have:

  • await-async-query
  • no-await-sync-query
  • prefer-presence-queries
  • prefer-query-by-disappearance
  • prefer-screen-queries

So query could have two meanings: the verb (to query) and the noun (a query). I think we should use the singular for the verb and the plural for the noun. All our usages are nouns, so all of them should be plural perhaps?

  • await-async-queries
  • no-await-sync-queries
  • prefer-presence-queries
  • prefer-queries-by-disappearance
  • prefer-screen-queries

I think we should keep prefer-query-by-disappearance as this is about the specific queryBy* queries.
All others are more generic, so those are better to be made consistent indeed.

@Belco90
Copy link
Member Author

Belco90 commented Sep 20, 2021

Do we enable them in all configs or only in specific framework configs?

I'd say in all applicable ones:

  • testing-library/no-render-in-setup: angular, react and vue (dom doesn't have a render method)
  • testing-library/no-wait-for-multiple-assertions: all
  • testing-library/no-wait-for-side-effects: all
  • testing-library/no-wait-for-snapshot: all
  • testing-library/prefer-presence-queries: all
  • testing-library/prefer-query-by-disappearance: all

This means we can just make no-unnecessary-act an error in the React config

Indeed!

I think we should keep prefer-query-by-disappearance as this is about the specific queryBy* queries.
All others are more generic, so those are better to be made consistent indeed.

As you mentioned, this is about the specific queryBy* queries so that's why I suggested renaming to "queries" in that one too.

@github-actions
Copy link

🎉 This issue has been resolved in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

github-actions bot commented Oct 5, 2022

🎉 This issue has been resolved in version 6.0.0-alpha.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

github-actions bot commented Oct 5, 2022

🎉 This issue has been resolved in version 6.0.0-alpha.11 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This issue has been resolved in version 6.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE This change will require a major version bump enhancement New feature or request released on @alpha released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants