-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Files order is not preserved in buffer mode #300
Comments
I don't usually suggest breaking things into separate tasks and relying on dependencies, but I feel like that would be the best way to deal with this case since it is a problem within modules from the ecosystem and not gulp itself (am I correct in this thinking?). |
@phated no this is just a feature of streams (or a flaw depending on the case) Order is not guaranteed with streams, but can be if you use something to specifically order them |
I'm planning a whole suite of tools for plugin authors to make streams easier. Right now things are pretty difficult. |
@nfroidure you can use sort-stream to sort your files if need be :) |
I think plugins should keep files in the same order for consistency with the streaming mode, but also because plugins like sort-stream can only sort files that expose a property allowing sorting and it is not always possible/available. But actually, i don't know how to do this without adding extra code to plugins. |
I found out that using stream mode + bufferstreams allows to keep files order while manipulating buffers. It could be possible to reproduce this behavior for buffer mode. |
Unfortunately just sorting the files or having an explicit ordering is probably not what's really desired in most cases. The most common use-case will probably be the desire to have the files come out in the order they went in, or maybe some permutation of that order. One way to achieve this might be a sequencing plugin: var sequence = require('gulp-sequence')();
gulp.src('**/*.js')
.pipe(sequence)
.pipe(uglify()) // Imagine uglify is async
.pipe(sequence.restore()) In order for this to work, the original sequence function would have to add a field to the vinyl object like 'order' or similar, which of course would be destroyed by any plugins that create new file objects. Any thoughts on this idea and/or solutions to the new file problem? |
@hughsk The possible problem with sort-stream is that the stream produced by gulp.src already has the file attached. I can't tell whether the actual bytes have been loaded at that point or not, but if so, that's an expensive thing to buffer in memory while you're queuing for a sort operation. A custom sort option would be useful, but unless I'm wrong about what's happening with the files, then it needs to be done either in glob itself (see #75) or in glob-stream before the file gets attached. |
@nfroidure, what are you actually trying to do? Folks are debating whether you need the files loaded, how costly it is to load them before sorting them, etc... and what you've indicated is I'm not exactly sure what is driving the various comments. Every plugin operates on a stream of vfs File objects, so regardless of how long that plugin takes, there's a stream coming of files coming out at the end. If the output comes out in an order that isn't what you want, then that would be a fine time to sort it. If for some reason you have a plugin that is randomly grabbing files from a stream or stream, then it sounds like bad design. If it's not random, then such behavior sounds complicated, but in theory there may be reasons. So I'm just not real clear on the problem that's being discussed. Is it that there are plugins that change the order of files, or is it that you are unable to change the order yourself? I guess I sort of understand @nlwillia's comment about needing the file content, even though I'm not sure this is a glob or glob-stream issue. (I guess I think he is wrong, as he mentioned was possible, about what's happening with the files). If you want to delay loading the content until you've sorted, then you can set {read:false} on gulp.src, then sort-stream it, then (I think this will work, haven't verified)...
Obviously that's supposed to be internal. An enterprising developer might use it to create a plugin, though, and that plugin might take options like "buffer only files that are currently null") but in this case if you've explicitly avoided reading the files prior to sorting, then you know you're in need of the full monty. |
I just reread this and @contra is saying that streams are not guaranteed to preserve order, and now I'm totally confused. We're talking about Node transform streams here, right? One has to fairly deliberately reorganize objects within a transport stream (including buffering of said objects so that you can emit them out of order) in order to change the objects' order... so maybe what we're really talking about is that some plugins change the order and we don't like that they do that? That seems a hard problem to fix without changing the plugin, and not a problem that I would classify as worthy of getting a "bug" tag on a gulp issue... |
You don't expect a plugin to have those side effects. I think this is a bug if we consider that Gulp should manage this for plugins author. If not, this must be documented in order to warn plugin authors of this possible side effect and maybe add a recipe to show how to implement plugins that works asynchronously with buffer contents. For me the problem comes from the buffer mode design. I think it should use callbacks to set/get buffers instead of setting/reading them synchronously. By doing this, virtual files would traverse the whole plugin pipeline, set the various callbacks and then, compute them asynchronously. This is the behavior of the stream mode. It could be made with promises. A plugin could look like this: var Transform = require('stream').Transform;
var plugin = new Transform({objectMode: true});
plugin._transform = function(file, unused, cb) {
file.contents = file.contents.then(function(buffer) {
// Do something with buffer
return buffer;
});
cb(null, file);
}; |
@nfroidure I agree, this needs to be documented and we need to make sure the tools exists to solve this. @stu-salsbury Stream ordering is not ensured if async operations take place in a transform |
@contra is this still a thing? |
@phated yeah, its a fundamental issue with async transform streams |
Removed the bug and enhancement tags, as this is mainly a documentation issue. Also, added help wanted tag if someone wants to write this up. |
I am new to the community and wish to contribute. I can work on the documentation. |
@piyushdubey I don't have any specific advice but generally, just submit doc changes as PRs and we can discuss back-and-forth. |
@phated Great! Should the docs be added under FAQs? |
@denieler gulp-filter is not part of gulp itself ;) |
@nfroidure I thought it is opened issue in gulp-filter repo |
When the buffer mode is used and some plugins are doing some async stuffs on buffers, it leads to unordered files in the stream.
Creating a plugin that works asynchronously on buffers and preserve the inputed files order is over complicated.
The problem doesn't appear for streaming mode since all pipes are done immediatly.
The text was updated successfully, but these errors were encountered: