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

Property matcher support #37

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Conversation

ethanresnick
Copy link

I understand that this code probably isn't ready to merge yet (e.g., it doesn't have tests or docs), but I wanted to at least open this PR in case others find it helpful.

Basically, it enables Jest 23 with property matchers, which make snapshot testing much more broadly applicable. The property matchers always go as the first argument (including when you're updating a snapshot with to.matchSnaphot(matchers, true)), and their values must come from the expect module that jest uses under the hood. I.e.,

import { any } from "expect";

// ...
expect(it).to.matchSnapshot({ id: any(String) })

ethanresnick and others added 26 commits September 20, 2018 17:22
Very powerful for snapshotting deep structures
Before, we were doing all kinds of broken things:

1. Undercloning. I.e., we were mutating deep objects in the user’s data
when a jsonpath matcher applied to them (because we only did a shallow
clone).

2. Overcloning. If the user didn’t have property matchers, we still did
an unecessary shallow clone on the data being snapshotted. This only
happened if it was an object without custom serializers, so it was
probably harmless, but still uneccessary.

3. We skipped all property matchers when a custom serializer was in
use, when we really only needed to log a warning.
Has breaking API changes
Prettier config, standardized import-sort, dependency updates
@naz
Copy link

naz commented Dec 8, 2021

This PR works great and would be super cool to merge! /cc @suchipi 🙏

@piranna
Copy link

piranna commented Apr 24, 2023

Can we get this merged? Also the same for update jest-snapshot dependency.

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.

4 participants