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

[mcollina]

Original issue: mqttjs/MQTT.js#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 mqttjs/MQTT.js#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 ***@***.***> 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 <#277 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABE4n3p5hT-a13ssY9XL4q5LsTx2lkBfks5rtmPRgaJpZM4M3CiD> .

[mcollina]

another duplicate max-mapper/websocket-stream#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 ***@***.***> 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 > < #277 (comment) >, > 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 <#277 (comment)>, 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.