Can't use it with yarn2

JVictorV commented 4 years ago

I'm submitting a...


[ ] Regression 
[x] Bug report
[x] Feature request
[ ] Documentation issue or request
[ ] Support request

Current behavior

outputs: ⚡ We will scaffold your app in a few seconds.. and do not create my project

Expected behavior

A new folder with the project

Minimal reproduction of the problem with instructions

npm install -g yarn@berry
cd /to/your/projects/folder
yarn dlx -p @nestjs/cli nest new my-nest-app

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

Yarn team recently released Yarn berry, the 2.0 version of yarn with a lot of new cool stuff, but unfortunately they don't support global packages the same way yarn 1 does. I think that adding support to it would be a nice feature

Environment

 _   _             _      ___  _____  _____  _     _____
| \ | |           | |    |_  |/  ___|/  __ \| |   |_   _|
|  \| |  ___  ___ | |_     | |\ `--. | /  \/| |     | |
| . ` | / _ \/ __|| __|    | | `--. \| |    | |     | |
| |\  ||  __/\__ \| |_ /\__/ //\__/ /| \__/\| |_____| |_
\_| \_/ \___||___/ \__|\____/ \____/  \____/\_____/\___/

[System Information]
OS Version     : Linux 5.4
NodeJS Version : v13.7.0
NPM Version    : 6.13.6 

[Nest CLI]
Nest CLI Version : 6.14.1 

[Nest Platform Information]
😼  cannot read your project package.json file, are you inside your project directory?

(No package.json because i could'nt create the project)

kamilmysliwiec commented 4 years ago

I think that "Yarn 2" (it actually should never be called Yarn 2, it's a completely different library) currently doesn't support many other things as well. I would recommend sticking with "Yarn 1" for now.

MarcusCemes commented 4 years ago

Yarn Berry (2) introduced some pretty hefty changes, the most notable being forced adoption of their PnP module resolution scheme, which requires patching of Node.JS's require API. This is usually done transparently by yarn (yarn node, yarn run build) or by packages that support PnP, such as babel, webpack, even the typescript compiler. Problems arise when tools expect a node_modules folder, and try to execute .bin scripts directly.

There are some pretty significant benefits to PnP compared to the current state of npm, however, such as reducing the NestJS starter template from 193 MB of node_modules dependencies to only 55 MB (size on disk), and reducing the number of files, (which is arguably much more important) from 31,873 to only 1,904 (with a similar reduction in folder count as well). This means faster start-up times, faster installs, much faster deployment with bundled dependencies, and even the possibility of actually committing dependencies due to a more deterministic and flat install structure.

Plus, with Yarn's native workspaces, it's easier than ever to set up a monorepo with nested dependencies that are correctly installed, scoped and resolved from the root, without the lerna symlink bootstrap pain, and eventual crying when you realise that by using typescript, you need an additional prebuild step and deployment of the entire node_modules folder if you decide to symlink any packages.

The node_mobules folder is a long-running joke and despite PnP being far from standard, it might be worth taking a look into. npm seem to have abandoned their tink project last year (main dev left the company), and many major tools, such as Gatsby, CRA, seem to be looking into supporting PnP, with others such as Jest, Typescript, Webpack, Babel working already (mostly) out of the box.

murbanowicz commented 4 years ago

@kamilmysliwiec could you please reconsider reopening this issue? Yarn2 can really improve the DX.

phryneas commented 4 years ago

A problem why @nestjs/cli won't start is the complex manual resolution algorithm found in https://github.com/nestjs/nest-cli/blob/master/lib/compiler/typescript-loader.ts that only searches for typescript on disk & in node_modules folders. Those don't exist any more with yarn-pnp.

As a dirty workaround, you can just patch that, though.

Include this code in your project as .yarn/patches/@nestjs__cli.patch

diff --git a/lib/compiler/typescript-loader.js b/lib/compiler/typescript-loader.js
index 0011917c..3a45f378 100644
--- a/lib/compiler/typescript-loader.js
+++ b/lib/compiler/typescript-loader.js
@@ -4,6 +4,7 @@ const fs_1 = require("fs");
 const path_1 = require("path");
 class TypeScriptBinaryLoader {
     load() {
+        return require("typescript");
         if (this.tsBinary) {
             return this.tsBinary;
         }

and then edit your package.json to require the module in its patched version:

"@nestjs/cli": "patch:@nestjs/cli@7.1.5#.yarn/patches/@nestjs__cli.patch"

TimonLukas commented 4 years ago

@kamilmysliwiec Since the fix for this seems pretty simple, isn't it worth to take another look at this? After trying around a bit with @phryneas fix (thanks for sharing that!) I couldn't find anything that doesn't work as expected (albeit with a very simple setup).

A naive fix could be:

class TypeScriptBinaryLoader {
    load () {
        if ('BERRY_BIN_FOLDER' in process.env.) {
            return require('typescript')
        }
        // rest of loading routine
    }
}

BERRY_BIN_FOLDER is an environment variable that is injected by yarn 2. But I'm sure there's a better way to check whether the tool is running through yarn 2 - maybe @arcanis can weigh in whether there's an "official" way to identify this?

arcanis commented 4 years ago

You can check for process.versions.pnp, which is only set within PnP environments.

TimonLukas commented 4 years ago

Wow, that was fast! Thank you.

Maybe another argument for implementing this - I created a new project using nest new project and compared installation times. A completely blank project of course is not a realistic test, but it does show that there is quite some difference (before every run all local cache was removed):

yarn 2 with PnP enabled:

❯ time yarn
➤ YN0000: ┌ Resolution step
➤ YN0060: │ pakclone@workspace:. provides eslint@npm:7.1.0 with version 7.1.0 which doesn't satisfy ^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 requested by eslint-plugin-import@npm:2.21.2
➤ YN0000: └ Completed in 0.22s
➤ YN0000: ┌ Fetch step
➤ YN0013: │ yallist@npm:3.1.1 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ yallist@npm:4.0.0 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ yargs-parser@npm:18.1.3 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ yargs@npm:15.3.1 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ yn@npm:3.1.1 can't be found in the cache and will be fetched from the remote registry
➤ YN0000: └ Completed in 2.71s
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed in 2.21s
➤ YN0000: Done with warnings in 5.27s
yarn  4.76s user 3.77s system 145% cpu 5.849 total

yarn 2 with nodeLinker: node-modules:

❯ time yarn
➤ YN0000: ┌ Resolution step
➤ YN0060: │ pakclone@workspace:. provides eslint@npm:7.1.0 with version 7.1.0 which doesn't satisfy ^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 requested by eslint-plugin-import@npm:2.21.2
➤ YN0000: └ Completed in 0.23s
➤ YN0000: ┌ Fetch step
➤ YN0013: │ yallist@npm:3.1.1 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ yallist@npm:4.0.0 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ yargs-parser@npm:18.1.3 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ yargs@npm:15.3.1 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ yn@npm:3.1.1 can't be found in the cache and will be fetched from the remote registry
➤ YN0000: └ Completed in 2.77s
➤ YN0000: ┌ Link step
➤ YN0007: │ @nestjs/core@npm:7.1.3 [c2e8d] must be built because it never did before or the last one failed
➤ YN0007: │ fsevents@patch:fsevents@npm%3A1.2.13#builtin<compat/fsevents>::version=1.2.13&hash=495457 must be built because it never did before or the last one failed
➤ YN0000: └ Completed in 26.88s
➤ YN0000: Done with warnings in 30.01s
yarn  18.38s user 23.86s system 137% cpu 30.617 total

yarn 1:

yarn install v1.22.4
[1/4] 🔍  Resolving packages...
warning @nestjs/cli > webpack > watchpack > watchpack-chokidar2 > chokidar@2.1.8: Chokidar 2 will break on node v14+. Upgrade to chokidar 3 with 15x less dependencies.
warning @nestjs/cli > webpack > watchpack > watchpack-chokidar2 > chokidar > fsevents@1.2.13: fsevents 1 will break on node v14+ and could be using insecure binaries. Upgrade to fsevents 2.
warning @nestjs/cli > fork-ts-checker-webpack-plugin > micromatch > snapdragon > source-map-resolve > resolve-url@0.2.1: https://github.com/lydell/resolve-url#deprecated
warning @nestjs/cli > fork-ts-checker-webpack-plugin > micromatch > snapdragon > source-map-resolve > urix@0.1.0: Please see https://github.com/lydell/urix#deprecated
warning jest > jest-cli > jest-config > jest-environment-jsdom > jsdom > request@2.88.2: request has been deprecated, see https://github.com/request/request/issues/3142
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
warning " > eslint-plugin-import@2.21.2" has incorrect peer dependency "eslint@^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0".
[4/4] 🔨  Building fresh packages...
success Saved lockfile.
✨  Done in 21.75s.
yarn  11.42s user 19.60s system 141% cpu 21.970 total

kamilmysliwiec commented 4 years ago

@TimonLukas If this can be fixed with ~1 additional condition without affecting anything else, PRs are more than welcome. :)

climmmm commented 4 years ago

It'd be great if NestJS could be deployed with Yarn 2. Indeed, the monorepo presented in the NestJs documentation is very good but it turns out to be quite greedy in terms of dependencies. The node_modules directory can be very fat. Moreover, when using the workspaces of Yarn 1, it is impossible to install only the dependencies for a single application.

Thanks for the ticket. Hopefully it will benefit the whole community!

kamilmysliwiec commented 4 years ago

It should be fixed in 7.4.0

StefanoAgazzi commented 4 years ago

@TimonLukas hi, I am trying to create a monorepo with different nest API and services but it is not clear to me how to do it with yarn workspaces. I think mainly because it is not clear to me how to create yarn workspaces, yarn doc is not helping me a lot.. Could you point me to or provide an example of nest monorepo using yarn workspaces, please?

TimonLukas commented 4 years ago

@StefanoAgazzi I haven't used Nest with a workspaces setup yet, but I have used workspaces in other projects and it's actually quite simple to set up.

Let's go with a very simple example, you have a project that has a frontend and a backend, and you want them managed using workspaces (not the example where you get the most out of the workspaces, but it's good enough to show the process. The folder structure could look like this:

project-folder/
    package.json
    frontend/
        package.json
    backend/
        package.json

Then, in your project-folder/package.json you add a new entry "workspaces": ["frontend", "backend"]. This tells yarn that the projects in the directories frontend and backend are workspaces of this project.

That's everything you have to do! Now, if you install the dependencies by executing yarn in project-folder/, all dependencies from both the frontend and the backend will be installed.

Ghirigoro commented 3 years ago

@TimonLukas: Sorry if I'm confused by this thread but is yarn v2 supported by NestJS? The comment above by @kamilmysliwiec makes me think that you wrote a PR fixing it and it is supported in version > 7.4.0. I've tried converting an existing next project into a yarn v2 monorepo but am unable to get it working (see below for details). Am I misreading this issue or just doing something wrong in the configuration?

Yarn v2 Issues

I changed the package.json scripts to use yarn instead of referencing node_modules/bin:

"prebuild": "yarn rimraf dist",
"build": "yarn nest build",
"start": "yarn nest start",
etc...

Using this I can use start:dev to compile but end up with an error running: [PackageLoader] No driver (HTTP) has been selected. In order to take advantage of the default driver, please, ensure to install the "@nestjs/platform-express" package ($ npm install @nestjs/platform-express).

andreialecu commented 3 years ago

@Ghirigoro I have various projects running NestJS on Yarn 2 with PnP successfully.

At a minimum, you may need to add the following to your .yarnrc.yml:

packageExtensions:
  "@nestjs/core@*":
    peerDependenciesMeta:
      "@nestjs/platform-express":
        optional: true
  "@nestjs/graphql@*":
    dependencies:
      apollo-env: 0.6.5
      graphql-tools: 6.0.13
    peerDependencies:
      apollo-server-express: "*"

Then run yarn install

I have opened various PRs across @nest repositories to fix compatibility, but they're yet to be reviewed and merged. See https://github.com/nestjs/nest/pull/5477 https://github.com/nestjs/graphql/pull/1165

Also see https://github.com/nestjs/nest-cli/pull/899 which you can temporarily patch by adding

  "resolutions": {
    "@nestjs/cli": "patch:@nestjs/cli@7.5.1#./.yarn/patches/nest-cli.patch"
  }

and this patch:

https://gist.github.com/andreialecu/f29591e693e27575745fda7e21eaf865

Ghirigoro commented 3 years ago

@andreialecu: Thanks! Adding the @nestjs/core@* extension helped. I'm now stuck on: The "class-validator" package is missing. Please, make sure to install this library ($ npm install class-validator) to take advantage of ValidationPipe. That is installed and was previously working fine. Do you know if there's a workaround for that as well?

andreialecu commented 3 years ago

You probably need to add it to packageExtensions as well as an optional dependency of @nestjs/core

Ghirigoro commented 3 years ago

@andreialecu I've tried that but with no success. Or at least I thought I did. This is what I have for my yarnrc.yml:

yarnPath: ".yarn/releases/yarn-berry.cjs"
packageExtensions:
  "@nestjs/core@*":
    peerDependencies:
      "class-validator": "*"
    peerDependenciesMeta:
      "class-validator":
        optional: true
      "@nestjs/platform-express":
        optional: true

In any case thanks for all your help

andreialecu commented 3 years ago

@Ghirigoro I think those should work. Remember to run yarn install after updating the settings.

Ghirigoro commented 3 years ago

@andreialecu I ran yarn install and also tried deleting the unplugged and cache folders, and ran yarn install again. No luck. Could this be related to https://github.com/nestjs/nest/issues/1386#issue-392704146 ?

andreialecu commented 3 years ago

@Ghirigoro do you have class-validator as a dependency in your workspace? A small repro would also help if you could create one, I have some experience with Yarn 2 and Nest and could take a look.

Ghirigoro commented 3 years ago

@andreialecu I do have both class-validator and class-transformer in my dependencies:

    "dependencies": {
        "@nestjs/bull": "^0.2.2",
        "@nestjs/common": "^7.4.4",
        "@nestjs/core": "^7.4.4",
        "@nestjs/platform-express": "^7.4.4",
        "aws-sdk": "^2.771.0",
        "bull": "^3.18.0",
        "class-transformer": "^0.3.1",
        "class-validator": "^0.12.2",
        "dotenv": "^8.2.0",
        "helmet": "^4.1.1",
        "mobx": "5.15.7",
        "mobx-state-tree": "^3.17.2",
        "module-alias": "^2.2.2",
        "reflect-metadata": "^0.1.13",
        "rimraf": "^3.0.2",
        "rxjs": "^6.5.4",
        "tldts": "^5.6.60"
    },

I'll try stripping the project down to just the nest bit and see if the problem is still there. If it is I'll post it. Thanks!

Ghirigoro commented 3 years ago

@andreialecu I made a stripped down project that recreates the issue. If and when you have time you can find it here: https://github.com/Ghirigoro/yarnv2andNest

andreialecu commented 3 years ago

@Ghirigoro it seems that those two dependencies are not used by @nestjs/core but by @nestjs/common instead. I overlooked that part, sorry.

It should work with these extensions:

packageExtensions:
    "@nestjs/core@*":
        peerDependenciesMeta:
            "@nestjs/platform-express":
                optional: true

    "@nestjs/common@*":
        peerDependenciesMeta:
            "class-validator":
                optional: true
            "class-transformer":
                optional: true

For reference on how I found them, look here: https://github.com/nestjs/nest/blob/85c6be1a4094f9abc6699984b9b564eea54edc38/packages/common/pipes/validation.pipe.ts#L63-L68

They should be marked as peer deps, or optional deps in https://github.com/nestjs/nest/blob/85c6be1a4094f9abc6699984b9b564eea54edc38/packages/common/package.json but they are not.

Ghirigoro commented 3 years ago

@andreialecu Yeah, I set pnpMode to loose and it helped me track things down to common instead of core but you beat me to the punch in terms of posting. The yarnrc you shared fixed it. Thanks so much for digging into this and all your help.

Should the missing dependencies be flagged as bugs or is this part of a future upgrade to yarn v2 support?

andreialecu commented 3 years ago

They're bugs, essentially.

It only worked with npm and yarnv1 because the installations used to be loose, and they could hijack dependencies from other packages. But yarn 2 and pnpm enforce stricter dependency trees, and these bugs started surfacing.

I'm waiting for more feedback from @kamilmysliwiec here: https://github.com/nestjs/nest/pull/5477

But I think a core NestJS maintainer should go over all dependencies and ensure everything is properly depended on.

nestjs / nest-cli