nodejs / readable-stream

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

TypeError: Cannot set property 'needReadable' of undefined #456

Closed thatcort closed 2 years ago

thatcort commented 3 years ago

I'm getting the following stack trace when I try to run my app in the browser:

Uncaught (in promise) TypeError: Cannot set property 'needReadable' of undefined
    at PassThrough.Transform (pipeline-2d51d34d.js:3056)
    at new PassThrough (pipeline-2d51d34d.js:3157)
    at Map.import (formats-common.js:53)
    at data.js:29

The app is built with Snowpack, which uses rollup-plugin-node-polyfills, which uses readable-stream to polyfill Node streams. When I step through the code, I think the problem is with how the Duplex prototype chain is initialized:

_stream_duplex.js::58
function Duplex(options) {
  if (!(this instanceof Duplex)) return new Duplex(options);
  Readable.call(this, options);  <<< This should add Readable to the prototype chain, but doesn't
  Writable.call(this, options);
...

Readable.call(this, options) goes to:

function Readable(options) {
  Duplex = Duplex || require('./_stream_duplex');
  if (!(this instanceof Readable)) return new Readable(options); <<< This condition is true first time, so a new Readable is returned

Since Readable isn't already in the prototype chain the instanceof check recurses to return a new object. The new object gets initialized, but then passed back up to Duplex, which just throws it away, rather than adding it to its own prototype chain.

I'm not sure what the best fix is. Should Duplex add a call to Object.setPrototypeOf(...)? This instanceof check seems to be used throughout the library, so it seems a bit strange that I would be hitting this issue for the first time, so maybe I'm missing something?

mcollina commented 3 years ago

I don't know. There seems to be something off with how snowpack is transpiling/compiling/exposing this.

It would likely get fixed once I finish v4.

kran6a commented 3 years ago

I can reproduce this using rollup + rollup-plugin-polyfill-node. It also happens using transform streams.

dalduba commented 3 years ago

Reproduced the same behavior with rollup and rollup-plugin-polyfill-node

mcollina commented 2 years ago

Fixed in v4.0.0 as soon as we ship