search

nodejs / readable-stream

Node-core streams for userland
https://nodejs.org/api/stream.html
Other
1.03k stars 227 forks source link

Readable constructor has extra properties attached #485

Closed cjihrig closed 1 year ago

cjihrig commented 2 years ago

Comparing Readable in Node.js 18.8.0 and readable-stream@4.1.0:

> stream.Readable
[Function: Readable] {
  ReadableState: [Function: ReadableState],
  _fromList: [Function: fromList],
  from: [Function (anonymous)],
  fromWeb: [Function (anonymous)],
  toWeb: [Function (anonymous)],
  wrap: [Function (anonymous)]
}
> const rs = await import('readable-stream')
undefined
> rs.Readable
<ref *1> [Function: Readable] {
  ReadableState: [Function: ReadableState],
  _fromList: [Function: fromList],
  from: [Function (anonymous)],
  fromWeb: [Function (anonymous)],
  toWeb: [Function (anonymous)],
  wrap: [Function (anonymous)],
  _uint8ArrayToBuffer: [Function: _uint8ArrayToBuffer],
  _isUint8Array: [Function: isUint8Array],
  isDisturbed: [Function: isDisturbed],
  isErrored: [Function: isErrored],
  isReadable: [Function: isReadable],
  Readable: [Circular *1],
  Writable: [Function: Writable] {
    WritableState: [Function: WritableState],
    fromWeb: [Function (anonymous)],
    toWeb: [Function (anonymous)]
  },
  Duplex: [Function: Duplex] {
    fromWeb: [Function (anonymous)],
    toWeb: [Function (anonymous)],
    from: [Function (anonymous)]
  },
  Transform: [Function: Transform],
  PassThrough: [Function: PassThrough],
  addAbortSignal: [Function: addAbortSignal],
  finished: [Function: eos] {
    finished: [Function: finished],
    [Symbol(nodejs.util.promisify.custom)]: [Getter]
  },
  destroy: [Function: destroyer],
  pipeline: [Function: pipeline] {
    [Symbol(nodejs.util.promisify.custom)]: [Getter]
  },
  compose: [Function: compose],
  Stream: <ref *2> [Function: Stream] {
    isDisturbed: [Function: isDisturbed],
    isErrored: [Function: isErrored],
    isReadable: [Function: isReadable],
    Readable: [Circular *1],
    Writable: [Function: Writable] {
      WritableState: [Function: WritableState],
      fromWeb: [Function (anonymous)],
      toWeb: [Function (anonymous)]
    },
    Duplex: [Function: Duplex] {
      fromWeb: [Function (anonymous)],
      toWeb: [Function (anonymous)],
      from: [Function (anonymous)]
    },
    Transform: [Function: Transform],
    PassThrough: [Function: PassThrough],
    pipeline: [Function: pipeline] {
      [Symbol(nodejs.util.promisify.custom)]: [Getter]
    },
    addAbortSignal: [Function: addAbortSignal],
    finished: [Function: eos] {
      finished: [Function: finished],
      [Symbol(nodejs.util.promisify.custom)]: [Getter]
    },
    destroy: [Function: destroyer],
    compose: [Function: compose],
    promises: [Getter],
    Stream: [Circular *2],
    _isUint8Array: [Function: isUint8Array],
    _uint8ArrayToBuffer: [Function: _uint8ArrayToBuffer]
  },
  default: [Circular *1]
}
mcollina commented 2 years ago

This is done for historic reason. This module exports Readable, while require('stream') exports Stream. I don't like it either, but I didn't think we should break it in v4. Should we do another major and fix it?

You might just get away with some other variant of https://github.com/nodejs/readable-stream/blob/main/lib/ours/index.js#L33-L64.

cjihrig commented 2 years ago

For my use case, it would be cleaner. Right now I'm just deleting the extra properties off of Readable.

Maybe something to consider in the next major release, but probably not worth artificially forcing a major release just for this.

mcollina commented 2 years ago

Would you mind sending a PR? At least we won't forget when we bump the major next year.