Skip to content

Commit e559842

Browse files
committed
stream: make readable & writable computed
This makes readable and writable automatically computed based on the stream state. Effectivly deprecating/discouraging manual management of this. Makes the properties more consistent and easier to reason about. Fixes: #29377 PR-URL: #31197 Refs: #29377 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent 9c753b3 commit e559842

File tree

8 files changed

+139
-15
lines changed

8 files changed

+139
-15
lines changed

doc/api/stream.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,8 @@ added: v11.4.0
495495

496496
* {boolean}
497497

498-
Is `true` if it is safe to call [`writable.write()`][stream-write].
498+
Is `true` if it is safe to call [`writable.write()`][stream-write], which means
499+
the stream has not been destroyed, errored or ended.
499500

500501
##### `writable.writableEnded`
501502
<!-- YAML
@@ -1134,7 +1135,8 @@ added: v11.4.0
11341135

11351136
* {boolean}
11361137

1137-
Is `true` if it is safe to call [`readable.read()`][stream-read].
1138+
Is `true` if it is safe to call [`readable.read()`][stream-read], which means
1139+
the stream has not been destroyed or emitted `'error'` or `'end'`.
11381140

11391141
##### `readable.readableEncoding`
11401142
<!-- YAML

lib/_http_incoming.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,6 @@ function IncomingMessage(socket) {
6363
this.trailers = {};
6464
this.rawTrailers = [];
6565

66-
this.readable = true;
67-
6866
this.aborted = false;
6967

7068
this.upgrade = null;

lib/_stream_duplex.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
const {
3030
ObjectDefineProperties,
31+
ObjectGetOwnPropertyDescriptor,
3132
ObjectKeys,
3233
ObjectSetPrototypeOf,
3334
} = primordials;
@@ -71,6 +72,7 @@ function Duplex(options) {
7172
}
7273

7374
ObjectDefineProperties(Duplex.prototype, {
75+
writable: ObjectGetOwnPropertyDescriptor(Writable.prototype, 'writable'),
7476

7577
destroyed: {
7678
get() {

lib/_stream_readable.js

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
const {
2525
ArrayIsArray,
26+
Boolean,
2627
NumberIsInteger,
2728
NumberIsNaN,
2829
ObjectDefineProperties,
@@ -181,9 +182,6 @@ function Readable(options) {
181182

182183
this._readableState = new ReadableState(options, this, isDuplex);
183184

184-
// legacy
185-
this.readable = true;
186-
187185
if (options) {
188186
if (typeof options.read === 'function')
189187
this._read = options.read;
@@ -1057,6 +1055,20 @@ Readable.prototype[SymbolAsyncIterator] = function() {
10571055
// because otherwise some prototype manipulation in
10581056
// userland will fail
10591057
ObjectDefineProperties(Readable.prototype, {
1058+
readable: {
1059+
get() {
1060+
const r = this._readableState;
1061+
if (!r) return false;
1062+
if (r.readable !== undefined) return r.readable && !r.endEmitted;
1063+
return Boolean(!r.destroyed && !r.errorEmitted && !r.endEmitted);
1064+
},
1065+
set(val) {
1066+
// Backwards compat.
1067+
if (this._readableState) {
1068+
this._readableState.readable = !!val;
1069+
}
1070+
}
1071+
},
10601072

10611073
readableHighWaterMark: {
10621074
enumerable: false,
@@ -1198,7 +1210,6 @@ function endReadableNT(state, stream) {
11981210
// Check that we didn't get one last unshift.
11991211
if (!state.errorEmitted && !state.endEmitted && state.length === 0) {
12001212
state.endEmitted = true;
1201-
stream.readable = false;
12021213
stream.emit('end');
12031214

12041215
if (state.autoDestroy) {

lib/_stream_writable.js

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
const {
2929
Array,
30+
Boolean,
3031
FunctionPrototype,
3132
ObjectDefineProperty,
3233
ObjectSetPrototypeOf,
@@ -233,9 +234,6 @@ function Writable(options) {
233234

234235
this._writableState = new WritableState(options, this, isDuplex);
235236

236-
// legacy.
237-
this.writable = true;
238-
239237
if (options) {
240238
if (typeof options.write === 'function')
241239
this._write = options.write;
@@ -776,6 +774,21 @@ function onFinished(stream, state, cb) {
776774
stream.prependListener('error', onerror);
777775
}
778776

777+
ObjectDefineProperty(Writable.prototype, 'writable', {
778+
get() {
779+
const w = this._writableState;
780+
if (!w) return false;
781+
if (w.writable !== undefined) return w.writable;
782+
return Boolean(!w.destroyed && !w.errored && !w.ending);
783+
},
784+
set(val) {
785+
// Backwards compat.
786+
if (this._writableState) {
787+
this._writableState.writable = !!val;
788+
}
789+
}
790+
});
791+
779792
ObjectDefineProperty(Writable.prototype, 'destroyed', {
780793
// Making it explicit this property is not enumerable
781794
// because otherwise some prototype manipulation in

test/parallel/test-http2-socket-proxy.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,14 @@ server.on('stream', common.mustCall(function(stream, headers) {
8585

8686
stream.respond();
8787

88-
socket.writable = 0;
89-
socket.readable = 0;
90-
assert.strictEqual(socket.writable, 0);
91-
assert.strictEqual(socket.readable, 0);
88+
socket.writable = true;
89+
socket.readable = true;
90+
assert.strictEqual(socket.writable, true);
91+
assert.strictEqual(socket.readable, true);
92+
socket.writable = false;
93+
socket.readable = false;
94+
assert.strictEqual(socket.writable, false);
95+
assert.strictEqual(socket.readable, false);
9296

9397
stream.end();
9498

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
5+
const { Readable } = require('stream');
6+
7+
{
8+
const r = new Readable({
9+
read() {}
10+
});
11+
assert.strictEqual(r.readable, true);
12+
r.destroy();
13+
assert.strictEqual(r.readable, false);
14+
}
15+
16+
{
17+
const mustNotCall = common.mustNotCall();
18+
const r = new Readable({
19+
read() {}
20+
});
21+
assert.strictEqual(r.readable, true);
22+
r.on('end', mustNotCall);
23+
r.resume();
24+
r.push(null);
25+
assert.strictEqual(r.readable, true);
26+
r.off('end', mustNotCall);
27+
r.on('end', common.mustCall(() => {
28+
assert.strictEqual(r.readable, false);
29+
}));
30+
}
31+
32+
{
33+
const r = new Readable({
34+
read: common.mustCall(() => {
35+
process.nextTick(() => {
36+
r.destroy(new Error());
37+
assert.strictEqual(r.readable, false);
38+
});
39+
})
40+
});
41+
r.resume();
42+
r.on('error', common.mustCall(() => {
43+
assert.strictEqual(r.readable, false);
44+
}));
45+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
5+
const { Writable } = require('stream');
6+
7+
{
8+
const w = new Writable({
9+
write() {}
10+
});
11+
assert.strictEqual(w.writable, true);
12+
w.destroy();
13+
assert.strictEqual(w.writable, false);
14+
}
15+
16+
{
17+
const w = new Writable({
18+
write: common.mustCall((chunk, encoding, callback) => {
19+
callback(new Error());
20+
})
21+
});
22+
assert.strictEqual(w.writable, true);
23+
w.write('asd');
24+
assert.strictEqual(w.writable, false);
25+
w.on('error', common.mustCall());
26+
w.destroy();
27+
}
28+
29+
{
30+
const w = new Writable({
31+
write: common.mustCall((chunk, encoding, callback) => {
32+
process.nextTick(() => {
33+
callback(new Error());
34+
assert.strictEqual(w.writable, false);
35+
});
36+
})
37+
});
38+
w.write('asd');
39+
w.on('error', common.mustCall());
40+
}
41+
42+
{
43+
const w = new Writable({
44+
write: common.mustNotCall()
45+
});
46+
assert.strictEqual(w.writable, true);
47+
w.end();
48+
assert.strictEqual(w.writable, false);
49+
}

0 commit comments

Comments
 (0)