MQTT.js icon indicating copy to clipboard operation
MQTT.js copied to clipboard

Browser detection using global __webpack_require__ is misleading

Open ArminEberle opened this issue 4 years ago • 8 comments

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

ArminEberle avatar Jan 15 '21 15:01 ArminEberle

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,

lopengil avatar May 12 '21 21:05 lopengil

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 :-)

Symsystem avatar Aug 24 '21 14:08 Symsystem

@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?

YoDaMa avatar Aug 24 '21 20:08 YoDaMa

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 ;-)

Symsystem avatar Aug 25 '21 21:08 Symsystem

@Symsystem hi i'm sorry for the delay I've been mulling this change over. This will contradict the PR created here: https://github.com/mqttjs/MQTT.js/commit/eedc2b26cd6063a0b1152432a00f70de5e0b9bae

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.

YoDaMa avatar Dec 15 '21 19:12 YoDaMa

Any progress? or have any workaround?

navono avatar Aug 30 '22 02:08 navono

check this: https://github.com/stomp-js/stompjs/issues/28

navono avatar Aug 30 '22 03:08 navono

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.

ArminEberle avatar Sep 07 '22 11:09 ArminEberle

Fixed by https://github.com/mqttjs/MQTT.js/pull/1571

robertsLando avatar Jun 23 '23 14:06 robertsLando

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.

yourichabert avatar Jul 11 '23 14:07 yourichabert

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

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

?

robertsLando avatar Jul 11 '23 14:07 robertsLando

@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.

yourichabert avatar Jul 11 '23 14:07 yourichabert

Try:

import { MqttClient, connect } from 'mqtt/dist/mqtt.min';

robertsLando avatar Jul 11 '23 14:07 robertsLando

@robertsLando I just tried and unfortunately it has failed the same way.

yourichabert avatar Jul 11 '23 15:07 yourichabert

In the end, as a workaround, I updated my WebPack config to make WebPack replace lines of code matching regex /const IS_BROWSER =.*\n/ with const IS_BROWSER = false;\n, using the string-replace-loader loader.

webpack.config.ts:

                {
                    test: /\.[jt]s$/,
                    include: path.resolve(__dirname, 'app', 'node_modules', 'mqtt'),
                    use: {
                        loader: 'string-replace-loader',
                        options: {
                            search: /const IS_BROWSER =.*\n/,
                            replace: 'const IS_BROWSER = false;\n'
                        }
                    }
                },

yourichabert avatar Jul 12 '23 16:07 yourichabert

check this: stomp-js/stompjs#28

It works for me! Thanks!

zichil avatar Aug 07 '23 03:08 zichil