Skip to content

Commit

Permalink
stream: remove thenable support
Browse files Browse the repository at this point in the history
Remove support for returning thenables in stream
implementation methods. This is causing more confusion
and issues than it's worth.

Refs: #39535

PR-URL: #40773
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
ronag authored Apr 8, 2022
1 parent d36b60e commit 58e645d
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 360 deletions.
5 changes: 4 additions & 1 deletion doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -3019,14 +3019,17 @@ it was an aborted or graceful destroy.

<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/40773
description: End-of-life.
- version:
- v17.2.0
- v16.14.0
pr-url: https://github.com/nodejs/node/pull/40860
description: Documentation-only deprecation.
-->

Type: Documentation-only
Type: End-of-Life

An undocumented feature of Node.js streams was to support thenables in
implementation methods. This is now deprecated, use callbacks instead and avoid
Expand Down
34 changes: 2 additions & 32 deletions lib/internal/streams/destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,20 +106,7 @@ function _destroy(self, err, cb) {
}
}
try {
const result = self._destroy(err || null, onDestroy);
if (result != null) {
const then = result.then;
if (typeof then === 'function') {
then.call(
result,
function() {
process.nextTick(onDestroy, null);
},
function(err) {
process.nextTick(onDestroy, err);
});
}
}
self._destroy(err || null, onDestroy);
} catch (err) {
onDestroy(err);
}
Expand Down Expand Up @@ -285,24 +272,7 @@ function constructNT(stream) {
}

try {
const result = stream._construct(onConstruct);
if (result != null) {
const then = result.then;
if (typeof then === 'function') {
then.call(
result,
function() {
if (!called) {
process.nextTick(onConstruct, null);
}
},
function(err) {
if (!called) {
process.nextTick(onConstruct, err);
}
});
}
}
stream._construct(onConstruct);
} catch (err) {
onConstruct(err);
}
Expand Down
13 changes: 1 addition & 12 deletions lib/internal/streams/readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -493,18 +493,7 @@ Readable.prototype.read = function(n) {

// Call internal read method
try {
const result = this._read(state.highWaterMark);
if (result != null) {
const then = result.then;
if (typeof then === 'function') {
then.call(
result,
nop,
function(err) {
errorOrDestroy(this, err);
});
}
}
this._read(state.highWaterMark);
} catch (err) {
errorOrDestroy(this, err);
}
Expand Down
67 changes: 2 additions & 65 deletions lib/internal/streams/transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,8 @@ function Transform(options) {
}

function final(cb) {
let called = false;
if (typeof this._flush === 'function' && !this.destroyed) {
const result = this._flush((er, data) => {
called = true;
this._flush((er, data) => {
if (er) {
if (cb) {
cb(er);
Expand All @@ -128,33 +126,6 @@ function final(cb) {
cb();
}
});
if (result !== undefined && result !== null) {
try {
const then = result.then;
if (typeof then === 'function') {
then.call(
result,
(data) => {
if (called)
return;
if (data != null)
this.push(data);
this.push(null);
if (cb)
process.nextTick(cb);
},
(err) => {
if (cb) {
process.nextTick(cb, err);
} else {
process.nextTick(() => this.destroy(err));
}
});
}
} catch (err) {
process.nextTick(() => this.destroy(err));
}
}
} else {
this.push(null);
if (cb) {
Expand All @@ -180,9 +151,7 @@ Transform.prototype._write = function(chunk, encoding, callback) {
const wState = this._writableState;
const length = rState.length;

let called = false;
const result = this._transform(chunk, encoding, (err, val) => {
called = true;
this._transform(chunk, encoding, (err, val) => {
if (err) {
callback(err);
return;
Expand All @@ -204,38 +173,6 @@ Transform.prototype._write = function(chunk, encoding, callback) {
this[kCallback] = callback;
}
});
if (result !== undefined && result != null) {
try {
const then = result.then;
if (typeof then === 'function') {
then.call(
result,
(val) => {
if (called)
return;

if (val != null) {
this.push(val);
}

if (
wState.ended ||
length === rState.length ||
rState.length < rState.highWaterMark ||
rState.length === 0) {
process.nextTick(callback);
} else {
this[kCallback] = callback;
}
},
(err) => {
process.nextTick(callback, err);
});
}
} catch (err) {
process.nextTick(callback, err);
}
}
};

Transform.prototype._read = function() {
Expand Down
19 changes: 1 addition & 18 deletions lib/internal/streams/writable.js
Original file line number Diff line number Diff line change
Expand Up @@ -693,24 +693,7 @@ function callFinal(stream, state) {
state.pendingcb++;

try {
const result = stream._final(onFinish);
if (result != null) {
const then = result.then;
if (typeof then === 'function') {
then.call(
result,
function() {
if (!called) {
process.nextTick(onFinish, null);
}
},
function(err) {
if (!called) {
process.nextTick(onFinish, err);
}
});
}
}
stream._final(onFinish);
} catch (err) {
onFinish(err);
}
Expand Down
Loading

0 comments on commit 58e645d

Please sign in to comment.