Fix underlying shared fs.read array

[yawlhead91]

About

Quick fix to the to a underline is issue where if you run critical in a loop, recursive function or through gulp can cause this error.

Unhandled rejection Error: Error: File.contents can only be a Buffer, a Stream, or null.

The reason for this is that when multiple calls to node fs methods in our case fs.readFileAsync are made this can result in Buffers that share a single underlying ArrayBuffer. So basically if you look here .. https://github.com/addyosmani/critical/blob/master/lib/file-helper.js#L340
we where trying to assign a array to the file.content which is why this error was appearing.

This is a vey small noice error, that probably rarely happens, but an issue is still is.

I added a small test that runs the critical in a loop twice this is not really sufficient as in my local case I was running critical over dozens of pages.

If you see here my production build I have run it on a few of my files which works perfect unlike before.

=========================================================

Ps. If you need any help on an issues I am more than happy to help

[bezoerb]

Thanks for working on this @yawlhead91 :)
Good catch. The PR looks good so far! I'll take a deeper look this evening!

[bezoerb]

@yawlhead91: The test succeeds even without your fix. Even if i'll add > 20 entries. Were you able to ensure that .pop() returns the correct value?

[yawlhead91]

@bezoerb yes in my case I have tested it locally quite a bit with success. But you are right about the test, as i said In my other comment I will spend some time this weekend getting tests to you reproducing the issue. It is proven quite hard to reproduce the issue in mocha. My apologies about the other tests. 👍

[bezoerb]

@yawlhead91 any updates on this?

[matteodelabre]

I’m experiencing the same issue. I noticed that it always happens when postcss-uncss is required in the gulpfile. Here is a minimal setup for reproducing the bug. After extracting, you just need to run npm install then npm start. You’ll notice that if you comment the line requiring postcss-uncss in the gulpfile, everything works again.

(Node version 8.2.1, npm version 5.3.0.)

[bezoerb]

Fixed in e9761a7 Switching from promisify(fs) to fs-extra already did the job

[bezoerb]

@matteodelabre: Thanks for the reproducing setup 🎉