gulp 5 seems to break existing streaming plugins

[fluffynuts]

What were you expecting to happen?

After upgrading from gulp 4.x to gulp 5, I was expecting my built-in plugin to "streamify" async functions so they can be put in the gulp pipeline without having to write bespoke plugins for everything to work as before.

What actually happened?

Tests around streamify hang. Breaking into the code, I see that the original transform creation is called, the transform is passed back, and the body of the transform is never executed.

Please give us a sample of your gulpfile

See example at https://github.com/fluffynuts/gulp-streamify - streamify.spec.ts has the calls to gulp, so you can npm test to see the failures.

Whilst I've pulled this out into its own repo to make it easier to use to repro the issue, this plugin is not published, and may never be, as it's consumed internally on another project.

I've included the original function as-is, using through2, and an "updated" one based on the docs at https://github.com/gulpjs/gulp/tree/master/docs/writing-a-plugin - both don't work.

Please provide the following information:

• OS & version: Windows 11 23h2 (22631.3155)
• node version (run node -v): 18.18.2
• npm version (run npm -v): 9.8.1
• gulp version (run gulp -v): 5.0.0

[3cp]

I saw similar issue with merge2 which is a simple PassThrough stream.

[fluffynuts]

I've updated the title and uploaded a new test scenario because it turns out async is not the problem here - it looks like gulp 5 doesn't cope with any existing stream-based plugins, perhaps streamx (according to https://medium.com/gulpjs/announcing-gulp-v5-c67d077dbdb7) doesn't play well with other streams? It certainly seems like it should.

Changing streamify.ts to pull Transform from streamx causes typescript errors in the consumer:

[phated]

At a glance, I can't see what is causing your problems. The entire vinyl-fs test suite runs with streamx, readable-stream, and core streams so we know they worked when used "normally" (for whatever that means...)

[phated]

Oh, nevermind. I wasn't looking at your tests, just the transform implementation. You aren't using streams correctly. If you don't sink your streams, such as with a Writeable then they won't be flowing.

I'm going to close this but let us know if you see a different issue when you sink the streams.

[fluffynuts]

ok, whilst I try to figure out what I have to do to "sink correctly", please take note that:

1. this worked 100% fine in gulp 3 and 4 (and works again when I downgrade from 5 to 4). This is quite a breaking change which will likely cause many plugins to fail, and it would appear to be a change in behavior from the expectation from node streams? Or was the prior behavior buggy?
2. I've followed the example as per https://github.com/gulpjs/gulp/tree/master/docs/writing-a-plugin as well, so if I'm missing something from there, then, at the very least, the documentation could likely be improved.

[fluffynuts]

Looking at the source for dest in vinyl-fs, it looks like I can use the lead package - calling that function around the stream I was about to return to "get it started", and yet the tests at https://github.com/fluffynuts/gulp-streamify still fail. I'd really appreciate some help.

@phated until this issue is actually resolved (ie a user of this project can use it as they understand the documentation), please re-open.

[phated]

No, our lead package is made to sink an entire pipeline. You can't just wrap it around a Transform stream and shove it in the middle of the pipeline.

I'm happy to help but I don't have the bandwidth to teach people streams from scratch. Sorry 😔

[fluffynuts]

I'm not looking to be taught "streams from scratch". Clearly I knew "enough" to implement something which has worked across gulp 3 and 4, for a very long time. However, I'm always aware that there's much I don't know - I'm trying to figure out what I don't know that, in this situation, causes failure only on gulp 5.

Remember: this worked 100% fine before gulp 5. Something in the streams implementation has changed. In that case, my existing knowledge of node streams (which may be incomplete) is also now (apparently) invalid.

Also, dest() from vinyl-fs doesn't wrap the entire pipeline in a lead call - it wraps it's result in lead(...) ( https://github.com/gulpjs/vinyl-fs/blob/ad025afed42f70a064801180185b500235d5272a/lib/dest/index.js#L44 ) - so now I'm confused because:

1. Streams are no longer behaving like before, but apparently I should expect that, because I need to be taught how to use streams?
2. You've told me to wrap the entire stream in lead, but dest from vinyl-fs doesn't - I followed the example in there to try to resolve this, and apparently that's not correct?
3. Even if I sink the test stream via .pipe(gulp.dest("/somewhere")), the internal code of the transform is not called. So do I understand that now a consumer would have to wrap the entire gulp pipeline they made in lead(...)? That doesn't seem right.

[fluffynuts]

I've also added another example to the repo: looking at the example code at https://github.com/gulpjs/lead, I've written a test which first does exactly what the example does (and that works) and then attempts to pipe the result of gulp.src() into the exact same transform the exact same way, and it fails as do my other tests. Everything I've read says that if you pipe into a writable stream, that's your sink, and Transform is a Duplex, which is writable, so I'm obviously missing something fundamental, and if the behavior I'm seeing is expected, then prior behavior was buggy, and I was relying on it?

[3cp]

Even if your gulp-streamify has issue, I don't think it should take all the blame. Because it worked with gulp 4 which uses standard nodejs stream.

Besides merge2, I also found gulp-typescript stopped working in gulp 5.

My guess is maybe there is an edge case in streamx (which gulp migrated to) that no unit tests have covered.

The open issues on streamx seem getting no attention. https://github.com/mafintosh/streamx/issues

[3cp]

Saying it's all 3rd party plugin issue, that sounds bit premature.

The fact is they worked in gulp 4, failed in gulp 5. If this's not caused by a deliberate breaking change, we should call this a regression.

Regression should be addressed.

[phated]

it wraps it's result in lead(...)

Yes, this is because lead sinks a stream that is in the Writable position. You wrapped a stream in the middle of your pipeline with lead which will do nothing because it knows it is not in the Writable position. Instead of looking at our implementation of dest() (which has a bunch of hacks to work around bad stream usage), you should look at the tests to see how to properly write Source -> Transform -> Sink pipelines.

[fluffynuts]

I'll gladly go read code to see what's up, but I would really appreciate a nudge in the right direction with "look at the tests": which project? vinyl-fs? gulp-cli? Even better - point me at what you consider to be 100% proper usage in a specific test so I may learn.

I agree with @3cp though - this worked before, feels like a regression, shows up elsewhere.

[3cp]

I tried to debug merge2, so far have not figured out what's the edge condition. I will continue to try to find a minimum way to reproduce it.

gulp-typescript is too big for me to debug, but it's much widely used.

[phated]

In working on the glob-stream bug: gulpjs/glob-stream#127 I realized that the fundamental issue here might be that the "Sandbox" module is probably producing a path outside the cwd so nothing is globbed. That issue will be fixed by gulpjs/glob-stream#131.

[fluffynuts]

@phated I think you may be correct - I've updated my side to keep only my original function and use Sandbox's .run method to run within the sandboxed folder - and all passes (of course, then I don't need the leading ${ sandbox.path } in the filter ${ sandbox.path }/* any more either.

I've subscribed to PR #131 and will try again when that's been merged in (: