-
-
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
feat: create new @jest/expect
package
#12404
Conversation
Not at all, I haven't started to look at actual code yet, so this is wonderful! 👍 |
packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts
Outdated
Show resolved
Hide resolved
packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts
Outdated
Show resolved
Hide resolved
@mrazauskas ping me when this is ready for a first pass review? 🙂 |
@SimenB Only a couple of type tests are missing. Otherwise the code is ready for review. |
@mrazauskas I pushed what I played with locally, hopefully no conflict. (I got excited!) |
expect.setState({expand: config.expand}); | ||
export type {JestExpect} from './types'; | ||
|
||
export function createJestExpect(): JestExpect { |
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 meant expect
can export a createJestExpect
that we use here so we don't mutate the shared expect
with our extend
. But since extend
work by using module state and globals instead of internal to some closure that doesn't make sense.
But I don't think we need to export this one (I like keeping it, tho, for internal use)
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.
this is fantastic!! 🎉
expect({}).toMatchSnapshot(); | ||
}); | ||
|
||
expectType<void>(jestExpect.addSnapshotSerializer({} as any)); |
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.
This type is lost after bundling, because of augmentation. I will try .d.ts
file and /// <reference path="..." />
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.
huh. might be a bug in the bundler? Feels like declare module 'foobar'
should never be elided
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.
No luck with triple slash. Sounds like this issue: microsoft/rushstack#1709
wow this is amazing @mrazauskas |
@@ -16,6 +16,7 @@ export type JestExpect = { | |||
<T = unknown>(actual: T): JestMatchers<void, T> & | |||
Inverse<JestMatchers<void, T>> & | |||
PromiseMatchers<T>; | |||
addSnapshotSerializer: typeof addSerializer; |
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.
Adds duplication, but it helps to work around the problem. Type is aded here to be visible for the user, but also left in augmentation to avoid casting in code. I will follow the issue in api-extractor
and will clean up after it will be solved.
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.
add a comment above it?
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.
above this line, that is
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.
(done)
@mrazauskas is this good to go, or are you planning to write more tests? 🙂 I also think we should update https://jestjs.io/docs/getting-started#using-typescript and https://jestjs.io/docs/expect#expectextendmatchers for next release to mention how to add TS types for |
Yes, this can land now. Thanks! You are right, it is time to rework docs. I will give it a try. Many nice details were added and talked about in last days. Worth to write a few lines. |
@jest/expect
package@jest/expect
package
Released in https://github.com/facebook/jest/releases/tag/v28.0.0-alpha.2 if you wanna play with it in a project outside of this repo 🙂 |
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
@SimenB Hope this isn’t overlapping with anything you do, I just couldn’t stop myself from trying out adding
@jest/expect
package.Seems like this is the cleanest way to separate snapshot matchers implementation from
expect
. Also most flexible, since it becomes very easy to use@jest/expect
as stand alone package.Alternatives: moving
expect
types to@jest/types
(#12059); movingjestExpect.ts
fromjest-circus
tojest-runtime
. I was playing with both. That was rather complex and did not allow stand alone usage as@jest/expect
does.The implementation is slightly unfinished (
jasmine
should be rework), but seems like it does the right job.Test plan
Green CI.