nodejs / readable-stream

Node-core streams for userland
https://nodejs.org/api/stream.html
Other
1.03k stars 227 forks source link

fix: add string decoder as a dependency #522

Closed jeswr closed 1 year ago

jeswr commented 1 year ago

Adds string_decoder as a dependency as it is imported here.

The string_decoder package happened to already be a devDependency of readable-stream and hence was not caught as missing by the recently added webpacking tests which have access to the devDependencies. Ideally a test should be added where they do not have access to these.

It appears to me that this used to be a dependency but that was removed in https://github.com/nodejs/readable-stream/commit/81c52641299e5ea3a5e37179967bb13691cad081#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519 - it is not clear to me why as a comment was made about this at the time https://github.com/nodejs/readable-stream/pull/471#discussion_r862719133.

mcollina commented 1 year ago

Can you skip those failing jobs in our workflow file? Unfortunately GHA is broken on those old runtimes.