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

Support expect.addSnapshotSerializer() #29

Merged
merged 3 commits into from Apr 27, 2022

Conversation

fisker
Copy link
Contributor

@fisker fisker commented Apr 27, 2022

@SimenB
Copy link
Contributor

SimenB commented Apr 27, 2022

This runner can probably import and use @jest/expect instead of expect directly, fwiw. then you can drop the manual expect.extend call to add the snapshot matchers

@fisker
Copy link
Contributor Author

fisker commented Apr 27, 2022

Good idea, I'll have a test.

@fisker
Copy link
Contributor Author

fisker commented Apr 27, 2022

It doesn't work as expected, but maybe because there are multiple version of expect. I'll try again.

@SimenB
Copy link
Contributor

SimenB commented Apr 27, 2022

yarn dedupe maybe? not sure what error you got

@fisker
Copy link
Contributor Author

fisker commented Apr 27, 2022

No error, but expect.addSnapshotSerializer added serializer didn't work. Added to somewhere else?

yarn dedupe

Can't, haven't updated Jest.

@fisker
Copy link
Contributor Author

fisker commented Apr 27, 2022

Yah, that's the problem. I didn't install @jest/expect for this runner, so I'm using an old version. After I installed a new version of @jest/expect to the runner package, it works.

Can this cause problem? Not sure.

@fisker fisker marked this pull request as ready for review April 27, 2022 14:08
Copy link
Owner

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

@jest/expect is just a tiny wrapper around expect (https://unpkg.com/browse/@jest/expect@28.0.2/build/index.js), so it shouldn't cause any problem that we cannot already have with the direct dependency on expect

@nicolo-ribaudo
Copy link
Owner

I'm assuming the tests will pass on main 😄

@nicolo-ribaudo nicolo-ribaudo merged commit 220ea95 into nicolo-ribaudo:main Apr 27, 2022
@@ -1,18 +1,10 @@
/* eslint-disable import/extensions */

import mock from "jest-mock";
import { expect } from "expect";
import { jestExpect as expect } from "@jest/expect";
import snapshot from "jest-snapshot";
Copy link
Contributor

Choose a reason for hiding this comment

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

unused

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks! Fixed in 9b0a402

@fisker fisker deleted the addSnapshotSerializer branch April 28, 2022 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Support expect.addSnapshotSerializer()
3 participants