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

Use PassThrough stream instead of through2 #11

Merged
merged 2 commits into from
Jun 17, 2015
Merged

Use PassThrough stream instead of through2 #11

merged 2 commits into from
Jun 17, 2015

Conversation

shinnn
Copy link
Contributor

@shinnn shinnn commented Jun 17, 2015

We don’t need to use through2 in this case. We can use just a bare PassThrough stream directly.

I introduced readable-stream module for more compatibility, instead of using the built-in stream. (ref: Why I don't use Node's core 'stream' module)

We don’t need to use through2 in this case. We can use just a bare
PassThrough stream.

I introduced readable-stream module for more compatibility.
@scottcorgan
Copy link
Owner

What are the benefits of readable-stream over through2? Curious before I make this change.

@shinnn
Copy link
Contributor Author

shinnn commented Jun 17, 2015

@scottcorgan

  1. readable-stream is one of the dependencies of through2. It means that directly using readable-stream reduces the package file size.
  2. Using a language built-in function instead of a user-land library is more flexible and easier to understand. People can contribute to this project without knowing what through2 is/does.

@scottcorgan scottcorgan merged commit 601f707 into scottcorgan:master Jun 17, 2015
@scottcorgan
Copy link
Owner

Merged and published as 1.3.1.

I usually default to user land modules, but in this case, it seems like a good idea to go core.

@mattdesl
Copy link
Collaborator

Not a huge deal, but this increases tap-dev-tool's compressed bundle size from 117kb to 143kb. Most likely because a lot of modules are using through2, and not a lot are using PassThrough (which honestly I didn't realize even existed! 😆 )

@scottcorgan
Copy link
Owner

@mattdesl is that something that we should addressed?

I've always just used through2 as a default when creating streams.

@shinnn shinnn deleted the remove-through2 branch June 17, 2015 21:32
@shinnn
Copy link
Contributor Author

shinnn commented Jun 17, 2015

The simplest way to reduce package size is just using the built-in stream module.

var PassThrough = require('stream').PassThrough;

However, the problem is, the Streams version differs depending on the Node version. For example the latest io.js includes Stream3 but node v0.10 includes Streams2. This would causes some unexpected behaviors especially in node v0.8 which includes Streams1. That is why I introduced readable-stream.

If you want to guarantee a stable streams base, regardless of what version of
Node you, or the users of your libraries are using, use readable-stream only and avoid the "stream" module in Node-core, for background see this blogpost.

@shinnn
Copy link
Contributor Author

shinnn commented Jun 17, 2015

Most likely because a lot of modules are using through2

Yes, and most modules still use through2 v0.6.x which depends on the outdated readable-stream, though v2.0.0 has been released.

If all the module authors upgrade through2 to v2.0.0 which depends on the latest readable-stream, npm will de-duplicate them well, because tap-out depends on the latest readable-stream.

@mattdesl
Copy link
Collaborator

I don't think it's a huge problem in my case since the tool is typically only used during development.

@scottcorgan
Copy link
Owner

Cool.

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