SocketCluster / socketcluster-client

JavaScript client for SocketCluster
MIT License
291 stars 92 forks source link

v5.0.15 will case error in react-native ios used in remote-redux-devtools #67

Closed feyy closed 6 years ago

feyy commented 7 years ago

image

zalmoxisus commented 7 years ago

@jondubois, here's the detailed log: https://github.com/zalmoxisus/remote-redux-devtools/issues/51#issuecomment-257514941

jondubois commented 7 years ago

@zalmoxisus @feyy Does it work with v5.0.14?

jondubois commented 7 years ago

This is the only change which was introduced in v5.0.15 https://github.com/SocketCluster/socketcluster-client/commit/6c7f4e17badddb5dbd0710442936c46c1149e3ec

jondubois commented 7 years ago

I can roll it back if that's the problem.

jondubois commented 7 years ago

It's strange because we use global here https://github.com/SocketCluster/socketcluster-client/blob/master/lib/scsocketcreator.js#L44 but it doesn't seem to cause any problems?

zalmoxisus commented 7 years ago

@jondubois, I didn't reproduce that. We updated the lib from socketcluster-client@4 to @5, so it's not related to a specific patch version I guess.

jondubois commented 7 years ago

@zalmoxisus Can you find out which version introduced the problem? It's quite hard to setup react native on my Linux machine so I can't test.

zalmoxisus commented 7 years ago

@jondubois, I tried earlier versions. It appears only on 5.0.15. Removing global.WebSocket || from here solves the issue. Strangely, it appears only on iOS, not for Android.

sjmueller commented 7 years ago

Can also confirm that this breaks react-native, indeed because of 6c7f4e1.

jondubois commented 7 years ago

@sjmueller @zalmoxisus Do you know if this issue has been resolved in newer versions of socketcluster-client? The logic that was causing problems was changed a while ago:

https://github.com/SocketCluster/socketcluster-client/blob/master/lib/sctransport.js#L7-L17

zalmoxisus commented 7 years ago

@jondubois the fix is landed on React Native >= 0.43, so the older versions of socketcluster-client should work as well.

jondubois commented 7 years ago

Thanks @zalmoxisus :)