Browser detection using global __webpack_require__ is misleading

[ArminEberle]

Hi there,

we're using your library from a node standalone application.
It works fine when running our .js files as original.

Anyway when we build the node application using webpack, it seems to me that this has an unwanted impact on the browser detection in lib/connect/index.js and lib/connect/ws.js, effectively leading to connection problems.

We want to switch to a webpack build in order reduce our container sizes in kubernetes.

This is where it fails then:

ReferenceError: WebSocket is not defined
    at createBrowserWebSocket (/.build-tmp/webpack-dist/index.js:1993:16)
    at Object.browserStreamBuilder (/.build-tmp/webpack-dist/index.js:2019:16)
    at MqttClient.wrapper [as streamBuilder] (/.build-tmp/webpack-dist/index.js:2633:36)
    at MqttClient._setupStream (/.build-tmp/webpack-dist/index.js:395:22)
    at new MqttClient (/.build-tmp/webpack-dist/index.js:374:8)
    at Object.connect (/.build-tmp/webpack-dist/index.js:2635:16)

The line numbers are not correct of course in this scenario.

I'm just asking if you already know of this issue and could give me a hint, if not I can create a reproducible demo of the problem or bugfix fork if you want me to.

Best Regards,

Armin

AB#9138553

[lopengil]

Hi, is there any update with this issue? I'm having the same problem, when running the js files it works fine but when running the webpack bundle it fails here:

ReferenceError: WebSocket is not defined
    at createBrowserWebSocket (...\dist\main.js:1940:16)
    at Object.browserStreamBuilder (...\dist\main.js:1966:16)
    at MqttClient.wrapper [as streamBuilder] (...\dist\main.js:2567:36)
    at MqttClient._setupStream (...\dist\main.js:343:22)
    at new MqttClient (...\dist\main.js:322:8)
    at Function.connect (...\dist\main.js:2569:16)
    at MosquittoClient.connect (...\dist\main.js:17090:42)
    at ...\dist\main.js:16934:25
    at Generator.next (<anonymous>)

I added the following configuration in the webpack.config.js file to make the build process work:

...
    plugins: [
      new webpack.NormalModuleReplacementPlugin(/^mqtt$/, "mqtt/dist/mqtt.js"),
    ],

But still not working when running the built file. Any tips?

Best,

[Symsystem]

Hi there,
We have the exact same issue with our product. We build our product with Webpack for both platforms (Browsers and NodeJS), and we integrated MQTT.js as dependency. Unfortunately, impossible to get the right websocket implementation while running with NodeJS because the second part of that predicate is wrong :
const IS_BROWSER = (typeof process !== 'undefined' && process.title === 'browser') || typeof __webpack_require__ === 'function' (/lib/connect/ws.js, line 17)

The check of __webpack_require__ is true in any case !

We temporarily solved the issue building and wrapping the library ourselves and then we removed that second part of the predicate, it works well on both environments.

Could you consider to review the "IS_BROWSER" condition in the next versions? Or simply to accept that pull request.

Best :-)

[YoDaMa]

@Symsystem your PR removes the check that's been in the code for a couple years so I'm confused without any tests why you're sure it will always be true and why your PR fixes anything. Can you create a test for your PR, or provide me with steps to repro to demostrate the change fixes a bug?

[Symsystem]

Hi @YoDaMa , Thanks for reacting so fast !
I created a small project with instructions in the README.md file to reproduce the bug on you side here.
Probably just removing the second part of the predicate could cause issues with other configurations than mine (maybe the check was valid with older versions of webpack ?). But it seems wrong to guess the environment based on that variable, so we should definitely review the predicates in both ws.js and index.js.

Please tell me if you need anything else ;-)

[YoDaMa]

@Symsystem hi i'm sorry for the delay I've been mulling this change over. This will contradict the PR created here: eedc2b2

This seems to hit on the fact that we should probably not have runtime-based isomorphism... it seems fraught.

If you can provide a solution that works with compiling for webpack in Angular and with your use case, then we can fix it with that.

[navono]

Any progress? or have any workaround?

[navono]

check this: stomp-js/stompjs#28

[ArminEberle]

Coming back from my side I can report that we're building our node standalone project with esbuild now (much faster than webpack), so this circumvents the problem with the mqtt.js browser detection in a different way.
Anyway leaving this ticket open for the other requestors.

[robertsLando]

Fixed by #1571

[youri-c]

Hello,

I just encountered the issue with version 4.3.7, with a Node standalone application in which we use WebPack. Then I saw this thread and that the issue should be fixed by #1571, so I replaced version 4.3.7 with version 5.0.0-beta.2. But the issue is still present in 5.0.0-beta.2.

From having a look at #1571, this is logical, because in lib/is-browser.js, the test typeof __webpack_require__ === 'function' will still make IS_BROWSER true when using WebPack.

[robertsLando]

@yourichabert How are you importing mqtt lib? Are you using:

import mqtt from 'mqtt/dist/mqtt.min'

?

[youri-c]

@robertsLando wow, thanks for you reactivity!

I am importing it this way:

import { MqttClient, connect } from 'mqtt';

And then the error happens when calling connect.