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

Investigate how to generate snapshots without using debug() #1

Closed
rogeliog opened this issue Dec 6, 2016 · 8 comments
Closed

Investigate how to generate snapshots without using debug() #1

rogeliog opened this issue Dec 6, 2016 · 8 comments

Comments

@rogeliog
Copy link
Owner

rogeliog commented Dec 6, 2016

No description provided.

@sapegin
Copy link

sapegin commented Dec 6, 2016

Maybe just use enzyme-to-json? Should fix #2 as well.

@rogeliog
Copy link
Owner Author

rogeliog commented Dec 6, 2016

I really like the developer experience that the serializer provides, it makes it really easy to use for the end user and comes with a really little amount of cognitive overhead.

This package used to provide something to the end user that enzyme-to-json was not providing, but @adriantoine added a serializer to enzyme-to-json a few hours ago 👍

Given that the community is already using enzyme-to-json I have the feeling that pushing forward with jest-serializer-enzyme might just cause fragmentation and hurt the community rather than helping out. On the other hand I like the idea of having a package like this one, that the only thing that provides is a serializer.

I think that it would be great if all Jest serializers follow some kind of convention like jest-serializer-{name}. Imagine if we had stuff like jest-serializer-cycle or jest-serializer-vue, that would be awesome :)

I see a couple of paths that we could take in order for the user to be able to consume jest-serializer-enzyme:

  • Make jest-serializer-enzyme depend on enzyme-to-json
  • Diverge completely from enzyme-to-json 😞
  • Get rid of this repo and make enzyme-to-json also publish jest-serializer-enzyme

cc: @cpojer I would love to hear your thoughts on this 😄

@sapegin
Copy link

sapegin commented Dec 6, 2016

Make jest-serializer-enzyme depend on enzyme-to-json

That’s what I was suggesting ;-)

@cpojer
Copy link

cpojer commented Dec 6, 2016

Yeah I agree it may be best to continue using enzyme-to-json. I'm sorry @rogeliog but your project made snapshot testing with enzyme a lot better – you had a simple idea, so thank you very much :) Hope to see you around on PRs for Jest one day :)

@adriantoine
Copy link

Hi!

To give an update, the serializer I published yesterday wasn't working at all... but I fixed it this morning (v1.4.2) 😊 It's now usable and I made unit tests to make sure it keeps working.

Also thanks @rogeliog for bringing the idea of an Enzyme serializer and this repo helped me a lot to figure out how to implement a serializer. I have to say I wasn't catching up with Jest releases so I missed that serializer update, but as @cpojer said it's hard to keep up nowadays. As I said in my README, the only reason I made a serializer is that the output of jest-serializer-enzyme is different from the one of enzyme-to-json, so it will be easier for people to just use an enzyme-to-json serializer.

Get rid of this repo and make enzyme-to-json also publish jest-serializer-enzyme

I agree that a name convention for serializers would be great, especially when you have to search through NPM or GitHub, so I'm happy to publish a serializer under an conventional name, but who decides what the convention is?

Thanks again @rogeliog and feel free to submit a PR to enzyme-to-json!

@cpojer
Copy link

cpojer commented Dec 7, 2016

Thanks @adriantoine and @rogeliog. This is best open source collaboration in action: don't get too attached to projects but rather stick with ideas and share them. I'm really happy with the outcome of this.

This also reminds me, we should make serializers follow a resolution algorithm so that the config isn't so annoying. I'll send a PR for this!

@cpojer
Copy link

cpojer commented Dec 7, 2016

jestjs/jest#2244

@rogeliog
Copy link
Owner Author

rogeliog commented Dec 13, 2016

@adriantoine @cpojer, I agree that we should merge into a single project.

  • I removed all the implementation code from this project to avoid confusion and added enzyme-to-json as a dependency. Now the only thing that this project is doing is delegating everything to enzyme-to-json.
  • I released 1.0.0 with the enzyme-to-json integration just to leave this npm package in a better spot
  • Moving forward, I think that best thing for the community is getting rid of this GitHub repo and do everything from enzyme-to-json

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

No branches or pull requests

4 participants