nodejs-storage icon indicating copy to clipboard operation
nodejs-storage copied to clipboard

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

Open danielbankhead opened this issue 2 years ago • 0 comments

Now that we've resolved the following:

  • https://github.com/googleapis/nodejs-storage/issues/1915
  • https://github.com/googleapis/nodejs-storage/issues/1916

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 avatar Aug 12 '22 19:08 danielbankhead