Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[x] stream: properties cleanup #29046

Closed
wants to merge 4 commits into from
Closed

[x] stream: properties cleanup #29046

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bb30d85
stream: always initialized
ronag Aug 8, 2019
2654dbf
stream: removed redudant option to defineProperty
ronag Aug 8, 2019
f17dca0
stream: move properties directly after constructor
ronag Aug 8, 2019
5d1898b
stream: use shorthand notation for properties
ronag Aug 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 15 additions & 45 deletions lib/_stream_duplex.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,45 +69,42 @@ function Duplex(options) {
}

Object.defineProperty(Duplex.prototype, 'writableHighWaterMark', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get() {
return this._writableState.highWaterMark;
}
});

Object.defineProperty(Duplex.prototype, 'writableBuffer', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get: function() {
return this._writableState && this._writableState.getBuffer();
get() {
return this._writableState.getBuffer();
}
});

Object.defineProperty(Duplex.prototype, 'writableLength', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get() {
return this._writableState.length;
}
});

Object.defineProperty(Duplex.prototype, 'writableFinished', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get() {
return this._writableState.finished;
}
});

Object.defineProperty(Duplex.prototype, 'destroyed', {
get() {
return this._readableState.destroyed && this._writableState.destroyed;
},
set(value) {
// Backward compatibility, the user is explicitly
// managing destroyed
this._readableState.destroyed = value;
this._writableState.destroyed = value;
}
});


// The no-half-open enforcer
function onend() {
// If the writable side ended, then we're ok.
Expand All @@ -122,30 +119,3 @@ function onend() {
function onEndNT(self) {
self.end();
}

Object.defineProperty(Duplex.prototype, 'destroyed', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get() {
if (this._readableState === undefined ||
this._writableState === undefined) {
return false;
}
return this._readableState.destroyed && this._writableState.destroyed;
},
set(value) {
// We ignore the value if the stream
// has not been initialized yet
if (this._readableState === undefined ||
this._writableState === undefined) {
return;
}

// Backward compatibility, the user is explicitly
// managing destroyed
this._readableState.destroyed = value;
this._writableState.destroyed = value;
}
});
117 changes: 42 additions & 75 deletions lib/_stream_readable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,30 +178,59 @@ function Readable(options) {
Stream.call(this);
}

// Exposed for testing purposes only.
Readable._fromList = fromList;

Object.defineProperty(Readable.prototype, 'destroyed', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get() {
if (this._readableState === undefined) {
return false;
}
return this._readableState.destroyed;
},
set(value) {
// We ignore the value if the stream
// has not been initialized yet
if (!this._readableState) {
return;
}

// Backward compatibility, the user is explicitly
// managing destroyed
this._readableState.destroyed = value;
}
});

Object.defineProperty(Readable.prototype, 'readableHighWaterMark', {
get() {
return this._readableState.highWaterMark;
}
});

Object.defineProperty(Readable.prototype, 'readableBuffer', {
get() {
return this._readableState.buffer;
}
});

Object.defineProperty(Readable.prototype, 'readableFlowing', {
get() {
return this._readableState.flowing;
},
set(state) {
this._readableState.flowing = state;
}
});

Object.defineProperty(Readable.prototype, 'readableLength', {
get() {
return this._readableState.length;
}
});

Object.defineProperty(Readable.prototype, 'readableObjectMode', {
get() {
return this._readableState.objectMode;
}
});

Object.defineProperty(Readable.prototype, 'readableEncoding', {
get() {
return this._readableState.encoding;
}
});

Readable.prototype.destroy = destroyImpl.destroy;
Readable.prototype._undestroy = destroyImpl.undestroy;
Readable.prototype._destroy = function(err, cb) {
Expand Down Expand Up @@ -1027,68 +1056,6 @@ Readable.prototype[Symbol.asyncIterator] = function() {
return createReadableStreamAsyncIterator(this);
};

Object.defineProperty(Readable.prototype, 'readableHighWaterMark', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get: function() {
return this._readableState.highWaterMark;
}
});

Object.defineProperty(Readable.prototype, 'readableBuffer', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get: function() {
return this._readableState && this._readableState.buffer;
}
});

Object.defineProperty(Readable.prototype, 'readableFlowing', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get: function() {
return this._readableState.flowing;
},
set: function(state) {
if (this._readableState) {
this._readableState.flowing = state;
}
}
});

// Exposed for testing purposes only.
Readable._fromList = fromList;

Object.defineProperty(Readable.prototype, 'readableLength', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get() {
return this._readableState.length;
}
});

Object.defineProperty(Readable.prototype, 'readableObjectMode', {
enumerable: false,
get() {
return this._readableState ? this._readableState.objectMode : false;
}
});

