search

max-mapper / filereader-stream

Read an HTML5 File object (from e.g. HTML5 drag and drops) as a stream.
BSD 2-Clause "Simplified" License
102 stars 18 forks source link

Respond to backpressure suggestions #6

Closed konklone closed 8 years ago

konklone commented 9 years ago

This updates filereader-stream to listen to backpressure suggestions from downstream.

This is my first meaningful stream work, and I'm prepared to be told I've done something silly. But it seems to work in empirical testing. Without this patch, filereader-stream has no way to be told to stop reading a very large file into memory.

I haven't included a test, mostly because I think it would take me a long time to figure out how to create an anonymous write stream that establishes a high watermark. Maybe it's simpler than I think? Suggestions welcome.

max-mapper commented 9 years ago

looks like your editor has some aggressive whitespace stuff turned on

max-mapper commented 9 years ago

also this looks good but honestly this module should just get rewritten to use require('through2') and we'd get this for free

konklone commented 9 years ago

Yeah, my editor auto-kills trailing whitespace on save. https://github.com/maxogden/filereader-stream/pull/6/files?w=1 will view the diff without whitespace.

I'm not quite to the point of grokking through2 yet, though I've tried a few times tonight.

konklone commented 9 years ago

I just updated this PR with a tiny commit, dc4a320, that makes the reader emit pause and resume events when those events occur. Now that the stream supports backpressure management, those events are more frequent and more important.

konklone commented 8 years ago

Is this PR likely to be merged?

max-mapper commented 8 years ago

yes, just got lost in my inbox sorry

On Sunday, November 1, 2015, Eric Mill notifications@github.com wrote:

Is this PR likely to be merged?

— Reply to this email directly or view it on GitHub https://github.com/maxogden/filereader-stream/pull/6#issuecomment-152841513 .

Sent from my phone

max-mapper commented 8 years ago

@konklone I ended up rewriting the module. 1.0.0 is out now