Breaking: Use cloneable-readable to clone streams - possibly breaking (closes #85)

[phated]

Don't Merge

@mcollina was this what you were thinking for the usage of cloneable-readable in the #85 discussion? I'm not sure if I implemented with the same vision, any guidance or corrections would be really helpful.

[mcollina]

Yes, exactly. It should be equivalent to the previous approach, but without the memory issues. Plus, it keeps the number of references tracked down.

Let me know how I can help you further, cloneable-readable is an experiment: let me know if you plan to go ahead with that route, I'll keep that up to date and bump it to v1.0.0.

[phated]

@mcollina would you have any suggestions on tests I could add to have further coverage on this?

[mcollina]

Are you sure you need this? I would avoid, given you are already wrapping on demand https://github.com/gulpjs/vinyl/pull/99/files#diff-168726dbe96b3ce427e7fedce31bb0bcR85.

[mcollina]

Forget it, it's needed, I haven't looked at this for a bit of time.

[mcollina]

I've add a test in #102.

[phated]

@mcollina I noticed you are handling the on('data') case, but it doesn't seem to handle on('readable'). Thoughts?

[mcollina]

@phated just fixed, update to v0.3.0.

[phated]

@mcollina sorry to keep bugging you, but does the test I added at a4c3e14 look correct for readable events? Thanks!

[mcollina]

@phated yes, it's correct.

[mcollina]

This would be a problem, we need to fix it: mcollina/cloneable-readable#1

[phated]

Thanks for all the help on this @mcollina - I'll be bumping major with this change to get further testing in the wild. Once we feel comfortable with cloneable-readable, I'd like it to get bumped to 1.0