Mqtt.js: Don't rely on node built-ins

[chrste90]

I'm trying to use mqtt.js in an angular v6 app.

The problem is that mqtt.js uses process which doesn't exist in a browser environment.
In older versions of angular the cli provided node-shims to get it working but as of v6 it doesn't do it anymore. See https://github.com/angular/angular-cli/issues/9827#issuecomment-369578814 for more context.

It would be nice if mqtt.js wouldn't rely on node build-ins like process.

[mcollina]

@chrste90 I would really like if we could do so. Would you like to send to highlight the main offenders?
I suspect this is shared also to some of our dependencies (I know of a few myself), so it would be very hard to fix.

Let me know if you plan to open a PR.

[chrste90]

@mcollina Oh, you are right, there are some more problematic dependencies. If i start an angular app, i get this error:

Uncaught ReferenceError: process is not defined
    at Object../node_modules/process-nextick-args/index.js (index.js:3)
    at __webpack_require__ (bootstrap:74)
    at Object../node_modules/readable-stream/lib/_stream_readable.js (_stream_readable.js:26)
    at __webpack_require__ (bootstrap:74)
    at Object../node_modules/readable-stream/readable-browser.js (readable-browser.js:1)
    at __webpack_require__ (bootstrap:74)
    at Object../node_modules/mqtt/lib/store.js (store.js:8)
    at __webpack_require__ (bootstrap:74)
    at Object../node_modules/mqtt/lib/client.js (client.js:7)
    at __webpack_require__ (bootstrap:74)

So i think the main problematic dependency is readable-stream (Or their dependency process-nextick-args). Do you know if it is possible to replace readable-stream with another dependency or should i try to bring up the error on their site? But if i look at their repo: https://github.com/nodejs/readable-stream, it looks like it is a library especially for node.js, so i don't think it would be approriate to change the use of node built-ins there.

Also for this repo, i just searched for process in the code and found out that there are some uses of process.nextTick(). I'm not very familiar with this but i think it should be possible to replace process.nextTick() with setTimeout()? If so, i could create a PR changing the behaviour (or create a switch to either use nextTick on node oder setTimeout on browser).

The uses of process.env in the tests should do no harm if one would try to use this lib in angular.

But for bin/sub.js and bin/pub.js there are uses of process.exit(). Is this code used by consumers or only for examples in this repo (sorry, i'm not very familiar with mqtt.js yet :))

[mcollina]

setTimeout has a completely different meaning than process.nextTick, and it would a _very slow_ replacement for Node.js.

I don't think we can easily change the behavior in readable-stream, but we can specify a browser field in process-nexttick-args package.json, to avoid using process in the browser.

But for bin/sub.js and bin/pub.js there are uses of process.exit(). Is this code used by consumers or only for examples in this repo (sorry, i'm not very familiar with mqtt.js yet :))

These are executables for Node.js, it should stay as it is.

[chrste90]

I don't think we can easily change the behavior in readable-stream, but we can specify a browser field in process-nexttick-args package.json, to avoid using process in the browser.

Okay i don't exactly understand what you mean by this. Is it something that can be done in this repo or should it be changed in process-nexttick-args?

Any other ideas what can be done with process.nextTick()?

[RangerMauve]

Ideally it should be replaced by the asap module that handles environment differences gracefully.

[mcollina]

Ideally it should be replaced by the asap module that handles environment differences gracefully.

We can't use that module because the behavior is different in Node.js. However, we can use the same technique that's in asap to not use process in there.

[chrste90]

Is it something you can do? I'm not very familiar with either one or the other, so it would be very hard for me do make the right changes and don't break anything in this lib.

[mcollina]

I've absolutely not bandwidth to do this.

[chrste90]

Okay, then i will need to find some time and make it working somehow.

For the Readable-Streams: Do you think it can be possible to change to https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream/ReadableStream or does the stream api behave differently than the readable-stream package which is used now?

[chrste90]

I also did some quick research and found out that the best alternative in browser for nextTick() should be Promise.resolve().then().

What about making a switch: In browser use Promise.resolve().then() and in an node environment process.nextTick()? Maybe somthing simple like:

if (process) {
    process.nextTick()
} else {
    Promise.resolve().then()
}

[sclausen]

@mcollina any opinion about this? If you agree I would like to prepare a PR.

[mcollina]

@sclausen unfortunately it's not possible for two reasons. Firstly, promises wrap every function in a try-catch, changing how error would work. Secondly, Promise is not available in all the supported of readable-stream.

The approach with setTimeout is fine. There is a need for a browser field in package.json to instruct to use that in the browser, so we can avoid to load process  completely.

[sclausen]

Everything I tried doesn't work and it's not only process that interferes, but also e.g. streams. You can't add the browserified bundle of mqtt.js to the [email protected] project.
I think I may rewrite mqtt.js to only use browser API like ReadableStream and WriteableStream, so it will become something like mqtt.browser.js or mqtt.browser.ts.

[RangerMauve]

Why can't you import a browserified version of mqtt.js? What did you use to create the browserify bundle?

This sounds more like an angular-cli problem than an mqtt.js problem.

[sclausen]

@RangerMauve: I actually can import a javascript file and browserifying mqtt.js works like a charm, but the file doesn't get embedded in the bundle angular-cli (or ng-packagr) creates afterwards.

This sounds more like an angular-cli problem than an mqtt.js problem.

Yes, yes it totally is.

angular-cli is an awesome project but such a major breaking change without any migration options is very... unpleasant. I totally understand why they decided to remove the node-shims, but without any other options to achieve such goals? Not nice.

TL;DR

I think we're done here. However we get to fix this issue with our library is nothing which has to be achieved in this project.

[RangerMauve]

Sorry to hear you got screwed over so hard! Do post any updates if you find a clever workaround!

[sclausen]

I think the best workaround is not switching to angular-cli as a build tool for my library and staying with manual webpack and browserify. After all it worked like a charm in the past.

[Mbonea-Mjema]

Hey am new to all this and I would like to use mqtt and I keep getting the Reference error

[sclausen]

Hey am new to all this and I would like to use mqtt and I keep getting the Reference error

The first thing you should do is reading this thread. Then you should understand what has been written and then you should know how to deal with the mentioned error.

[Mbonea-Mjema]

@sclausen Thanks for the quick response..but am totally new to nodejs and angular..can you give me some steps to follow or something

[sclausen]

You can browserify mqtt.js as described in the docs here, then you put the file in a folder of your angular project like that src/vendor/mqtt.js.
Then you can import the mqtt client like the following import * as mqttClient from '../vendor/mqtt';

[Mbonea-Mjema]

Thanks for the help... I used the "browserify'd" version of mqtt.js and It worked like a charm here is a link
https://gist.github.com/ajsharp/4178e69d3b545ec0a437