Skip to content

Commit

Permalink
Remove reader.isActive
Browse files Browse the repository at this point in the history
The conservative thing to do is to not have this ability for now. Released readers are now indistinguishable from readers for closed streams.
  • Loading branch information
domenic committed Mar 16, 2015
1 parent 189636c commit f4ac4d5
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 55 deletions.
14 changes: 0 additions & 14 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,6 @@ it would look like
constructor(stream)

get closed()
get isActive()

cancel(reason)
read()
Expand Down Expand Up @@ -580,19 +579,6 @@ Instances of <code>ReadableStreamReader</code> are created with the internal slo
<li> Return <b>this</b>@\[[closedPromise]].
</ol>

<h5 id="reader-is-active">get isActive</h5>

<div class="note">
The <code>isActive</code> getter returns whether or not the stream reader is currently
<a lt="active reader">active</a>.
</div>

<ol>
<li> If IsReadableStreamReader(<b>this</b>) is <b>false</b>, throw a <b>TypeError</b> exception.
<li> If <b>this</b>@\[[ownerReadableStream]] is <b>undefined</b>, return <b>false</b>; otherwise, return
<b>true</b>.
</ol>

<h5 id="reader-cancel">cancel(reason)</h5>

<div class="note">
Expand Down
8 changes: 0 additions & 8 deletions reference-implementation/lib/readable-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,6 @@ class ReadableStreamReader {
return this._closedPromise;
}

get isActive() {
if (IsReadableStreamReader(this) === false) {
throw new TypeError('ReadableStreamReader.prototype.isActive can only be used on a ReadableStreamReader');
}

return this._ownerReadableStream !== undefined;
}

