- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 225
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
Don't invoke expect.extend #333
Don't invoke expect.extend #333
Conversation
We should keep the index file, but only export all matchers. We should also add something to the readme explaining how to use this |
👍 I started thinking that's what I should do. As far as the documentation, the extend they need to do now would replace the setupFilesAfterEnv, right? |
It should happen in |
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'm worried about this being a breaking change. Is there a way we can make this opt-in instead, at least short term? If we want to switch the default, we could add a warning to switch to the new version for at least a major before cutting it all the way over. |
This is breaking yes, but there are already breaking changes on master (dropping old node versions). Whether or not we'll have a "migration release" I don't have any opinions about |
I'm more worried about the migration path than just being a breaking change. What if we allowed for something like: "jest": {
"setupFilesAfterEnv": ["jest-extended/all"]
} |
README.md
Outdated
// ./testSetup.js | ||
|
||
// add all jest-extended matchers | ||
import matchers from 'jest-extended'; |
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.
Should this be
import * as matchers from 'jest-extended';
|
Yeah, either works. That may just be a nice compromise for folks who don't already have a custom setup env and don't want to bother setting one up just for this. |
Made changes I think as discussed. I also wasn't sure how we wanted to land this and with what other changes it might be shipped. |
Codecov Report
@@ Coverage Diff @@
## main #333 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 126 126
Lines 744 744
Branches 126 126
=========================================
Hits 744 744 Continue to review full report at Codecov.
|
@keeganwitt I think we can make a new release after we solve this one |
Would it be better to also have #332 landed first before cutting release? |
@keeganwitt see #332 (comment). I'm fine with only support v27, but we should first make a release of everything we have so people on older versions that 27 can use it |
…l` in Jest `setupFilesAfterEnv` config This is due to a breaking change in `jest-extended@1.1.0` + https://github.com/jest-community/jest-extended/releases/v1.0.0 + jest-community/jest-extended#333
* chore(deps): update dependency jest-extended to v1 * chore(encrypted-archive): rename `jest-extended` to `jest-extended/all` in Jest `setupFilesAfterEnv` config This is due to a breaking change in `jest-extended@1.1.0` + https://github.com/jest-community/jest-extended/releases/v1.0.0 + jest-community/jest-extended#333 Co-authored-by: Renovate Bot <bot@renovateapp.com> Co-authored-by: Sonishi Izuka <sounisi5011@users.noreply.github.com>
What
Don't extend expect with jest-extended matchers automatically. Let the user choose which to include with Jest in the scope that's appropriate.
Why
This allows the user more control. Also the existing extend used
global.expect
, which would not be the case wheninjectGlobals=false
.Notes
Housekeeping