platform-ws adapter lacks required websockets dependency

[dcapaccio]

I'm submitting a...

[ ] Regression 
[x ] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

When using the @nestjs/platform-ws adapter, the function useWebSocketAdapter fails because there is no dependency on the npm installer for @nestjs/websockets which is necessary for the platform-ws adapter.

Expected behavior

When installing the @nestjs/platform-ws adapter in npm, the @nestjs/websockets library should be a downloaded dependency.

Minimal reproduction of the problem with instructions

Set up a project with Nest CLI, use npm to install the platform-ws adapter, and change the websocketadapter to platform-ws and build.

What is the motivation / use case for changing the behavior?

Platform-ws does not compile without the @nestjs/websockets library, therefore it should be a dependency.

Environment

Nest version: 6.3.0

For Tooling issues:
- Node version: 10.10.0
- Platform: Linux

Others:
Developing an app in an offline environment highlights issues of property dependency lists.

[Destreyf]

@dcapaccio

I don't believe this is a bug, i believe this in an intentional implementation for the platform adapters, if you look at the Gateway page the example provided for installing packages includes both the @nestjs/websockets and @nestjs/platform-socket.io packages.

It appears @nestjs/platform-ws has a peer dependency of @nestjs/websockets, you can see that here: https://github.com/nestjs/nest/blob/master/packages/platform-ws/package.json

[dcapaccio]

I suppose I may just not understand peer dependencies then. So by design, assuming that if you are using the @nestjs/platform-ws adapter, you would already need the @nestjs/websockets package in the app anyway?

I guess then, that instead of it being implemented as a dependency, the documentation for the WsAdapter could be modified to include it where the npm i @nestjs/platform-ws line is on this page: https://docs.nestjs.com/websockets/adapter

[Destreyf]

@dcapaccio

I have to agree that the documentation could be more clear with respect to the websockets library being required in order to install/use platform-ws.

I do think that platform-ws could have a hard dependency on websockets to simplify installation but i do not know the full reasoning behind adding it as a peer dependency instead of as a normal dependency.

It might be due to the nature of being able to mix versions or have slightly out of sync package versions for different packages as they get incrementally bumped for fixes but i think it would be best to have @kamilmysliwiec chime in on the reason behind this approach as i believe it was done deliberately.

[kamilmysliwiec]

Every shared Nest package should be installed as a peer dependency in order to share the metadata registry.

I have to agree that the documentation could be more clear with respect to the websockets library being required in order to install/use platform-ws.

Could you please create a corresponding issue in the docs repo?

[dcapaccio]

Sure, I'll create a corresponding issue in the other repo.

Every shared Nest package should be installed as a peer dependency in order to share the metadata registry.

Good to know. I'll make sure that next time I pull my dependencies on the internet I'll grab all of the packages under the nest scope. Developing offline makes this more difficult to know which packages I need vs. which ones are optional. But once I get the package right I should be good to go.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.