max-mapper / websocket-stream

websockets with the node stream API
BSD 2-Clause "Simplified" License
668 stars 114 forks source link

require('ws').Server causes undefined error on client #113

Closed alexleonescalera closed 7 years ago

alexleonescalera commented 7 years ago

I'm using this library with React Native via aws-iot-device-sdk. React Native uses browser WebSockets, therefore it is going for the ws-fallback.js instead of the ws package.

https://github.com/maxogden/websocket-stream/blob/master/server.js#L3 is failing since the browser WebSocket object doesn't have a Server property.

mcollina commented 7 years ago

I do not think 'ws' can run on react-native. You should check with them.

cc @lpinca.

lpinca commented 7 years ago

React Native uses browser WebSockets

Yes, in this case you should use the native WebSocket object which obviously doesn't work as a server.

alexleonescalera commented 7 years ago

Thanks for the feedback. I was thinking more between the lines of conditionally include the server code inside the index.js. Something like:

module.exports = require('./stream.js')

if (!WebSockets) {
  var Server = require('./server.js')
  module.exports.Server = Server 
  module.exports.createServer = Server
}
mcollina commented 7 years ago

because of this line this should be done by your bundler. We may need some special directives for react-native., something like: https://github.com/nodejs/readable-stream/blob/master/package.json#L44. Would you mind firing up a PR that address this issue?

alexleonescalera commented 7 years ago

Sure, I'll look into it and send a PR. Thanks for the info.

EDIT: this seems to be an issue with React Native's packager. I tried with Webpack, and the browser field works just fine. I commented about it on MQTT.js repo since that thread seems to have deeper insight into this particular issue: https://github.com/mqttjs/MQTT.js/issues/573#issuecomment-293376128

alexleonescalera commented 7 years ago

FYI: opened an issue on React Native: https://github.com/facebook/react-native/issues/13474