cancel(reason) {
if (IsReadableStreamReader(this) === false) {
return Promise.reject(
Expand Down
7 changes: 0 additions & 7 deletions reference-implementation/test/brand-checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ function realWritableStream() {
function fakeReadableStreamReader() {
return {
get closed() { return Promise.resolve(); },
get isActive() { return false; },
cancel(reason) { return Promise.resolve(); },
read() { return Promise.resolve({ value: undefined, done: true }); },
releaseLock() { return; }
Expand Down Expand Up @@ -161,12 +160,6 @@ test('ReadableStreamReader.prototype.closed enforces a brand check', t => {
getterRejects(t, ReadableStreamReader.prototype, 'closed', realReadableStream());
});

test('ReadableStreamReader.prototype.isActive enforces a brand check', t => {
t.plan(2);
getterThrows(t, ReadableStreamReader.prototype, 'isActive', fakeReadableStreamReader());
getterThrows(t, ReadableStreamReader.prototype, 'isActive', realReadableStream());
});

test('ReadableStreamReader.prototype.cancel enforces a brand check', t => {
t.plan(2);
methodRejects(t, ReadableStreamReader.prototype, 'cancel', fakeReadableStreamReader());
Expand Down
29 changes: 15 additions & 14 deletions reference-implementation/test/readable-stream-reader.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,8 @@ test('Reading from a reader for an empty stream will wait until a chunk is avail
});
const reader = rs.getReader();

t.equal(reader.isActive, true, 'reader is active to start with');

reader.read().then(result => {
t.deepEqual(result, { value: 'a', done: false }, 'read() should fulfill with the enqueued chunk');
t.equal(reader.isActive, true, 'reader is still active');
t.end();
});

Expand All @@ -85,7 +82,7 @@ test('cancel() on a reader releases the reader before calling through', t => {
const passedReason = new Error('it wasn\'t the right time, sorry');
const rs = new ReadableStream({
cancel(reason) {
t.equal(reader.isActive, false, 'reader should be released by the time underlying source cancel is called');
t.doesNotThrow(() => rs.getReader(), 'should be able to get another reader without error');
t.equal(reason, passedReason, 'the cancellation reason is passed through to the underlying source');
}
});
Expand All @@ -109,14 +106,14 @@ test('closed should be fulfilled after stream is closed (.closed access before a

const reader = rs.getReader();
reader.closed.then(() => {
t.equal(reader.isActive, false, 'reader is no longer active when reader closed is fulfilled');
t.pass('reader closed should be fulfilled');
});

doClose();
});

test('closed should be fulfilled after reader releases its lock (multiple stream locks)', t => {
t.plan(4);
t.plan(2);

let doClose;
const rs = new ReadableStream({
Expand All @@ -133,13 +130,11 @@ test('closed should be fulfilled after reader releases its lock (multiple stream
doClose();

reader1.closed.then(() => {
t.equal(reader1.isActive, false, 'reader1 is no longer active when reader1 closed is fulfilled');
t.equal(reader2.isActive, false, 'reader2 is no longer active when reader1 closed is fulfilled');
t.pass('reader1 closed should be fulfilled');
});

reader2.closed.then(() => {
t.equal(reader1.isActive, false, 'reader1 is no longer active when reader2 closed is fulfilled');
t.equal(reader2.isActive, false, 'reader2 is no longer active when reader2 closed is fulfilled');
t.pass('reader2 closed should be fulfilled');
});
});

Expand All @@ -164,18 +159,24 @@ test('Multiple readers can access the stream in sequence', t => {
});

test('Cannot use an already-released reader to unlock a stream again', t => {
t.plan(2);
t.plan(1);

const rs = new ReadableStream();
const rs = new ReadableStream({
start(enqueue) {
enqueue('a');
}
});

const reader1 = rs.getReader();
reader1.releaseLock();

const reader2 = rs.getReader();
t.equal(reader2.isActive, true, 'reader2 state is active before releasing reader1');

reader1.releaseLock();
t.equal(reader2.isActive, true, 'reader2 state is still active after releasing reader1 again');
reader2.read().then(result => {
t.deepEqual(result, { value: 'a', done: false },
'read() should still work on reader2 even after reader1 is released');
});
});

test('cancel() on a released reader is a no-op and does not pass through', t => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,15 @@ export default (label, factory) => {
);

t.throws(() => reader.releaseLock(), /TypeError/, 'releaseLock should throw a TypeError');
t.equal(reader.isActive, true, 'the reader should still be active');

setTimeout(() => t.end(), 50);
});

test('releasing the lock should cause further read() calls to resolve as if the stream is closed', t => {
t.plan(3);
t.plan(2);
const { reader } = factory();

reader.releaseLock();
t.equal(reader.isActive, false, 'the reader should no longer be active');

reader.read().then(r =>
t.deepEqual(r, { value: undefined, done: true }, 'first read() should return closed result'));
Expand All @@ -97,34 +95,29 @@ export default (label, factory) => {
});

test('releasing the lock should cause closed to fulfill', t => {
t.plan(3);
t.plan(2);
const { reader } = factory();

reader.closed.then(v => t.equal(v, undefined, 'reader.closed got before release should fulfill with undefined'));

reader.releaseLock();
t.equal(reader.isActive, false, 'the reader should no longer be active');

reader.closed.then(v => t.equal(v, undefined, 'reader.closed got after release should fulfill with undefined'));
});

test('canceling via the reader should cause the reader to become inactive', t => {
t.plan(3);
test('canceling via the reader should cause the reader to act closed', t => {
t.plan(1);
const { reader } = factory();

t.equal(reader.isActive, true, 'the reader should be active before releasing it');
reader.cancel();
t.equal(reader.isActive, false, 'the reader should no longer be active');
reader.read().then(r => t.deepEqual(r, { value: undefined, done: true },
'read()ing from the reader should give a done result'))
});

test('canceling via the stream should fail', t => {
t.plan(3);
t.plan(1);
const { stream, reader } = factory();

t.equal(reader.isActive, true, 'the reader should be active before releasing it');
stream.cancel().catch(e => t.equal(e.constructor, TypeError, 'cancel() should reject with a TypeError'));
t.equal(reader.isActive, true, 'the reader should still be active');
});
};

0 comments on commit f4ac4d5

Please sign in to comment.