privatenumber / get-tsconfig

Lightweight tsconfig.json parser & paths resolver
MIT License
187 stars 13 forks source link

Does not handle correctly absolute paths in `baseUrl` and `paths` #50

Closed andreww2012 closed 1 year ago

andreww2012 commented 1 year ago

Nuxt 3.6.x generates a tsconfig file with absolute paths in baseUrl and paths fields. This causes eslint-import-resolver-typescript (which depends on get-tsconfig) to incorrectly report that imports with nuxt aliases cannot be resolved.

Reproduction

https://github.com/andreww2012/eslint-plugin-import-with-nuxt-3.6-false-reports

npm i
npm run repro
npm run repro:eslint
privatenumber commented 1 year ago

Can you update your reproduction to be minimal (e.g. only get-tsconfig as a dependency)? Currently, it's possible the bugs are in the get-tsconfig usages.

FWIW we have tests that prove absolute paths are handled: https://github.com/search?q=repo%3Aprivatenumber%2Fget-tsconfig+absolute+path%3A%2F%5Etests%5C%2Fspecs%5C%2F%2F&type=code

andreww2012 commented 1 year ago

@privatenumber npm run repro only uses get-tsconfig in the example. Could you verify whether its output correct or incorrect?

privatenumber commented 1 year ago

But your file loads a directory that isn't in the project: https://github.com/andreww2012/eslint-plugin-import-with-nuxt-3.6-false-reports/blob/03a4501c53ded2dba533066fbbd66f3d1f71c320/get-tsconfig.js#L3

I'm closing this for now, but I'm happy to re-open once the repo is minimal (get-tsconfig should be the only dependency) and all required files are present.

andreww2012 commented 1 year ago

You need to run npm i so nuxt generates its tsconfig. That's what the line you mentioned looks for. :) I could not include the generated tsconfig in that repo as it contains private absolute paths.

privatenumber commented 1 year ago

I looked into it and I'm able to reproduce it, but this reproduction could have been a lot more minimal. If you use parseTsconfig, the absolute paths do not need to be real.

Will fix soon.

andreww2012 commented 1 year ago

Thank you very much and sorry if the reproduction repo was too bloated. I tried to demonstrate my workflow and how I came to the need of absolute paths handling.

andreww2012 commented 1 year ago

I just tried 4.6.1, and running npm run repro from my test repo now yields the following:

{
  baseUrl: { broken: '..', correct: '..' }, // fixed
  matcher: {
    broken: [
      'C:/Users/my-username/Documents/a/b/eslint-plugin-import-with-nuxt-3.6-false-reports/C:/Users/my-username/Documents/a/b/eslint-plugin-import-with-nuxt-3.6-false-reports/.nuxt/imports'
    ],
    correct: [ 'C:/Users/my-username/Documents/a/.nuxt/imports' ]
  }
}

I understood that the generated absolute paths do not need to be real, but the output still looks quite weird (plus eslint-import-resolver-typescript continues not to work).

Just for comparison, this is 4.6.0 output

```ts { baseUrl: { broken: './C:/Users/my-username/Documents/a/b/eslint-plugin-import-with-nuxt-3.6-false-reports', correct: '..' }, matcher: { broken: [ 'C:/Users/my-username/Documents/a/b/eslint-plugin-import-with-nuxt-3.6-false-reports/.nuxt/C:/Users/my-username/Documents/a/b/eslint-plugin-import-with-nuxt-3.6-false-reports/C:/Users/my-username/Documents/a/b/eslint-plugin-import-with-nuxt-3.6-false-reports/.nuxt/imports' ], correct: [ 'C:/Users/my-username/Documents/a/.nuxt/imports' ] } } ```

privatenumber commented 1 year ago

:tada: This issue has been resolved in version 4.6.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

privatenumber commented 1 year ago

Give this a try ^

andreww2012 commented 1 year ago

Everything works now, thank you a lot for the quick fix 💯 🥇