Websocket Module does not work with yarn workspaces

[BrunnerLivio]

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

The SocketModule gets imported using the library called "optional". Unfortunately optional has a bug , which does not resolve the node_modules path correctly.

Disclaimer: With yarn workspaces you have multiple node_modules folder. One in ${projectRoot}/node_modules and multiple in ${projectRoot}/packages/*/node_modules. The package optional does not handle this

Expected behavior

It should correctly resolve the path, even with yarn workspaces.

I think we have to remove the library optional and find another way...

Environment

Nest version: 5.3.10

 
For Tooling issues:
- Node version: XX  
- Platform:  

Wow this issue cost me so much time to track down. So happy I finally found the cause.

[marcus-sa]

+1 for removing it.
Solution would be to tell lerna to symlink the optional packages even though it's not defined in the dependencies.

[BrunnerLivio]

I think the nohoist option shoud do it. Will give feedback tomorrow.

However, I still think we should find another solution, even if I am able to fix it using nohoist.

[BrunnerLivio]

I am currently discussion with the owner of optional for a fix, but honestly the better way to go is to do not use something like optional packages, rather force the user to import a WebsocketModule or in his AppModule.

[BrunnerLivio]

Kinda forgot about this issue. I fixed it indeed with the nohoist option of yarn.

package.json

{
   [...],
    "workspaces": {
        "packages": [
            "packages/*"
        ],
        "nohoist": [
            "**/@nestjs/",
            "**/@nestjs/**"
        ]
    }
}

Here is a minimal reproduction of the bug:

https://github.com/BrunnerLivio/nestjs-monorepo-starter/tree/nohoist-bug

The README shows how it runs with or without the nohoist update.
This blog article from yarn explains pretty well what is going on:
https://yarnpkg.com/blog/2018/02/15/nohoist/

---

This bug is not urgent, since there is a working workaround. However I think hoisting will come more and more important in the NodeJS community. In my opinion there are more and more articles coming out because of the broken system npm has (example). Hoisting for example tries to fight against the enormous pile of redundant packages an npm install produces. For huge enterprise application this can be a life saver. Unfortunately nestjs is not built to work with yarn hoisting. I think we should definitely try to fix this in the future.

[kamilmysliwiec]

Let's track this here #3035

[lock]

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.