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

refactor(deps)!: Remove duplexify dependency + Revamp Stream Implementation #2041

Open
danielbankhead opened this issue Aug 12, 2022 · 0 comments
Labels
api: storage Issues related to the googleapis/nodejs-storage API. next major: breaking change this is a change that we should wait to bundle into the next major version type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. web

Comments

@danielbankhead
Copy link
Contributor

danielbankhead commented Aug 12, 2022

Now that we've resolved the following:

We should remove duplexify. In all cases where it is used today, native Node.js Stream options, namely Transform, make more sense.

Today Duplexify streams are passed around to too many functions and have lots of random ‘side effects’ to keep track of; leading issues that are difficult to debug. A refactor using native Node.js streams and pipelines will make things much more straightforward and would shrink the overall codebase. For example, we can trust the 'close' event more in native streams than in the duplexify implementation; namely in the cases of requests (duplexify streams 'close' before a 'response' is received).

This is a breaking change as duplexify.Duplexify is exposed as a return type in some public APIs.

As an additional note, in the future we can move to the Web Streams API for web compatibility - moving to Node.js Streams first will make this step much easier as the Readable, Writable, and Transform implementations are similar.

@danielbankhead danielbankhead added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p3 Desirable enhancement or fix. May not be included in next release. next major: breaking change this is a change that we should wait to bundle into the next major version labels Aug 12, 2022
@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/nodejs-storage API. label Aug 12, 2022
@ddelgrosso1 ddelgrosso1 removed the priority: p3 Desirable enhancement or fix. May not be included in next release. label Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/nodejs-storage API. next major: breaking change this is a change that we should wait to bundle into the next major version type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. web
Projects
None yet
Development

No branches or pull requests

2 participants