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

fix(no-await-sync-events): stop reporting user-event by default #803

Conversation

Belco90
Copy link
Member

@Belco90 Belco90 commented Aug 22, 2023

Checks

Changes

  • Stop reporting user-event methods by default, assuming v14 is the standard now.

Context

Fixes #801
Fixes #669

@Belco90 Belco90 added the bug Something isn't working label Aug 22, 2023
@Belco90 Belco90 requested a review from a team August 22, 2023 14:30
@Belco90 Belco90 self-assigned this Aug 22, 2023
lib/configs/angular.ts Outdated Show resolved Hide resolved
lib/configs/dom.ts Outdated Show resolved Hide resolved
lib/configs/react.ts Outdated Show resolved Hide resolved
lib/rules/no-await-sync-events.ts Outdated Show resolved Hide resolved
lib/rules/no-await-sync-events.ts Outdated Show resolved Hide resolved
lib/rules/no-await-sync-events.ts Outdated Show resolved Hide resolved
tests/lib/rules/no-await-sync-events.test.ts Outdated Show resolved Hide resolved
@Belco90 Belco90 requested review from MichaelDeBoey and a team August 23, 2023 08:34
@MichaelDeBoey MichaelDeBoey added the BREAKING CHANGE This change will require a major version bump label Aug 23, 2023
Copy link
Member

@MichaelDeBoey MichaelDeBoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should release this in a major release btw as this is breaking behavior for this specific rule and people need to adjust their config if they want the current released functionality (for instance when only enabling this specific rule)

lib/rules/no-await-sync-events.ts Outdated Show resolved Hide resolved
Co-authored-by: Michaël De Boey <info@michaeldeboey.be>
Signed-off-by: Mario Beltrán <belco90@gmail.com>
@Belco90
Copy link
Member Author

Belco90 commented Aug 23, 2023

We should release this in a major release btw as this is breaking behavior for this specific rule and people need to adjust their config if they want the current released functionality (for instance when only enabling this specific rule)

Not really. In our semantic version policy, we specify that a bug fix in a rule that results in eslint-plugin-testing-library reporting fewer errors will be a patch release.

The rule was reporting both fire-event and user-event. The change done here is to report only fire-event by default. The changes in the presets were needed to reflect the new default change correctly. However, the actual config of the rule and the way it reports the errors are the same.

I get the point of your comment, tho. A change in the number of errors reported could potentially be a breaking change. That's why we defined the semver policy, inspired by ESLint semver policity itself, so users know what to expect.

lib/rules/no-await-sync-events.ts Outdated Show resolved Hide resolved
@MichaelDeBoey MichaelDeBoey removed the BREAKING CHANGE This change will require a major version bump label Aug 23, 2023
lib/rules/no-await-sync-events.ts Outdated Show resolved Hide resolved
lib/rules/no-await-sync-events.ts Outdated Show resolved Hide resolved
lib/rules/no-await-sync-events.ts Outdated Show resolved Hide resolved
lib/rules/no-await-sync-events.ts Outdated Show resolved Hide resolved
lib/rules/no-await-sync-events.ts Outdated Show resolved Hide resolved
lib/rules/no-await-sync-events.ts Outdated Show resolved Hide resolved
lib/rules/no-await-sync-events.ts Outdated Show resolved Hide resolved
@Belco90 Belco90 merged commit 88246fc into main Aug 24, 2023
29 checks passed
@Belco90 Belco90 deleted the 801-contradicting-issue-reports-between-async-await-events-and-no-await-sync-events branch August 24, 2023 15:51
@github-actions
Copy link

🎉 This PR is included in version 6.0.1 🎉

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
bug Something isn't working released
Projects
None yet
2 participants