nestjs / nest-cli

CLI tool for Nest applications 🍹
https://nestjs.com
Other
1.95k stars 390 forks source link

assets in nest-cli.json should be independant from sourceRoot #567

Closed mscudlik closed 4 years ago

mscudlik commented 4 years ago

I'm submitting a...

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

Current behavior

In the compilerOptions of nest-cli.json you can specify an array of assets. Unfortunately the path for assets re-uses sourceRoot (where the project files are). As a consequence it is not possible to run nest build and include files from the project root using i.e. src as sourceRoot

UseCase: i am using typeorm and the ormconfig.js configuration file of typeorm, which is expected to be in the project root folder. Note: i have to place my configuration there (and not i.e. in app.module.ts) because i want to be able to use typeorm-cli as well (for the migrations) and i don't want to duplicate the configuration.

Problem: the ormconfig.json/ts/js is not copied to dist folder running nest build. Adding "assets" : [ "ormconfig.js" ] does not work because assets uses src as root folder Adding "assets" : [ "../ormconfig.js" ] does not work because src is considered as root which you can't go up

nest-cli.json

{
  "collection": "@nestjs/schematics",
  "sourceRoot": "src",
  "compilerOptions": {
    "deleteOutDir": true
  }
}

tsconfig.json

{
  "compilerOptions": {
    "module": "commonjs",
    "declaration": true,
    "removeComments": true,
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "target": "es2017",
    "sourceMap": true,
    "outDir": "./dist",
    "baseUrl": "./",
    "incremental": true
  },
  "exclude": ["node_modules", "dist"]
}

Expected behavior

the sourceRoot should not be considered for assets, or it should be possible to specify an assetRoot in nest-cli.json, so that it is possible to put files (like ormconfig) in dist after nest build (without using additional tools like copyfiles)

"assets" : [ "ormconfig.js" ]

Environment


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

[System Information]
OS Version     : Windows 10
NodeJS Version : v12.14.1
NPM Version    : 6.13.4

[Nest CLI]
Nest CLI Version : 6.14.1

[Nest Platform Information]
@nestjsx/crud-typeorm version : 4.4.1
platform-express version      : 6.11.1
@nestjsx/crud version         : 4.4.1
passport version              : 6.1.1
swagger version               : 4.2.1
typeorm version               : 6.2.0
common version                : 6.11.1
config version                : 0.1.0
core version                  : 6.11.1
kamilmysliwiec commented 4 years ago

TypeORM CLI allows you to specify which config should be used with the --config flag:

typeorm migration:generate --config src/ormconfig.js

Hence, you can move it into the src directory.

mscudlik commented 4 years ago

Hi Kamil,

using the cli yes it does allow to specify a location, but then it doesn't work anymore for running the nest app. Unfortunately it is only looking in the project root (at least according to the documentation:

https://typeorm.io/#/using-ormconfig

The goal is to have only one configuration - for the app and the migrations and for multiple environments (local development, cloud). I could import the file and pass it to TypeOrmModule.forRoot(<here>), but isn't that a bit hacky when ormconfig should be loaded by convention with a specific name and a specific location?

Thanks and best regards, Matthias

SkySails commented 2 years ago

I have the exact same issue. I already tried moving the configuration to the /src directory just like @kamilmysliwiec suggested, but that breaks the implementation as @mscudlik already has explained.

I agree with @mscudlik in that duplicating the config is not DRY and that it should be possible to use the TypeORM module using the standards that come with the TypeORM library, without manual intervention like keeping track of the config file yourself.

I currently need to manually include the ormconfig-file when building, and since I import dependencies inside of said config file, I need to apply manual patches in order to make the import paths valid.


I currently populate my nest-cli.json file with the following:

{
  "collection": "@nestjs/schematics",
  "sourceRoot": "src",
  "compilerOptions": {
    "plugins": [{ "name": "@nestjs/swagger", "options": { "introspectComments": true } }],
    "assets": [{ "include": "../environment/.env*", "outDir": "./dist/environment" }]
  }
}

As you can see, I am able to access a folder that is outside the src directory and include its contents in the build. How come that I cannot do this for files that are in the root directory as well? Attempting to do so just prompts me with a Can't go up that far error.

@kamilmysliwiec I fail to see the benefit of this restriction. Is it just there as a security measure? How would it be less secure to allow access to the project root directory, seeing as developers have to manually and explicitly include files?

Without looking further into it at all, the below patch works fine for my usecase but I have no idea what else this change might affect.

diff --git a/node_modules/@nestjs/cli/lib/compiler/helpers/copy-path-resolve.js b/node_modules/@nestjs/cli/lib/compiler/helpers/copy-path-resolve.js
index 3602615..efccace 100644
--- a/node_modules/@nestjs/cli/lib/compiler/helpers/copy-path-resolve.js
+++ b/node_modules/@nestjs/cli/lib/compiler/helpers/copy-path-resolve.js
@@ -16,7 +16,7 @@ function dealWith(inPath, up) {
     if (!up) {
         return inPath;
     }
-    if (depth(inPath) < up) {
+    if (depth(inPath) < up - 1) {
         throw new Error('cant go up that far');
     }
     return path.join(...path.normalize(inPath).split(path.sep).slice(up));