Skip to content

Add missing socket.io to Ros.js#429

Closed
MatthijsBurgh wants to merge 1 commit intodevelopfrom
missing_io
Closed

Add missing socket.io to Ros.js#429
MatthijsBurgh wants to merge 1 commit intodevelopfrom
missing_io

Conversation

@MatthijsBurgh
Copy link
Copy Markdown
Contributor

No description provided.

@MatthijsBurgh MatthijsBurgh requested a review from a team April 21, 2021 11:54
@Rayman
Copy link
Copy Markdown
Contributor

Rayman commented Apr 21, 2021

Does the socket.io layer even work? I've asked the question before but I didn't get an answer: #322

@MatthijsBurgh
Copy link
Copy Markdown
Contributor Author

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

But at least I know io wasn't imported.

@J-Rojas
Copy link
Copy Markdown
Contributor

J-Rojas commented Apr 23, 2021

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.

@MatthijsBurgh
Copy link
Copy Markdown
Contributor Author

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

@francoissunatori
Copy link
Copy Markdown

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

@MatthijsBurgh
Copy link
Copy Markdown
Contributor Author

@francoissunatori see #429 (comment)

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.

k-aguete pushed a commit to k-aguete/roslibjs that referenced this pull request Oct 21, 2022
Bumps [mocha](https://github.com/mochajs/mocha) from 9.0.2 to 9.0.3.
- [Release notes](https://github.com/mochajs/mocha/releases)
- [Changelog](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md)
- [Commits](mochajs/mocha@v9.0.2...v9.0.3)

---
updated-dependencies:
- dependency-name: mocha
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants