Image comparison of snapshots crashes node v9.2.0

[Kraxxis]

Node v9.2.0 crashes when attempting to use this jest matcher

/usr/bin/node[24137]: ../src/node_zlib.cc:189:static void node::{anonymous}::ZCtx::Write(const v8::FunctionCallbackInfo<v8::Value>&) [with bool async = false]: Assertion `Buffer::HasInstance(args[4])' failed.
 1: node::Abort() [node]                                                                                                               
 2: node::Assert(char const* const (*) [4]) [node]                                                                                     
 3: 0x55f886814175 [node]                                                                                                              
 4: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) [node]                         
 5: 0x55f8861002df [node]                                                                                                              
 6: 0x55f88610082f [node]                                                                                                              
 7: 0x1755837042fd

After doing some research and investigation, the problem appears to be the pngjs package and how the package interacts with the internal zlib changes to node. Open issue tracking is here: pngjs/pngjs#95

[anescobar1991]

Thanks for bringing this to my attention. I’ll be tracking the issue on the pngjs package and evaluating if there is anything I can do.

[Kraxxis]

On my project, I am just getting screenshot tests integrated for the first time, so having a blocker like this right when I am getting the test environment set up was very disruptive for me. I needed to move forward, and I have found a small, temporary work around for now.

I made a local copy of this Jest Matcher in my project, so I could change the pngjs dependency to its fork, pngjs-nozlib. This fork was a drop in replacement. Doing this, I was able to work around this issue for now, although I have no idea how much of an impact this change will have with respect to compression, speed, or accuracy.

EDIT: Sorry about the below misclick

[adrianmcli]

I’ll be tracking the issue on the pngjs package and evaluating if there is anything I can do.

@anescobar1991 any news on the progress of this bug? I am trying to deploy visual regression testing with this library, but I don't want to break the team's builds.

Perhaps we should integrate Kraxxis' workaround into this project?

[anescobar1991]

@adrianmcli The only thing that can be done from the Jest-image-snapshot side would be to use a different library than pngjs (maybe using the solution @Kraxxis has) but I need to evaluate our choices first and make sure no regressions are caused by the switch. I’ll do that today.

[anescobar1991]

Assuming that pngjs-nozlib really does work out of the box with node 9.x.x as @Kraxxis mentions (I have not tried myself) I think the right thing to do here is to open a PR to pngjs with the changes from pngjs-nozlib. It looks like the only thing pngjs would lose from that solution is backwards compatibility with node 0.10 which I am thinking the pngjs author may be okay with at this point as long as it is versioned appropriately.

Otherwise I don't feel comfortable using pngjs-nozlib in jest-image-snapshot as there is no guarantee of maintenance (as evidenced by fact that https://www.npmjs.com/package/pngjs-nozlib does not even point to the correct github repo). Aside from that sadly there are not really any pngjs alternatives available that I could find.

@Kraxxis or @adrianmcli do you mind looking into this?