Skip to content

Conversation

@Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jun 30, 2021

This package was only being used in tests. The tests are always run in Node.js, so I switched them to use the built-in Node.js stream API instead.

I'm guessing this package was originally included to allow using this package in either the browser or Node.js. But it wasn't setup that way. I did consider using readable-stream in place of stream to make browser compatibility easier for consumers, but that would have required more effort to get working, as the version of readable-stream in-use was outdated and incompatible with this package. Plus it's fairly common for bundlers to replace built-in modules like stream on-the-fly so it would have been of marginal benefit.

This package was only being used in tests. The tests are always run in
`Node.js`, so I switched them to use the built-in Node.js stream API
instead.

I'm guessing this package was originally included to allow using this
package in either the browser or Node.js. But it wasn't setup that way.
I did consider using `readable-stream` in place of `stream` to make
browser compatibility easier for consumers, but that would have
required more effort to get working, as the version of
`readable-stream` in-use was outdated and incompatible with this
package. Plus it's fairly common for bundlers to replace built-in
modules like `stream` on-the-fly so it would have been of marginal
benefit.
@Gudahtt Gudahtt requested a review from a team as a code owner June 30, 2021 05:12
Copy link
Contributor

@ryanml ryanml left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++

@Gudahtt Gudahtt merged commit a18b877 into main Jun 30, 2021
@Gudahtt Gudahtt deleted the remove-readable-stream branch June 30, 2021 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants