search

jonkemp / gulp-useref

Parse build blocks in HTML files to replace references to non-optimized scripts or stylesheets.
MIT License
705 stars 93 forks source link

Fix for issue #176, useref doesn't process all assets #182

Closed asselin closed 8 years ago

asselin commented 8 years ago

Depending on how much buffering was available in the gulp pipe, some assets would not be sent to the stage of the pipe. Root cause is an error in API usage. emit('end') was being called directly, instead of calling the transform stream's flush callback function.

Fixes #176

jonkemp commented 8 years ago

Can you add a test that fails without this fix?

asselin commented 8 years ago

I can take a crack, but it won't be today.

One way to test this is to pipe useref into a through2 stream that collects the filenames in the transform, and then in the flush function, compares the collected filenames to the list of expected filenames. If the transform function inserts a delay before calling the callback, the files will queue up in useref, and not all of them will be passed to the through2 stream before it's flush is called. To simplify, I'd also configure the through2 stream to have a small buffer (default IIRC is 16) so that you don't have to run useref with input that will create a ton of files.

jonkemp commented 8 years ago

Thank you. And thanks for pointing this out. I'll take a look as well when I have some time.

asselin commented 8 years ago

I fixed a few things in the PR update:

  1. The tests were not consistent in how they compared filename paths, so the tests failed on Windows due to / vs \
  2. The plugin didn't reinitialize some global variables when called again, so if a test failed, subsequent tests would timeout.
  3. Added the test that shows how the assets wouldn't all get added to the stream if the buffer filled up.

Note: I also found a similar bug with the additionalStreams option. I'll open a separate bug on that one.

jonkemp commented 8 years ago

Thanks! Can you squash your commits?

Edit: Before you squash, see my comments below.

asselin commented 8 years ago

Squashed and pushed.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.07%) to 97.368% when pulling b0326e884aa90b5f2fcae7dd67651883b79a1cd4 on PointSource:feature/fix-missing-assets into a5594dcc972b13447b1fddfff0cf621e6ace003a on jonkemp:master.