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

support json output to allow for nicer interop with jest snapshot testing #16

Open
conorhastings opened this issue Dec 5, 2016 · 15 comments

Comments

@conorhastings
Copy link

Right now using preact-render-to-string directly you end up with double encoding in the snapshot output
screen shot 2016-12-04 at 8 02 07 pm

see this twitter thread -- https://twitter.com/stillconor/status/805567906310451200 -- for more details.

cc @developit , I'd be interesting in taking this on if you could provide high level guidance on how this would would be worked on.

@developit
Copy link
Member

developit commented Dec 5, 2016

@conorhastings That would be awesome - I've been pondering the best approach myself. Right now the renderer doesn't use an intermediary, so it's straight to string concat. I think the simplest approach would be to add a { json:true } option added that builds up objects instead of strings. It might end up being quite a few branches in the code though, not sure what to do about that. I guess another option would be to always render to Objects first, then if { json:true } isn't passed do the xml/html stringification, but that'd make this two-pass, which would be quite a bit worse performance.

@ruyadorno
Copy link

ruyadorno commented Apr 26, 2017

I have been doing just that in my current project using jest-serializer-html-string to serialize the html correctly

if that interests anyone I can put together a small boilerplate but should be fairly straightforward, just @developit example + jest snapshotSerializer config

@developit
Copy link
Member

that would be very useful @ruyadorno

@paranoidjk
Copy link

@ruyadorno great job 👍
our project(https://github.com/ant-design/ant-design-mobile) use jest snapshot test, if preact can works with jest snapshot test, then we will be more confident to use preact replace react

@ruyadorno
Copy link

I know you all have seen it in twitter but I'll leave the boilerplate link here for future reference:

https://github.com/ruyadorno/preact-jest-snapshot-test-boilerplate

@paranoidjk
Copy link

paranoidjk commented May 12, 2017

@ruyadorno @developit seems like the render result is consistent, but html string is not serialized as the same way of enzyme-to-json
ant-design/ant-design-mobile#1306

@paranoidjk
Copy link

paranoidjk commented May 12, 2017

@ruyadorno
as ant-design/ant-design-mobile#1306 ci result shownd,
The problem is jest-serializer-html-string not place html attribute at new line, so the react snapshot not be consistent with preact snapshot, I will create a PR to fix this.

@paranoidjk
Copy link

the difference is whether put each prop on a separate line.
from https://github.com/adriantoine/enzyme-to-json#user-content-serializer:

This is inspired by jest-serializer-enzyme, I first added a note to jest-serializer-enzyme but I then realised that the output is different, so it is not retro compatible with enzyme-to-json because it's using Enzyme debug helper which doesn't put each prop on a separate line.

@developit
Copy link
Member

@paranoidjk ah - you probably want to use the JSX string renderer:

import preactRender from 'preact-render-to-string/jsx';

preactRender(<foo a="b" c="d">bar</foo>)
/*
<foo
  a="b"
  c="d"
>
  bar
</foo>
*/

@ruyadorno
Copy link

@paranoidjk yeah, even though it might be valid html, it's def not the standard way html is usually rendered, I'd rather stick with the default options from the html beautifier within jest-serializer-html-string as that would make more sense for most people (including me).

@marvinhagemeister
Copy link
Member

@ruyadorno jest-serializer-html-string doesn't seem to match jest's rendering behaviour. Jest is more like the jsx renderer, which has the advantage of clearer diffs on attribute changes. Because each attribute is a single line it's much easier to see which ones are removed and added when multiple attribute changes are made for a single component.

I love your package though, keep up the good work 👍

@developit
Copy link
Member

Trying to think of the slimmest way to produce JSON output from the string renderer. It's the same flow, just seems awkward to have to have that many branches.

@whogood
Copy link

whogood commented May 30, 2017

@ruyadorno thanks for sharing your solution

@nathancahill
Copy link

nathancahill commented Oct 18, 2017

Just came across this thread, my fork of preact-render-to-string renders to JSON: preact-render-to-json and is compatible with Jest and React testing frameworks.

@ascorbic
Copy link

@nathancahill Posting this here, as you don't have issues enabled on your repo: preact-render-to-json doesn't work with hooks, because of preactjs/preact#1373

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

8 participants