From 6e2a21d7eae0b730395016cb9bf91ee853d804ec Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Sun, 20 Apr 2014 17:00:01 -0400 Subject: [PATCH] Don't exert backpressure after the first push. Before this commit, BaseReadableStream would immediately exert backpressure (i.e. return `false` from `push`), even if the buffer was empty at the time of being pushed into. After it, backpressure will only be exerted if the buffer is nonempty. This sets the stage for (I think) solving #24, since before this commit, it was impossible for a BaseReadableStream to ever return `true` from `push`. After it, that is possible, as long as the stream clears its internal buffer as fast as it can. --- README.md | 1 + reference-implementation/lib/base-readable.js | 2 - .../test/base-readable-stream.js | 41 +++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 8d159d0f1..441767f1d 100644 --- a/README.md +++ b/README.md @@ -204,6 +204,7 @@ BaseReadableStream.prototype.pipeTo = (dest, { close = true } = {}) => { 1. Set `this.[[pulling]]` to **false**. 1. Set `this.[[state]]` to `"readable"`. 1. Resolve `this.[[waitPromise]]` with **undefined**. + 1. Return **true**. 1. If `this.[[state]]` is `"readable"`, 1. Push `data` onto `this.[[buffer]]`. 1. Set `this.[[pulling]]` to **false**. diff --git a/reference-implementation/lib/base-readable.js b/reference-implementation/lib/base-readable.js index b0e3c4545..85635aeb4 100644 --- a/reference-implementation/lib/base-readable.js +++ b/reference-implementation/lib/base-readable.js @@ -91,8 +91,6 @@ BaseReadableStream.prototype._push = function _push(data) { else if (this._state === 'readable') { this._buffer.push(data); this._pulling = false; - - return true; } return false; diff --git a/reference-implementation/test/base-readable-stream.js b/reference-implementation/test/base-readable-stream.js index 4a95843b7..7430afb95 100644 --- a/reference-implementation/test/base-readable-stream.js +++ b/reference-implementation/test/base-readable-stream.js @@ -467,3 +467,44 @@ test('BaseReadableStream cancellation puts the stream in a closed state (after w }); }, t.ifError.bind(t)); }); + +test('BaseReadableStream returns `true` for the first `push` call; `false` thereafter, if nobody reads', function (t) { + t.plan(5); + + var pushes = 0; + var stream = new BaseReadableStream({ + start : function (push) { + t.equal(push('hi'), true); + t.equal(push('hey'), false); + t.equal(push('whee'), false); + t.equal(push('yo'), false); + t.equal(push('sup'), false); + } + }); +}); + +test('BaseReadableStream continues returning `true` from `push` if the data is read out of it', function (t) { + t.plan(12); + + var stream = new BaseReadableStream({ + start : function (push) { + // Delay a bit so that the stream is successfully constructed and thus the `stream` variable references something. + setTimeout(function () { + t.equal(push('hi'), true); + t.equal(stream.state, 'readable'); + t.equal(stream.read(), 'hi'); + t.equal(stream.state, 'waiting'); + + t.equal(push('hey'), true); + t.equal(stream.state, 'readable'); + t.equal(stream.read(), 'hey'); + t.equal(stream.state, 'waiting'); + + t.equal(push('whee'), true); + t.equal(stream.state, 'readable'); + t.equal(stream.read(), 'whee'); + t.equal(stream.state, 'waiting'); + }, 0); + } + }); +});