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

Missing assets using the additionalStreams option #184

Open
asselin opened this issue Feb 22, 2016 · 4 comments
Open

Missing assets using the additionalStreams option #184

asselin opened this issue Feb 22, 2016 · 4 comments

Comments

@asselin
Copy link
Contributor

asselin commented Feb 22, 2016

If you use the additionalStreams option, gulp-useref may not stream through all of the assets. This is a timing dependent bug.

@asselin
Copy link
Contributor Author

asselin commented Feb 22, 2016

Updated an existing test to reliably repro the issue.

In debugging, there are a couple of problems I'm seeing:

  1. es.wait (https://github.com/jonkemp/gulp-useref/blob/master/lib/streamManager.js#L175) is not waiting for both streams to finish. It looks like it's a bug in the es code (or maybe, since it works with old style streams, in combining old and new style streams).
  2. The testcase makes an assumption on file ordering that is invalid

The testcase uses this fixture:

<!-- build:js scripts/combined.js -->
<script src="./scripts/this.js"></script>
<script src="./scripts/anotherone.js"></script>
<!-- endbuild -->

<script src="./scripts/renamedthat.js"></script>
<script src="./scripts/renamedyet.js"></script>

However, useref() won't include 'renamedthat.js' and 'renamedyet.js' in it's output, so as far as reorderTheStream is concerned, they are unsorted files (https://github.com/jonkemp/gulp-useref/blob/master/lib/reorderTheStream.js#L21). This means that whatever order the additionalStreams wind up resolving their files in will be the order they're added to gulp-useref's output stream.

I can take a crack at fixing this if you want, but my preferred way to fix the first issue would be to use a promise. Any objection to adding Q as a dependency?

For the second issue, what is the expected behavior in this scenario? How about if the 'renamed' block was surrounded by another build:js block?

@jonkemp
Copy link
Owner

jonkemp commented Feb 23, 2016

Yes, I would prefer not to use Promises here.

@jonkemp
Copy link
Owner

jonkemp commented Apr 16, 2016

I need more information here. The failing test in #185 passes once the file is added back to the stream. That seems to invalidate this claim that it is timing dependent. I also fixed the test so it is not order dependent because you are correct, the additional files are not being sent through in any particular order.

@asselin
Copy link
Contributor Author

asselin commented Apr 18, 2016

OK, I'll take a look at the updated code and see if I can reproduce the issue.

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

2 participants