Skip to content
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

Merged
merged 21 commits into from Feb 16, 2022
Merged

Conversation

mrazauskas
Copy link
Contributor

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); moving jestExpect.ts from jest-circus to jest-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.

@SimenB
Copy link
Member

SimenB commented Feb 16, 2022

@SimenB Hope this isn’t overlapping with anything you do, I just couldn’t stop myself from trying out adding @jest/expect package.

Not at all, I haven't started to look at actual code yet, so this is wonderful! 👍

@SimenB
Copy link
Member

SimenB commented Feb 16, 2022

@mrazauskas ping me when this is ready for a first pass review? 🙂

@mrazauskas
Copy link
Contributor Author

mrazauskas commented Feb 16, 2022

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.

@SimenB
Copy link
Member

SimenB commented Feb 16, 2022

@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 {
Copy link
Member

@SimenB SimenB Feb 16, 2022

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)

Copy link
Member

@SimenB SimenB left a 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));
Copy link
Contributor Author

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="..." />

Copy link
Member

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

Copy link
Contributor Author

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

@Biki-das
Copy link
Contributor

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;
Copy link
Contributor Author

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.

Copy link
Member

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?

Copy link
Member

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(done)

@SimenB
Copy link
Member

SimenB commented Feb 16, 2022

@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 @jest/globals, not just @types/jest.

@mrazauskas
Copy link
Contributor Author

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.

@SimenB SimenB changed the title feat!: create new @jest/expect package feat: create new @jest/expect package Feb 16, 2022
@SimenB SimenB merged commit 755640d into jestjs:main Feb 16, 2022
@mrazauskas mrazauskas deleted the feat-jest-expect branch February 16, 2022 16:54
@SimenB
Copy link
Member

SimenB commented Feb 16, 2022

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 🙂

@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants