Skip to content

Commit

Permalink
stream: proper instanceof for Writables
Browse files Browse the repository at this point in the history
Use `[Symbol.hasInstance]()` to return `true` when asking for
`new Duplex() instanceof Writable`.

PR-URL: #8834
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Calvin Metcalf <calvin.metcalf@gmail.com>
  • Loading branch information
addaleax committed Oct 10, 2016
1 parent a6e1be4 commit 2a4b068
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 4 deletions.
5 changes: 4 additions & 1 deletion doc/api/stream.md
Expand Up @@ -1559,7 +1559,9 @@ Because JavaScript does not have support for multiple inheritance, the
to extending the `stream.Readable` *and* `stream.Writable` classes).

*Note*: The `stream.Duplex` class prototypically inherits from `stream.Readable`
and parasitically from `stream.Writable`.
and parasitically from `stream.Writable`, but `instanceof` will work properly
for both base classes due to overriding [`Symbol.hasInstance`][]
on `stream.Writable`.

Custom Duplex streams *must* call the `new stream.Duplex([options])`
constructor and implement *both* the `readable._read()` and
Expand Down Expand Up @@ -2009,3 +2011,4 @@ readable buffer so there is nothing for a user to consume.
[Transform]: #stream_class_stream_transform
[Writable]: #stream_class_stream_writable
[zlib]: zlib.html
[`Symbol.hasInstance`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/hasInstance
27 changes: 24 additions & 3 deletions lib/_stream_writable.js
Expand Up @@ -134,11 +134,32 @@ Object.defineProperty(WritableState.prototype, 'buffer', {
'instead.')
});

// Test _writableState for inheritance to account for Duplex streams,
// whose prototype chain only points to Readable.
var realHasInstance;
if (typeof Symbol === 'function' && Symbol.hasInstance) {
realHasInstance = Function.prototype[Symbol.hasInstance];
Object.defineProperty(Writable, Symbol.hasInstance, {
value: function(object) {
// Trying to move the `realHasInstance` from the Writable constructor
// to here will break the Node.js LazyTransform implementation.
return object._writableState instanceof WritableState;
}
});
} else {
realHasInstance = function(object) {
return object instanceof this;
};
}

function Writable(options) {
// Writable ctor is applied to Duplexes, though they're not
// instanceof Writable, they're instanceof Readable.
if (!(this instanceof Writable) && !(this instanceof Stream.Duplex))
// Writable ctor is applied to Duplexes, too.
// `realHasInstance` is necessary because using plain `instanceof`
// would return false, as no `_writableState` property is attached.
if (!(realHasInstance.call(Writable, this)) &&
!(this instanceof Stream.Duplex)) {
return new Writable(options);
}

this._writableState = new WritableState(options, this);

Expand Down
29 changes: 29 additions & 0 deletions test/parallel/test-stream-inheritance.js
@@ -0,0 +1,29 @@
'use strict';
require('../common');
const assert = require('assert');
const { Readable, Writable, Duplex, Transform } = require('stream');

const readable = new Readable({ read() {} });
const writable = new Writable({ write() {} });
const duplex = new Duplex({ read() {}, write() {} });
const transform = new Transform({ transform() {} });

assert.ok(readable instanceof Readable);
assert.ok(!(writable instanceof Readable));
assert.ok(duplex instanceof Readable);
assert.ok(transform instanceof Readable);

assert.ok(!(readable instanceof Writable));
assert.ok(writable instanceof Writable);
assert.ok(duplex instanceof Writable);
assert.ok(transform instanceof Writable);

assert.ok(!(readable instanceof Duplex));
assert.ok(!(writable instanceof Duplex));
assert.ok(duplex instanceof Duplex);
assert.ok(transform instanceof Duplex);

assert.ok(!(readable instanceof Transform));
assert.ok(!(writable instanceof Transform));
assert.ok(!(duplex instanceof Transform));
assert.ok(transform instanceof Transform);

0 comments on commit 2a4b068

Please sign in to comment.