roslibjs icon indicating copy to clipboard operation
roslibjs copied to clipboard

Add missing socket.io to Ros.js

Open MatthijsBurgh opened this issue 5 years ago • 6 comments

MatthijsBurgh avatar Apr 21 '21 11:04 MatthijsBurgh

Does the socket.io layer even work? I've asked the question before but I didn't get an answer: https://github.com/RobotWebTools/roslibjs/issues/322

Rayman avatar Apr 21 '21 14:04 Rayman

@Rayman I am not sure it works. It isn't tested.

But at least I know io wasn't imported.

MatthijsBurgh avatar Apr 21 '21 14:04 MatthijsBurgh

I believe the reason why this is left out is because socketio is non-essential and an optional integration whereas the other dependencies are critical to the function of roslib. Socketio is a relatively large library (60kb minified) larger than roslib is currently. So I would keep it as a peer dependency and not require it here.

J-Rojas avatar Apr 23 '21 16:04 J-Rojas

I am totally fine by making it a peer dependency, but right now it is marked as a normal dependency.

Also right now io is just not available inside ros.js

MatthijsBurgh avatar Apr 23 '21 20:04 MatthijsBurgh

Why wasn't this already merged? socket.io is already in package.json

francoissunatori avatar Jul 27 '22 16:07 francoissunatori

@francoissunatori see https://github.com/RobotWebTools/roslibjs/pull/429#issuecomment-825789600

The question remains if it works and whether it should be included in the compiled libs. If not included a user can always import it him/herself.

MatthijsBurgh avatar Jul 28 '22 03:07 MatthijsBurgh