Object.defineProperty(Readable.prototype, 'readableEncoding', {
enumerable: false,
get() {
return this._readableState ? this._readableState.encoding : null;
}
});

// Pluck off n bytes from an array of buffers.
// Length is the combined lengths of all the buffers in the list.
// This function is designed to be inlinable, so please take care when making
Expand Down
111 changes: 42 additions & 69 deletions lib/_stream_writable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,13 @@ function WritableState(options, stream, isDuplex) {
this.corkedRequestsFree = corkReq;
}

Object.defineProperty(WritableState.prototype, 'buffer', {
get: internalUtil.deprecate(function writableStateBufferGetter() {
return this.getBuffer();
}, '_writableState.buffer is deprecated. Use _writableState.getBuffer ' +
'instead.', 'DEP0003')
});

WritableState.prototype.getBuffer = function getBuffer() {
var current = this.bufferedRequest;
const out = [];
Expand All @@ -174,13 +181,6 @@ WritableState.prototype.getBuffer = function getBuffer() {
return out;
};

Object.defineProperty(WritableState.prototype, 'buffer', {
get: internalUtil.deprecate(function writableStateBufferGetter() {
return this.getBuffer();
}, '_writableState.buffer is deprecated. Use _writableState.getBuffer ' +
'instead.', 'DEP0003')
});

// Test _writableState for inheritance to account for Duplex streams,
// whose prototype chain only points to Readable.
var realHasInstance;
Expand Down Expand Up @@ -240,6 +240,41 @@ function Writable(options) {
Stream.call(this);
}

Object.defineProperty(Writable.prototype, 'destroyed', {
get() {
return this._writableState.destroyed;
},
set(value) {
// Backward compatibility, the user is explicitly
// managing destroyed
this._writableState.destroyed = value;
}
});

Object.defineProperty(Writable.prototype, 'writableObjectMode', {
get() {
return this._writableState.objectMode;
}
});

Object.defineProperty(Writable.prototype, 'writableFinished', {
get() {
return this._writableState.finished;
}
});

Object.defineProperty(Writable.prototype, 'writableBuffer', {
get() {
return this._writableState.getBuffer();
}
});

Object.defineProperty(Writable.prototype, 'writableHighWaterMark', {
get() {
return this._writableState.highWaterMark;
}
});

// Otherwise people can pipe Writable streams, which is just wrong.
Writable.prototype.pipe = function() {
errorOrDestroy(this, new ERR_STREAM_CANNOT_PIPE());
Expand Down Expand Up @@ -333,16 +368,6 @@ Writable.prototype.setDefaultEncoding = function setDefaultEncoding(encoding) {
return this;
};

Object.defineProperty(Writable.prototype, 'writableBuffer', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get: function() {
return this._writableState && this._writableState.getBuffer();
}
});

function decodeChunk(state, chunk, encoding) {
if (!state.objectMode &&
state.decodeStrings !== false &&
Expand All @@ -352,16 +377,6 @@ function decodeChunk(state, chunk, encoding) {
return chunk;
}

Object.defineProperty(Writable.prototype, 'writableHighWaterMark', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get: function() {
return this._writableState.highWaterMark;
}
});

// If we're already writing something, then just put this
// in the queue, and wait our turn. Otherwise, call _write
// If we return false, then we need a drain event, so set that flag.
Expand Down Expand Up @@ -599,7 +614,6 @@ Object.defineProperty(Writable.prototype, 'writableLength', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get() {
return this._writableState.length;
}
Expand Down Expand Up @@ -684,47 +698,6 @@ function onCorkedFinish(corkReq, state, err) {
state.corkedRequestsFree.next = corkReq;
}

Object.defineProperty(Writable.prototype, 'destroyed', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get() {
if (this._writableState === undefined) {
return false;
}
return this._writableState.destroyed;
},
set(value) {
// We ignore the value if the stream
// has not been initialized yet
if (!this._writableState) {
return;
}

// Backward compatibility, the user is explicitly
// managing destroyed
this._writableState.destroyed = value;
}
});

Object.defineProperty(Writable.prototype, 'writableObjectMode', {
enumerable: false,
get() {
return this._writableState ? this._writableState.objectMode : false;
}
});

Object.defineProperty(Writable.prototype, 'writableFinished', {
// Making it explicit this property is not enumerable
// because otherwise some prototype manipulation in
// userland will fail
enumerable: false,
get() {
return this._writableState.finished;
}
});

Writable.prototype.destroy = destroyImpl.destroy;
Writable.prototype._undestroy = destroyImpl.undestroy;
Writable.prototype._destroy = function(err, cb) {
Expand Down
Loading