-
Notifications
You must be signed in to change notification settings - Fork 621
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
fix(expect): support expect.addSnapshotSerializer
#6173
fix(expect): support expect.addSnapshotSerializer
#6173
Conversation
The error about typo checking has submitted in this pr #6171. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6173 +/- ##
==========================================
- Coverage 96.51% 96.48% -0.03%
==========================================
Files 538 535 -3
Lines 41276 41204 -72
Branches 6162 6160 -2
==========================================
- Hits 39837 39756 -81
- Misses 1395 1404 +9
Partials 44 44 ☔ View full report in Codecov by Sentry. |
expect/expect.ts
Outdated
* `expect.addSnapshotSerializer` adds a module that formats application-specific data structures. | ||
* | ||
* For an individual test file, an added module precedes any modules from snapshotSerializers configuration, | ||
* which precede the default snapshot serializers for built-in JavaScript types and for React elements. |
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'm removing for React elements
part as that phrase feels strange in this repo.
@@ -915,3 +915,5 @@ export interface OldSnapshotPlugin { | |||
) => string; | |||
test: Test; | |||
} | |||
|
|||
export type SnapshotPlugin = NewSnapshotPlugin | OldSnapshotPlugin; |
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.
Is OldSnapshotPlugin
still common? How about only supporting NewSnapshotPlugin
?
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.
Yes, I checked some serialize plugins, they still use OldSnapshottPlugin
type definition, like:
jest-serializer-vue
jest-serializer-html
jest-serializer-react-helmet
jest-snapshot-serializer-ansi
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 enough
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.
LGTM. Thanks!
Part of #3964, support
expect.addSnapshotSerializer
api.