nestjs / nest-cli

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

tsconfig-paths rewrite to be broken when import with file extension (.js) #1437

Open msimon opened 2 years ago

msimon commented 2 years ago

Is there an existing issue for this?

Current behavior

With a project organised that way:

src:
  - main.ts
  - app.module.ts
  ....
tsconfig.json
...

Defining an import in main.ts the following way:

import { AppModule } from 'src/app.module.js';

result in it being compiled to:

const app_module_1 = require("src/app.module.js");

ignoring the non-relative path to relative path rewrite that should be done by tsconfig-paths.

Removing the .js from the import, result in the correct compilation:

import { AppModule } from 'src/app.module';
const app_module_1 = require("./app.module");

I took a quick look into nest-cli/lib/compiler/hooks/tsconfig-paths.hook.ts to see if I could spot the issues. My guess is that the match on l.56 does not return anything if the extension is set.

I'll try to ding more into it soon, but I figure I would ask in case there is an obvious fix for this.

Minimum reproduction code

https://github.com/HomeskilletHealthInc/nestJs_import_rewrite_bug

Steps to reproduce

Run:

nest build
cat ./dist/main.js

This code:

import { AppModule } from 'src/app.module';
import { AppService } from 'src/app.service.js';

is compiled to:

const app_module_1 = require("./app.module");
const app_service_js_1 = require("src/app.service.js");

Failing to rewrite src when .js is precise.

Expected behavior

Defining an import in main.ts the following way:

import { AppModule } from 'src/app.module.js';

Should compile to:

const app_module_1 = require("./app.module.js");

Package version

8.1.5

NestJS version

8.2.3

Node.js version

v14.17.0

In which operating systems have you tested?

Other

The reason I would to get the .js is because it is required for ESM

jmcdo29 commented 2 years ago

Can you please provide a minimum reproduction repository?

kamilmysliwiec commented 2 years ago

BTW using absolute paths (using "src" in your import paths) is considered as a bad practice

msimon commented 2 years ago

@jmcdo29 https://github.com/HomeskilletHealthInc/nestJs_import_rewrite_bug

@kamilmysliwiec Using absolute path is, but I don't believe that applies to non-relative path. The code will always compile to the same relative path, independently of the service/computer is being compiled on. The gain of using non-relative import is so much higher imo. It's even supported out of the box by NestJs.

Let me know if I misses something, but I haven't seen any large projects using fully relative path, nor a good argument to use them.

kamilmysliwiec commented 2 years ago

Yeah just to clarify: it applies only to paths that contain src directory (so using rootDir + absolute paths)

Val-istar-Guo commented 2 years ago

I had the same problem and couldn't import an esm package. Do I have to change all absolute paths to relative paths. Is there any solution to solve this problem temporarily.

amit78523 commented 1 year ago

I had the same problem and couldn't import an esm package. Do I have to change all absolute paths to relative paths. Is there any solution to solve this problem temporarily.

I am using this command temporarily nest build && tsc-alias tsc-alias is a node module.

Ttou commented 10 months ago

same problem, https://github.com/microsoft/TypeScript/issues/56350

demaisj commented 1 month ago

Encountered this bug today, here's a minimum reproduction repo and steps: https://github.com/demaisj/nestjs-tsconfig-path-issuerepro

  1. npm install
  2. setup path alias in tsconfig.json (done in repo)
    "paths": {
    "@/*": ["./src/*"]
    }
  3. use alias import with .js extension (done in repo)
    import { AppService } from "@/app.service.js";
  4. npm run start
  5. observe the import error from nodejs runtime
    
    node:internal/modules/cjs/loader:1148
    throw err;
    ^

Error: Cannot find module '@/app.service.js'


We can see in the compiled `app.module.js` file that the import alias without js extension is correctly resolved `@/app.controller → ./app.controller` contrary to the import alias with explicit extension`@/app.service.js → @/app.service.js`
Resolving alias with explicit extension would also allow users to work in an ESM environment.
JeromeYangtao commented 1 month ago

I had the same problem and couldn't import an esm package. Do I have to change all absolute paths to relative paths. Is there any solution to solve this problem temporarily.

I am using this command temporarily nest build && tsc-alias tsc-alias is a node module.

Do you have any solution for nest start @amit78523