Skip to content

Serializers not working as expected #162

Open
@DarkPurple141

Description

@DarkPurple141

I've filed a mirror issue here: emotion-js/emotion#1898

Can't get the setSerializer API to work as expected.

I've created a minimal reproduction repo here explaining the issue in more detail:
https://github.com/DarkPurple141/jest-emotion-typescript-minimal/tree/master

Activity

DarkPurple141

DarkPurple141 commented on Jun 14, 2020

@DarkPurple141
Author

It appears the underlying issue is that this API integrates with the older jest.SerializerPlugin and so doesn't support a number of more current serializers. Is this something the maintainers are looking at fixing?

alistairjcbrown

alistairjcbrown commented on Jun 27, 2020

@alistairjcbrown
Contributor

the underlying issue is that this API integrates with the older jest.SerializerPlugin

Do you have any more details on this? That may be an issue, but I'm not sure it's the issue here.

From your repo - let's run through each test

  • Ideal - diffing two React components (not rendered) displays the component output with Emotion generated classes, but no style output
    • We're passing in React components which aren't rendered, so I don't think Emotion's serializer is picking it up. Instead, it's all being serialized by the React serializer that comes as default in snapshot-diff, and so we just get the component output
  • vanilla toMatchSnapshot() - working as expected (nothing to do with snapshot-diff)
  • Using react-rest-render and using snapshotDiff() func - these two tests I believe are essentially doing the same thing; diffing two rendered React components displays the style output but not the component output
    • This one's pretty much the opposite of the first; we're passing in rendered React components which Emotion's serializer is picking up. But Emotion's serializer only deals with rendering the styles, so we're seeing [object Object] for the component output.

The expectation was, that by adding the Emotion serializer that we'd also see styles being output in React components. However, serializers added through setSerializer are independent - the example in the docs/tests are either replacing the built-in React serializer (using React renderer) with Enzyme's serializer, or adding serialization support for some new type. In this case, we want to augment React component serialization to also deal with CSS-in-JS styles. The current setup means when adding the Emotion serializer, either it will run or the React serializer will run (depending on the result from test), not both. Which is what we're seeing from the snapshot data in your repository.

Proposed solution

  1. Update documentation for setSerializer with more example and clearer instructions on how this works
  2. Currently, the React serializer uses pretty-format to serialize a rendered React component, using the same default serializers as jest-snapshot. To have this work with Emotion, we'd need to provide the ability to enhance the React serializer to also pass Emotion's serializer into pretty-format. This would make the Ideal solution work as expected, with the React serializer taking care of rendering with react-test-renderer and pretty-format using all available serializers for generating the snapshot.
DarkPurple141

DarkPurple141 commented on Jun 27, 2020

@DarkPurple141
Author

Hi @alistairjcbrown thanks for getting back.

Sorry I did investigate further but didn't update this ticket.

The rub seems to be in the second of your proposed solutions. Essentially, the pretty-format call for React specific serialization is wrapped and is never updated (even by calling setSerializer you don't actually hook into the snapshot-diff's serializer.

I think there may be an additional issue thought with the fact that snapshot-diff uses the test() print() interface and newer libs now use test() serializer() see: emotion-js/emotion#1898 (comment)

alistairjcbrown

alistairjcbrown commented on Jun 28, 2020

@alistairjcbrown
Contributor

@DarkPurple141

the pretty-format call for React specific serialization is wrapped and is never updated (even by calling setSerializer

setSerializer is for adding new top-level serializers (e.g. a serializer for serializing React components rendered by Enzyme), not for enhancing the existing React component serializer. The proposed solution above is to "provide the ability to enhance the React serializer to also pass Emotion's serializer into pretty-format"; this would be new functionality and probably a new API to allow that existing serializer to be enhanced.

I think there may be an additional issue

I think you're right that that may be an issue as serializers without print() are added, but it sounds like it's unrelated to this specific issue on using the Emotion serializer; it looks like Emotion currently provide their serializer with the test() print() interface, which is why we're successfully seeing snapshot data in your repo, rather than getting an exception on missing print function.

We should probably create an issue to track updating snapshot-diff to support both print() and serialize(), as documented in the pretty-format plugins section: https://github.com/facebook/jest/blob/a8129a7e4520ec545e0ddf97048fbc9e8a573076/packages/pretty-format/README.md#serialize

added 2 commits that reference this issue on Jun 28, 2020
5bc5d52
783b640
added a commit that references this issue on Jun 30, 2020
1cfd88e
added a commit that references this issue on Jul 18, 2020
8fa0af8
alflennik

alflennik commented on Oct 2, 2020

@alflennik

I've been playing around with snapshot-diff and it seems like it can really simplify my tests - the only thing is, whenever I change the internal implementation of my component, even if it doesn't affect any functionality, all the emotion classNames break.

Screen Shot 2020-10-02 at 11 23 57 AM

It looks like the PR could potentially fix this issue! If it makes it in I can promise it will make my day 😜

thymikee

thymikee commented on Oct 2, 2020

@thymikee
Member

Ugh, I need to get back to reviewing that 😅 hopefully this weekend!

RazerM

RazerM commented on Aug 1, 2022

@RazerM

I was surprised to learn that snapshot-diff doesn't pass the serializers to pretty-format, I'd expect it to do what jest-snapshot does: https://github.com/facebook/jest/blob/03cf86f60c42036a183c4fecac24882a06835427/packages/jest-snapshot/src/utils.ts#L162-L175

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @alistairjcbrown@RazerM@thymikee@alflennik@DarkPurple141

      Issue actions

        Serializers not working as expected · Issue #162 · jest-community/snapshot-diff