New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

`Uncaught TypeError` when bundled with browserify #150

Open
alrik opened this Issue Apr 12, 2018 · 10 comments

Comments

Projects
None yet
6 participants
@alrik

alrik commented Apr 12, 2018

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

alrik added a commit to realmq/realmq-web-sdk that referenced this issue Apr 13, 2018

@rianwouters

This comment has been minimized.

Show comment
Hide comment
@rianwouters

rianwouters May 4, 2018

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 awslabs/aws-mobile-appsync-sdk-js#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 #150
window.Paho = {MQTT};

Better workarounds appreciated!

rianwouters commented May 4, 2018

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 awslabs/aws-mobile-appsync-sdk-js#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 #150
window.Paho = {MQTT};

Better workarounds appreciated!

@konoufo

This comment has been minimized.

Show comment
Hide comment
@konoufo

konoufo Jun 12, 2018

Are there any plans to fix this ?

konoufo commented Jun 12, 2018

Are there any plans to fix this ?

@deftomat

This comment has been minimized.

Show comment
Hide comment
@deftomat

deftomat Aug 2, 2018

Still nothing?

deftomat commented Aug 2, 2018

Still nothing?

@icraggs

This comment has been minimized.

Show comment
Hide comment
@icraggs

icraggs Aug 3, 2018

Contributor

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?

Contributor

icraggs commented Aug 3, 2018

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

This comment has been minimized.

Show comment
Hide comment
@konoufo

konoufo Aug 3, 2018

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

konoufo commented Aug 3, 2018

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

@icraggs

This comment has been minimized.

Show comment
Hide comment
@icraggs

icraggs Aug 3, 2018

Contributor

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

Contributor

icraggs commented Aug 3, 2018

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

@konoufo

This comment has been minimized.

Show comment
Hide comment
@konoufo

konoufo commented Aug 4, 2018

@icraggs

This comment has been minimized.

Show comment
Hide comment
@icraggs

icraggs Aug 6, 2018

Contributor

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.

Contributor

icraggs commented Aug 6, 2018

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

This comment has been minimized.

Show comment
Hide comment
@konoufo

konoufo Aug 6, 2018

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

konoufo commented Aug 6, 2018

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

@MrSumant

This comment has been minimized.

Show comment
Hide comment
@MrSumant

MrSumant Aug 25, 2018

@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

MrSumant commented Aug 25, 2018

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment