-
Notifications
You must be signed in to change notification settings - Fork 274
[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
Conversation
There was a problem hiding this 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
website/docs/MigrationV12.md
Outdated
|
||
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. |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this 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 ReportBase: 96.09% // Head: 96.09% // Decreases project coverage by
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
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. |
* 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
* 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
Note: this PR merges to
v12
branch, for future v12 release.Summary
useBreakingChanges
(without changing the flag value)Test plan
All tests pass