Skip to content

[v12] refactor: enable all breaking changes #1313

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

Merged
merged 11 commits into from
Feb 16, 2023

Conversation

mdjastrzebski
Copy link
Member

@mdjastrzebski mdjastrzebski commented Feb 13, 2023

Note: this PR merges to v12 branch, for future v12 release.

Summary

  • enable by default all features hidden behind useBreakingChanges (without changing the flag value)
  • remove dead code & dead tests
  • update docs

Test plan

All tests pass

@mdjastrzebski mdjastrzebski marked this pull request as ready for review February 14, 2023 08:51
Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

👍🏼 I think we need to start writing blog posts where migration guide is only an extension :D


While potentially breaking, this should not cause issues in tests if you are using recommended queries and Jest Matchers from Jest Native package.

Problematic cases may include: examining `type` property of `ReactTestInstance`, referencing other nodes using `parent` or `children` props, etc.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like the most common case breaking will be reading element prop, which is not listed here and it's not clear that this particular case should be solved through usage of jest native matchers imo

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also it could be useful to mention that the parent of the element that is now returned is the composite element that was returned before v12 as it provides an easy way to fix broken tests at least temporarily

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated comments. I would not hint users about parent fix, as I think users are better by writing these tests in a recommended way.

Copy link
Collaborator

@pierrezimmermannbam pierrezimmermannbam left a comment

Choose a reason for hiding this comment

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

Very nice! We're very close to the release now! The documentation on testing environment mentions that some queries return composite components which is no longer true so it should be updated as well. I've left some other comments, mainly regarding the migration guide

@codecov
Copy link

codecov bot commented Feb 15, 2023

Codecov Report

Base: 96.09% // Head: 96.09% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (2e93220) compared to base (47b125c).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##              v12    #1313      +/-   ##
==========================================
- Coverage   96.09%   96.09%   -0.01%     
==========================================
  Files          49       49              
  Lines        3333     3275      -58     
  Branches      503      489      -14     
==========================================
- Hits         3203     3147      -56     
+ Misses        130      128       -2     
Impacted Files Coverage Δ
src/config.ts 100.00% <ø> (ø)
src/queries/text.ts 100.00% <ø> (+2.10%) ⬆️
src/queries/displayValue.ts 100.00% <100.00%> (ø)
src/queries/placeholderText.ts 100.00% <100.00%> (ø)
src/queries/role.ts 100.00% <100.00%> (ø)
src/render.tsx 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mdjastrzebski mdjastrzebski merged commit 20a8768 into v12 Feb 16, 2023
@mdjastrzebski mdjastrzebski deleted the refactor/enable-breaking-changes branch February 16, 2023 13:27
@mdjastrzebski mdjastrzebski mentioned this pull request Feb 17, 2023
6 tasks
mdjastrzebski added a commit that referenced this pull request Feb 20, 2023
* chore: enable all breaking changes

* fix: ci issues

* chore: fix lint

* chore: cleanup

* chore: add migration guide

* chore: update docs

* chore: docs tweaks

* chore: docs tweaks

* chore: remove useBreakingChanges config flag

* refactor: code review changes

* docs: tweaks
mdjastrzebski added a commit that referenced this pull request Mar 8, 2023
* chore: enable all breaking changes

* fix: ci issues

* chore: fix lint

* chore: cleanup

* chore: add migration guide

* chore: update docs

* chore: docs tweaks

* chore: docs tweaks

* chore: remove useBreakingChanges config flag

* refactor: code review changes

* docs: tweaks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants