-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Rename jest-matchers to expect #4345
Conversation
@@ -41,7 +41,7 @@ const JS_FILES_PATTERN = '**/*.js'; | |||
const IGNORE_PATTERN = '**/__tests__/**'; | |||
const PACKAGES_DIR = path.resolve(__dirname, '../packages'); | |||
|
|||
const INLINE_REQUIRE_BLACKLIST = /packages\/(jest-(circus|diff|get-type|jasmine2|matcher-utils|matchers|message-util|regex-util|snapshot))|pretty-format\//; | |||
const INLINE_REQUIRE_BLACKLIST = /packages\/expect|(jest-(circus|diff|get-type|jasmine2|matcher-utils|message-util|regex-util|snapshot))|pretty-format\//; |
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 though for a moment the Symbol bug was back before I remembered to change this... Gave me a scare!
@@ -1,5 +1,5 @@ | |||
{ | |||
"name": "jest-matchers", | |||
"name": "expect", |
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.
Isn't the plan to use expect directly, not to replace it with the contents of jest-matchers? expect has lots of important tests and behaviors that jest-matchers may not be replicating, so semver-major or not, that could end up screwing over a lot of people.
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.
Any concrete features you can think of?
Mocking is provided by jest-mock
, which will need docs of course, but I think there is feature parity.
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'd have to go through all the tests.
I'm pretty sure more people use expect than jest-matchers; I'm not sure why the former wouldn't be the one that should survive.
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.
Yep, this is happening @ljharb and @mjackson agreed on this almost a year back. It took us a long time to get everything in shape, and make jest-matchers
work in browsers etc. @skovhus built an extensive codemod that gets you 90% of the way there: skovhus/jest-codemods#62
Your assumption about more people using expect
than jest-matchers
is not correct according to this chart: http://www.npmtrends.com/expect-vs-jest-matchers although I'm willing to believe many people use a single-file bundle of it in a web site; which also means they don't rely on the npm package being called expect
and are not updating it through npm.
Either way, we'll publish expect
with version 21. If people want to keep using the old expect, patches can be released to the 1.x
version of the project.
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.
@ljharb The plan was always to donate the expect
package to the Jest team so they could run with it. They are doing an excellent job making an assertion library (jest-matchers) with very similar goals to expect, not to mention the syntax for most common assertions is practically identical, and I'd rather unite the community under one package than two.
As for "screwing over a lot of people" I don't think that's happening here. I've been very clear about my intentions for the expect project for a long time now and most people I talk to seem very supportive of the idea.
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.
Fair; I suppose I was just under the wrong impression.
I'm supportive of a package remaining maintained; if it needs to be replaced with a different implementation to do so, that's fine; I just hope that the breaking changes will be minimal and clearly documented.
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.
There's a a codemod (skovhus/jest-codemods#62) which will port most of existent expect
code. I don't think there's any breaking changes beyond that
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.
Codemods are great; but I don't think automated ones should be required to be able to have a smooth upgrade experience.
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "jest-matcher-utils", |
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 probably rename this package to something else too…
Finally, it took an entire year to get this ready, but I'm so excited <3 |
Nice! 🍺🍾 But yes, let us get the browser bugs fixed. |
Congrats everyone on moving this forward! You rock! 🤘🏻
I'm still using expect w/out the rest of Jest in the |
What sort of browser support is expected? Is IE10 gonna be enough? |
Sure, I think IE 10 is a reasonable expectation. Seems odd though that an assertion library shouldn't be able to work in almost any browser. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
I think this went deceptively smooth... What did I miss?
FWIW I'd still like to see #4074 fixed.
Closes #1679
/cc @mjackson @skovhus
Test plan
jest