eclipse / paho.mqtt.javascript

paho.mqtt.javascript
Other
1.15k stars 467 forks source link

`Uncaught TypeError` when bundled with browserify #150

Closed alrik closed 5 years ago

alrik commented 6 years ago

Hey guys,

I'm currently crafting a realtime sdk and set up a build pipeline with browserify to includes paho.mqtt along with the custom sdk logic.

In the browserify scope the paho.mqtt is required as module.

An error gets raised when messages are send or receive, stating:

Uncaught TypeError: Cannot read property 'length' of undefined
    at ClientImpl.LibraryFactory.ClientImpl._on_socket_message (build.js:2064)
    at WebSocket.<anonymous>

I traced down the error and it hapends due to Paho.MQTT is no global in require context. If we would replace Paho.MQTT with PahoMQTT it will also make it work for browserify builds.

I could submit a PR.

Cheers, Alrik

rianwouters commented 6 years ago

Similar issue here using webpack. Befdore that TypeError I get a connectionLost because Paho is not defined, caused by the fact that Paho it tries to access Paho,MQTT.Message, which is not defined in case webpack is used (i.e. module.exports is defined)

Look very similar to https://github.com/awslabs/aws-mobile-appsync-sdk-js/issues/76 However, the workaround provided there does not work as it defines Paho as {}

I ended up with the following workaround:

import MQTT from 'paho-mqtt'; // Workaround https://github.com/eclipse/paho.mqtt.javascript/issues/150 window.Paho = {MQTT};

Better workarounds appreciated!

konoufo commented 6 years ago

Are there any plans to fix this ?

deftomat commented 6 years ago

Still nothing?

icraggs commented 6 years ago

Hi. The main maintainer of this library @jpwsutton has just gone on vacation for a week or so. He released an updated version in June - I don't know if this is fixed by this update. I'll try and contact him and find out.

What's the easiest way to test it for someone not used to dealing with javascript much, like me?

konoufo commented 6 years ago

@icraggs I could set up a really minimal example test in a repo. This afternoon...

icraggs commented 6 years ago

@konoufo that would be great. I've talked to James, and he thinks he's fixed it with the latest update.

konoufo commented 6 years ago

@icraggs Here it is: https://github.com/konoufo/paho-mqtt-bundle-test Very straightforward.

icraggs commented 6 years ago

Ok, thanks. That worked, as in the problem was reproduced. Then I replaced the paho-mqtt.js with the latest here and the test worked. So the problem is then that the fixed hasn't been packaged into the node.js bundle and/or other locations, presumably.

konoufo commented 6 years ago

Ok great. So at this point, the issue is resolved at this repository level in the last commit ?

MrSumant commented 6 years ago

@icraggs I have the same issue but it is working as expected when i changed the content of paho-mqtt.js with latest master of this repo. So when are guys planning on getting this version to NPM

pkuczynski commented 5 years ago

Any plans for a release @icraggs and @jpwsutton?

pkuczynski commented 5 years ago

@icraggs I can confirm this bug is fixed in the latest release 1.1.0 and can be closed.

icraggs commented 5 years ago

Thanks very much!