Skip to content
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

Missing type definitions in Socket.IO 3.x #3690

Closed
renkei opened this issue Nov 10, 2020 · 8 comments
Closed

Missing type definitions in Socket.IO 3.x #3690

renkei opened this issue Nov 10, 2020 · 8 comments
Milestone

Comments

@renkei
Copy link

renkei commented Nov 10, 2020

I've updated my TypeScript project from version 2.x to version 3.0.1 of socket.io. After that I cannot compile my project anymore because no type definitions of cors, cookie and component-emitter are known.

After adding them (@types/cors, @types/cookie, @types/component-emitter) to the devDependencies section of my project it works again, but this is neither documented in the migration guide nor feels it like a good solution to add a 3rdParty component and then to get compiler errors because of missing dependencies.

@darrachequesne
Copy link
Member

Hmm... according to this, it seems that the types must actually be included in the dependencies section, and not in the devDependencies. Let's do this!

darrachequesne added a commit that referenced this issue Nov 17, 2020
@renkei
Copy link
Author

renkei commented Nov 23, 2020

This solves the issue for missing typings of cors and cookie, but not for component-emitter, used by package socket.io-parser where @types/component-emitter is included in the devDependencies section only. So, even with socket.io 3.0.3 I got a compiler error because of missing @types/component-emitter. I still have to add this to my own devDependencies section, although this is an implementation detail of socket.io that is nowhere documented.

@P4sca1
Copy link
Contributor

P4sca1 commented Nov 25, 2020

@darrachequesne
Copy link
Member

Fixed by 118cc68 and socketio/socket.io-parser@4efa005.

Thanks!

@darrachequesne darrachequesne added this to the 3.0.0 milestone Nov 25, 2020
@namoscato
Copy link

Just an FYI that including @types/node as a direct dependency poses some challenges for browser-based projects (i.e. see microsoft/TypeScript#18588).

I just got here via a karma@v6 upgrade (which included karma-runner/karma#3586) in which I was forced to explicitly define TSConfig types to mitigate this issue. With that said, do @types/node actually need to be a direct dependency of this project?

@darrachequesne
Copy link
Member

@namoscato @types/node is required for the compilation, as we rely on the Node.js events package.

It seems to be a recommandation from the TypeScript engineers. I'm open to suggestions on this though.

@renkei
Copy link
Author

renkei commented Jan 17, 2021

In general, it can be somehow tricky to handle the problem that on one hand you need typings only during development for code completion in VSCode and for the transpilation step from typescript to javascript but on the other hand it could happen that you have to make typings available even on a production build of your package because otherwise a consumer of your package could run into the mentioned dependency issue above.

For me, a clean solution would be to put typings only to devDependencies, always. This would have the side effect that npm install --production would never install typings. But for a development environment it would mean that the package documentation of socket.io should look like this:

Installation

  1. Add socket.io to dependencies section
  2. In a TypeScript environment put the following typings to devDependencies
  • @types/node
  • @types/cors,
  • @types/cookie,
  • @types/component-emitter

To be honest, I've never seen packages which document the required typings details like that. Usually, the package developer puts the required typings to the dependencies section so that everything is silently resolved from the package consumer perspective. This way is easy and seems to be the best one but is maybe not the strictly correct one.

@namoscato
Copy link

Yeah, I don't really have any strong suggestions. @renkei summarizes the situation well.

I could open an issue with Karma to see if they have any thoughts / preferences? This is only tangentially related to that project, but I could see this tripping up many users as part of their v6 upgrade.

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

No branches or pull requests

4 participants