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

jest-emotion and snapshot-diff not playing nice #1898

Closed
DarkPurple141 opened this issue Jun 11, 2020 · 6 comments
Closed

jest-emotion and snapshot-diff not playing nice #1898

DarkPurple141 opened this issue Jun 11, 2020 · 6 comments
Labels

Comments

@DarkPurple141
Copy link

Current behavior:

jest-emotion doesn't appear to work as a serializer for snapshot-diff. Tried various permutations of the config and the test setup but it doesn't play nice. It's unclear if this is a problem with the docs or an API thing.

To reproduce:

Minimal repo:
https://github.com/DarkPurple141/jest-emotion-typescript-minimal/tree/master

  1. install dependencies and run jest

Expected behavior:

Environment information:

  • react version: 16.9
  • emotion version: 10.0.28
@Andarist
Copy link
Member

This reads to me as "it's not working" and nothing more - I have no idea what doesn't work though. Please provide a detailed explanation of how this behaves now and how would you like it to work - it's too complicated for me to understand your issue quickly without extra information.

@Andarist
Copy link
Member

Ok, I've managed to see what's your issue but it wasn't at all apparent where I should look for it as you have provided 3 different test files without pointing to a specific problem. app.test.tsx gave me an answer but this was the last place I was looking into.

It seems that toMatchDiffSnapshot just doesn't run through added custom serializers:
https://github.com/jest-community/snapshot-diff/blob/2d81e0a5ca04f682cebf13cf94c9dd369d4c81c7/src/index.js#L89
https://github.com/jest-community/snapshot-diff/blob/2d81e0a5ca04f682cebf13cf94c9dd369d4c81c7/src/index.js#L37
but it seems that you maybe can tell it to run through by using setSerializers:
https://github.com/jest-community/snapshot-diff/blob/2d81e0a5ca04f682cebf13cf94c9dd369d4c81c7/src/index.js#L113

I hope this answers your question, if not - feel free to comment back but I'm going to close this right now as I think it got answered.

@DarkPurple141
Copy link
Author

Hi @Andarist apologies for the vagueness of the request. Stemmed from a bit of a documentation void in snapshot-diff and was going around in circles a bit with different configs.

Why I raised an issue here was because the serializer as exposed by the default export in jest-emotion doesn't work with snapshot-diff. I had tried to use the setSerializers API method as per https://github.com/DarkPurple141/jest-emotion-typescript-minimal/blob/master/setup.ts, but it doesn't work.

I say it doesn't work but what's interesting is the overlap/difference between these two tests:

https://github.com/DarkPurple141/jest-emotion-typescript-minimal/blob/26b81627eaf490ca2c703e1bf2b6c2feb53e1bb8/src/app.test.tsx#L21

and

https://github.com/DarkPurple141/jest-emotion-typescript-minimal/blob/26b81627eaf490ca2c703e1bf2b6c2feb53e1bb8/src/app.test.tsx#L32

The output has the partially correct serialized styles hoisted from emotion (which implies the serialisation worked?) but only when it's renderered through react-test-renderer and then it also doesn't have the actual dom-tree.

I think the issue is probably with snapshot-diff, I'll pursue more there.

@Andarist
Copy link
Member

I've looked very quickly and some of my findings are:

  • snapshot-diff is not using by default builtin serializers, it only uses a wrapper around react serializer which is based on React elements, react-test-renderer, etc. So it doesn't work with JSDOM output
  • it also only works with old serializer API - the one using { test, print } interface when the new API is using { test, serialize }, this makes it impossible to reuse all current jest's builtin serializers because they are written using the new API.

@DarkPurple141
Copy link
Author

Yep that appears to be the underlying problem. Thanks!

@jcwrightson
Copy link

Is there a workaround?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants