search

nestjs / swagger

OpenAPI (Swagger) module for Nest framework (node.js) :earth_americas:
https://nestjs.com
MIT License
1.69k stars 476 forks source link

Adapt relative paths resolution to tsconfig outDir depth #2706

Open mgrisole opened 1 year ago

mgrisole commented 1 year ago

Is there an existing issue for this?

Current behavior

This issue is almost a duplicate of https://github.com/nestjs/swagger/issues/2114, I can't figure out why this should be address in Discord since it is a real bug and we are facing exactly the same issue in a pnpm workspace.

I can get it working by performing this manual change on dist:

Possible related issues: https://github.com/nestjs/swagger/issues/1386 https://github.com/nestjs/swagger/issues/501

Minimum reproduction code

https://github.com/bindermuehle/monorepo-issue

Steps to reproduce

  1. npm i
  2. npm run start:dev -w nest

Expected behavior

nest/swagger should correctly resolve the paths

Package version

7.1.10

NestJS version

9.0.0

Node.js version

20.9.0

In which operating systems have you tested?

Other

No response

mgrisole commented 12 months ago

Removing one of these two statements fixes the issue, but I fail to understand the whole context at the moment: https://github.com/nestjs/swagger/blob/c4cbca6b6e7cc531132214e8dfd4654377e9447f/lib/plugin/utils/plugin-utils.ts#L153-L155 https://github.com/nestjs/swagger/blob/c4cbca6b6e7cc531132214e8dfd4654377e9447f/lib/plugin/utils/plugin-utils.ts#L192

aqeelat commented 3 months ago

@kamilmysliwiec We're also facing this issue.

We have a schemas package that's used in our frontends and backends.

Until this is fixed, our only options are:

  1. moving the entire package inside the nest src folder (bad idea)
  2. adding a path alias in tsconfig that points to the schemas package (also a bad idea)
jkalberer commented 1 month ago

This is a clear bug and I don't know why this PR was closed without any comment.

@aqeelat - can you give an example of the second option? (I'm trying to add paths to my tsconfig but can't figure it out) It's definitely not ideal but it beats waiting for this bug to actually get addressed.

jkalberer commented 1 month ago

Ok -- I tried to hack on it a bit but I'm not super familiar with the typescript / AST APIs.

So it seems that this code is trying to resolve the path relative to the source file instead of the destination file. _In my case, the issue is that I have a mono-repo with packages and this code is resolving a relative path instead of using the nodemodule reference.

Anyways, I'm wondering why it's trying to resolve the paths anyways as there is already an import expression in the file for the node.

Is there any reason why we can't just clone the existing node? Similar to what this is doing

I already time-boxed trying to come up with a solution (spent all yesterday) otherwise I'd open a PR going down that route but I think this is the right path forward and would allow us to get rid of a lot of code related to resolving the paths.

aqeelat commented 1 month ago

@aqeelat - can you give an example of the second option? (I'm trying to add paths to my tsconfig but can't figure it out) It's definitely not ideal but it beats waiting for this bug to actually get addressed.

Sorry @jkalberer, I just saw your mention. We didn’t touch tsconfig. We cd’ed into the src folder and ran something like ln -s ../../otherLibrary/src otherLibrary (make sure to use relative paths). It’s been working fine for us with no negative consequences. Highly recommend it.

You might face issues when you have different linting/formatting rules.