-
Notifications
You must be signed in to change notification settings - Fork 391
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
Add jest extensions on main module #175
Conversation
Codecov Report
@@ Coverage Diff @@
## master #175 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 19 19
Lines 234 234
Branches 57 57
=====================================
Hits 234 234
Continue to review full report at Codecov.
|
@gnapse Is the plan to move the type definitions to DefinitelyTyped and then introduce the dependency here like the other |
Yes, I was meaning to go through here with the piece of the puzzle that was missing that was preventing your PR to DT to be accepted. Based on our last comments in #123 we all converged to that (all meaning Kent, you and me). |
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.
LGTM!
@kentcdodds @jgoz was meaning to ask just to make sure: this is not a breaking change, right? The only thing I can see that could be a problem is that custom matchers are no longer exported on |
I don't think this is a breaking change. |
👍
If you don't re-export the matchers from |
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.
Looks good!
@gnapse Are you going to open a new DT PR or do you want me to do it? |
I thought the one you had started was still available to resume proposing it. Seems to still be open.
I know but still, it was even documented that you could selectively import the matchers and extend jest yourself. I don't want to take any chances. Would there be anything wrong in re-exporting them all on |
Sure, I can update that one with the latest typings.
Nothing technically wrong with it. But maybe it's worth doing a major bump to indicate that there was a significant change? I defer to you on this one. Though I think this change will need to be done in multiple steps:
|
@kentcdodds will love your input on all this. Both the breaking change issue, and the multiple steps. Would hate to see people's work flows broken for even a short while. |
That sounds fine to me 👍 Just make sure there's only one major version bump. I probably wouldn't merge anything until DT is ready to go and you've added the DT dep. No point in making all of these changes here if we're just going to move them right away. |
Then we shouldn't merge this PR first, right? Otherwise it'll create a new release right away 🤔 |
I forgot that releases are created on merge. I'll see if I can update the DT branch and refer the reviewers to Unless it's possible to create a beta/prerelease version instead of an actual version? |
We could/should have a |
Yeah, it doesn't happen very often, but when I want to group commits into a single release, I'll just make a temporary |
I created a I'm merging this soon. |
🎉 This PR is included in version 5.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
What:
extend-expect
module is kept and still does the same.matchers
module now that exports the matchers without having any side effect. It does not add them as jest extensions.Why:
#123 (comment)
Also related: DefinitelyTyped/DefinitelyTyped#37792
Checklist: