regression v2.2.7 and v2.2.8 are broken on some webpack / browserify deployments

[mcollina]

Original issue: https://github.com/mqttjs/MQTT.js/issues/596. It was fixed by blocking the version of readable-stream to 2.2.6.

cc @calvinmetcalf

[mcollina]

I cannot reproduce myself really, so something else should be going on.

I will leave it open, in case @CVarisco would like to produce a full example that is reproducing the problem.

I think the problem is that in https://github.com/nodejs/readable-stream/blob/master/lib/_stream_readable.js#L33, in some cases Stream is {} instead of null.

I'm not sure how to test this.

[RangerMauve]

@mcollina If the problem is Stream being an empty object, that's definitely related to the PR I sent in #258 and affects people trying to use React-Native. In https://github.com/mqttjs/MQTT.js/issues/573 I proposed to use the following change to see if it'd fix things:

var Stream;
(function () {
  try {
    Stream = require('st' + 'ream');
  } catch (_) {} finally {
    if (!Stream) Stream = require('events').EventEmitter;
  }
  // This is the addiition
  if(!Stream.on) Stream =  require('events').EventEmitter;
})();

I haven't had time to test this out, however.

[calvinmetcalf]

@RangerMauve in react native, is process.browser true?

[RangerMauve]

@calvinmetcalf No, it's the packager which causes it to happen by looking in the react-native property in package.json. It has no effect on bundlers which don't use the react-native field.

I don't think react-native even defines a process global, actually.

[calvinmetcalf]

so I just opened #278 which uses process.browser to tell if it should import stream or go with event emitter, I'm not sure of the behavior of react-native but I'm fairly confident that if it already worked, it has to have some form of process global just not sure what it is.

[RangerMauve]

In order to get it to run in React-Native I had to define process and Buffer globals, for process I was using this module, and React-Native was likely loading the browser version of it since it respects the browser field in package.json

[calvinmetcalf]

ok so then my pull will likely work there

On Fri, Apr 7, 2017 at 12:27 PM RangerMauve notifications@github.com wrote:

In order to get it to run in React-Native I had to define process and Buffer globals, for process I was using this module https://www.npmjs.com/package/process, and React-Native was likely loading the browser version of it since it respects the browser field in package.json

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nodejs/readable-stream/issues/277#issuecomment-292583981, or mute the thread https://github.com/notifications/unsubscribe-auth/ABE4n3p5hT-a13ssY9XL4q5LsTx2lkBfks5rtmPRgaJpZM4M3CiD .

[mcollina]

another duplicate https://github.com/maxogden/websocket-stream/issues/114. Il giorno ven 7 apr 2017 alle 18:37 Calvin Metcalf notifications@github.com ha scritto:

ok so then my pull will likely work there

On Fri, Apr 7, 2017 at 12:27 PM RangerMauve notifications@github.com wrote:

In order to get it to run in React-Native I had to define process and Buffer globals, for process I was using this module https://www.npmjs.com/package/process, and React-Native was likely loading the browser version of it since it respects the browser field in package.json

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/nodejs/readable-stream/issues/277#issuecomment-292583981 , or mute the thread < https://github.com/notifications/unsubscribe-auth/ABE4n3p5hT-a13ssY9XL4q5LsTx2lkBfks5rtmPRgaJpZM4M3CiD

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nodejs/readable-stream/issues/277#issuecomment-292586895, or mute the thread https://github.com/notifications/unsubscribe-auth/AADL4-IVC-ETZWqF0AtPkCw1Db90Mjpkks5rtmZjgaJpZM4M3CiD .

[mcollina]

Fixed in v2.2.9, hopefully.

[mcollina]

please confirm, I will leave it open.

[mcollina]

Closing now, this was solved.