-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add unpipe method #64
base: master
Are you sure you want to change the base?
Conversation
Curious, what is your usecase for unpipe? I haven't found any, since is why I haven't added it, but can be convinced |
Yes, I never use it until today. I'm working on a re-implementation of aws-lambda-fastify and I'm building a Readable Stream with streamx to simulate the http.IncomingMessage. So the tests are not passing because of this: https://github.com/fastify/fastify-multipart/blob/master/index.js#L500 (first time I saw someone using unpipe) |
btw I know that I can just extend from http.IncomingMessage and use the node Readable stream but with streamx I get a performance improve of almost 20k of req/seq more. The stream state lifecycle in streamx is more simple and faster. |
I don't like the unpipe but maybe we can added only for compatibility support. |
Okay nice! I can be convinced :) |
|
06a4c46
to
99dd12a
Compare
Is there a better/different implementation/option for unpipe? I am using it in one of my projects and it is preventing me from upgrading to vinyl-fs 4. |
Sounds like your code is just written incorrectly. |
How so? unpipe is a valid stream method. |
Hi @mafintosh, this is my first attempt to add the
unpipe
method. #11