From cbd12518d4f7748f0804f4a62b3c0bf42b88f5be Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 6 Oct 2019 12:07:51 +0200 Subject: [PATCH] doc: add note about forwarding stream options It is a common and unfortunate pattern to simply just forward any and all options into the base constructor without taking into account whether these options might conflict with basic stream assumptions. PR-URL: https://github.com/nodejs/node/pull/29857 Reviewed-By: James M Snell Reviewed-By: Luigi Pinca Reviewed-By: Ruben Bridgewater --- doc/api/stream.md | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/doc/api/stream.md b/doc/api/stream.md index 8189f5c84d93ab..ed81ba91a7760f 100644 --- a/doc/api/stream.md +++ b/doc/api/stream.md @@ -1641,13 +1641,24 @@ parent class constructor: const { Writable } = require('stream'); class MyWritable extends Writable { - constructor(options) { - super(options); + constructor({ highWaterMark, ...options }) { + super({ + highWaterMark, + autoDestroy: true, + emitClose: true + }); // ... } } ``` +When extending streams, it is important to keep in mind what options the user +can and should provide before forwarding these to the base constructor. For +example, if the implementation makes assumptions in regard to e.g. the +`autoDestroy` and `emitClose` options, it becomes important to not allow the +user to override these. It is therefore recommended to be explicit about what +options are forwarded instead of implicitly forwarding all options. + The new stream class must then implement one or more specific methods, depending on the type of stream being created, as detailed in the chart below: