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

Remove streaming content support? #133

Open
phated opened this issue Jun 30, 2017 · 6 comments
Open

Remove streaming content support? #133

phated opened this issue Jun 30, 2017 · 6 comments
Milestone

Comments

@phated
Copy link
Member

phated commented Jun 30, 2017

Streaming contents have made many things hard. I have some ideas about ways to replace some of the use cases and need to explore more to see if we can remove streaming contents completely.

@phated phated added this to the Future milestone Jun 30, 2017
@Marak
Copy link

Marak commented Oct 15, 2017

I'm a bit confused by this issue. If we remove streaming contents completely, wouldn't that break much of the vinyl / gulp ecosystem?

Also, if we remove streaming, wouldn't that mean that all files are going to be buffered into memory, or is that a vinyl-fs responsibility?

Any clarifications would be appreciated. I'm probably misunderstanding the issue.

@phated
Copy link
Member Author

phated commented Oct 15, 2017

@Marak this change would be a breaking change to the current Vinyl spec where the contents property can be a Buffer, Stream or null. However, it likely wouldn't break much of the gulp ecosystem because most plugins don't support streaming. The hope behind streaming contents was that more and more modules would support streaming over time (a good example being uglifyjs); however, over time, that hope has not held true and so plugins need to check if contents are streaming and most just error out.

This issue was really inspired by your desire to have a toJSON method, which cannot be supported if we allow for streaming contents and further compounded by the complexities of putting streams within streams (vinyl-fs, etc). I think we can come up with better solutions but it hasn't been explored yet.

This won't come until after gulp 4 which will include vinyl-fs 3.x/vinyl 2.x

@Marak
Copy link

Marak commented Oct 16, 2017

I agree that correctly implementing streaming modules for gulp eco-system is not super easy. A lot of the plugins I've seen don't bother to stream at all.

I'm still doing toJSON in my fork and it's working well. I've recently made some changes so fs.Stat properties are also serialized. Pretty much my use-case has been serializing vinyl instances to be sent over the wire. In some cases I don't care about contents when calling toJSON, I just need the metadata and stats, so I don't mind if I lose the stream or buffer reference.

@Marak
Copy link

Marak commented Oct 16, 2017

RE: #83

@phated
Copy link
Member Author

phated commented Oct 19, 2017

@Marak I'd love for better-stats to be properly serializable - node's fs.Stat leaves so much to be desired.

@phated
Copy link
Member Author

phated commented Sep 6, 2019

Also need to review https://www.npmjs.com/package/smart-buffer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

2 participants