scottcorgan / tap-out

A different tap parser
MIT License
23 stars 28 forks source link

Use PassThrough stream instead of through2 #11

Closed shinnn closed 9 years ago

shinnn commented 9 years ago

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)

scottcorgan commented 9 years ago

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

shinnn commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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! :laughing: )

scottcorgan commented 9 years ago

@mattdesl is that something that we should addressed?

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

shinnn commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

scottcorgan commented 9 years ago

Cool.