qmhc / vite-plugin-dts

A Vite plugin for generating `.d.ts` files.
MIT License
1.32k stars 91 forks source link

Invalid imports in generated d.ts for project with tsconfig path alias defined for any module #330

Open k-i-m opened 6 months ago

k-i-m commented 6 months ago

Describe the bug

I have a project, where I have path aliases defined in tsconfig.json to simplify usage of any module within src folder:

    "paths": {
      "*": ["src/*"],
    },

This setting allows to simplify the imports, so that instead of: import { Sample } from 'src/components/Sample'; (or even import { Sample } from '@/components/Sample';) we can use: import { Sample } from 'components/Sample';

When I build the app, the app itself works fine (thanks to the vite-tsconfig-paths), but all the d.ts files for .ts files that are inside a subfolder of src, are generated with invalid import paths.

The original ts file: image The output d.ts file: image

It was working fine in vite-plugin-dts@3.6.4. Any version since then (so starting from v3.7.0) has the issue. Possibly there is an issue in this commit that was introduced in 3.7.0: https://github.com/qmhc/vite-plugin-dts/commit/e8827cb6c8be1406e4e3a9a24639b7b54ca20d53

Reproduction

https://stackblitz.com/edit/vitejs-vite-sg53ed?file=dist%2Ftypes%2Fcomponents%2FSample%2Findex.d.ts

Steps to reproduce

package.json vite related libraries:

    "vite": "^5.2.0",
    "vite-plugin-dts": "^3.9.1",
    "vite-tsconfig-paths": "^4.3.2"

tsconfig.json add the following "paths":

    "paths": {
      "*": ["src/*"],
    }

vite.config.ts

import * as path from 'path';
import { defineConfig } from 'vite';
import react from '@vitejs/plugin-react';
import dts from 'vite-plugin-dts';
import tsconfigPaths from 'vite-tsconfig-paths';

import * as packageJSON from './package.json';

// https://vitejs.dev/config/
export default defineConfig({
  plugins: [
    react(),
    dts({
      entryRoot: path.resolve(__dirname, 'src'),
      outDir: path.resolve(__dirname, 'dist', 'types'),
    }),
    tsconfigPaths(),
  ],
  build: {
    rollupOptions: {
      external: Object.keys(packageJSON.dependencies),
    },
  },
});

System Info

System:
    OS: Windows 11 10.0.22621
    CPU: (20) x64 13th Gen Intel(R) Core(TM) i7-13800H
    Memory: 13.18 GB / 31.66 GB
  Binaries:
    Node: 18.20.2 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.22 - C:\Program Files\nodejs\yarn.CMD
    npm: 10.5.0 - C:\Program Files\nodejs\npm.CMD
    pnpm: 9.1.1 - C:\Program Files\nodejs\pnpm.CMD
  Browsers:
    Edge: Chromium (123.0.2420.97)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    @vitejs/plugin-react: ^4.2.1 => 4.2.1
    vite: ^5.2.0 => 5.2.11
    vite-plugin-dts: ^3.9.1 => 3.9.1

Validations

williamp-osttra commented 3 months ago

Just confirmed this problem still exists with version 4.0.3. It works in version <= 3.7.0.

giuvincenzi commented 3 months ago

The problem is still here. Are there any updates/workarounds?

kristof-mattei commented 3 months ago

Temp fix:

tsconfig.json:

    // ...

    "paths": {
      "@/*": [
        "./src/*",
      ]
    },                                                   /* Specify a set of entries that re-map imports to additional lookup locations. */

    // ...

vite.config.mts:

// ...

export default defineConfig(() => {
    const config: UserConfig = {
        build: {
            // ...
        },

        resolve: { alias: { "@/": path.resolve("src/") } },

        plugins: [
            viteTsConfigPaths(),
            dts({
                insertTypesEntry: true,
                entryRoot: "./src",
                exclude: ["test.setup.ts", "vite.config.ts", "src/tests/**"],
            }),
        ],
        // ...

Note the resolve: { alias: { "@/": path.resolve("src/") } },. Since it takes a string: string we can't import it from tsconfig.json, but at the moment, that is ok, I only have 1 line for my paths.

And the generated .d.ts don't contain the @/, but the ./, as expected.

lachieh commented 2 months ago

See note on #376

Hey 👋 v4.1.0 is released which should fix this. I tried to add some test cases to cover the cases mentioned here.

If any of you could update and test if you're still having the issue? If not, can you post your full tsconfig.json and vite.config.json? Even better would be a codesandbox reproduction.

@k-i-m, @giuvincenzi, @kristof-mattei Could you all try this update?

kristof-mattei commented 2 months ago

@lachieh

To start, thanks for releasing a bugfix. Then, sorry for the late reply.

I've tested with the old 4.0.3 and my resolve: { alias: { "@/": path.resolve("src/") } }, fix vs the current 4.1.1 without said fix.

Unfortunately it doesn't work for me.

❯ diff dist-4.0.3/core.d.ts dist-4.1.1/core.d.ts
1,6c1,6
< export * as checks from './checks';
< export * from './commands/base-program';
< export * from './commands/cargo';
< export * from './commands/cross';
< export * from './commands/rustup';
< export * as input from './input';
---
> export * as checks from '@/checks';
> export * from '@/commands/base-program';
> export * from '@/commands/cargo';
> export * from '@/commands/cross';
> export * from '@/commands/rustup';
> export * as input from '@/input';

Maybe I'm doing something wrong?

lachieh commented 2 months ago

Thanks @kristof-mattei. No worries, I find most gh issues happen quite async.

Could you post your tsconfig?

kristof-mattei commented 2 months ago

Thanks @kristof-mattei. No worries, I find most gh issues happen quite async.

Could you post your tsconfig?

https://github.com/actions-rs-plus/core/blob/main/tsconfig.json

In that project you can do npm run build and then look in dist/core.d.ts

and then in vite vite.config.ts you can comment out the resolve part and then build again. But then dist/core.d.ts isn't correct anymore.

lachieh commented 2 months ago

Ahh ok, looks like it goes away with the baseUrl being set. I thought that was a requirement, but looks like it was removed in TS4.1. I'll see if I can add some more tests to get coverage and fix the underlying issue.

kristof-mattei commented 2 months ago

Ahh ok, looks like it goes away with the baseUrl being set. I thought that was a requirement, but looks like it was removed in TS4.1. I'll see if I can add some more tests to get coverage and fix the underlying issue.

Let me know if you want me to retest stuff / help out.

k-i-m commented 2 months ago

The problem still exists in v 4.2.3

I have exact same output as was described in the issue

Please note that I have '*" in my path alias, not "@"

"paths": {
      "*": ["src/*"],
    }
lachieh commented 2 months ago

@k-i-m do you have a baseUrl set in your tsconfig? It should work without since TS4.1 doesn't need it anymore but I don't think this lib has accounted for that setting being missing.

I did add a test case for the "*" alias: https://github.com/qmhc/vite-plugin-dts/blob/a7e1c0cb80cf25056e02e04414af9412eeeea750/tests/transform.spec.ts#L191

lachieh commented 2 months ago

I added a PR to hopefully fix the issue

lachieh commented 1 month ago

v4.2.4 is out! Thanks, @qmhc!

@k-i-m can you retest your issue with this version?

@kristof-mattei the tsconfig file should no longer require the baseUrl now, either!

k-i-m commented 1 month ago

The main issue is solved but I still get some weird imports for my 3rd party depencendies, like for React: import { Component, ErrorInfo, ReactNode } from '../../react;

lachieh commented 1 month ago

Ah yes, that's a great point. It seems that there isn't really a way to support top level * alias without some actual dependency checking.

@qmhc what do you think?

qmhc commented 1 month ago

OH, an intractable disease. Let me think...

efriandika commented 2 weeks ago

Just confirmed this problem still exists with version 4.3.0

lachieh commented 2 weeks ago

@qmhc would you accept a solution that adds a check for this specific condition (top-level * alias)? I'm thinking we would need to use resolve to check if the import exists after it has been modified and then assume it is a dependency if it doesn't exist within the project?

Example:

  1. process like normal ('react' becomes '../../react')
  2. check if alias is '*' and attempt to resolve using import {resolve} from 'node:require'. If it resolves, leave the import as is and continue
  3. if it does not resolve, assume it is (or will be) installed as a dependency

Does that sound like it would work?

qmhc commented 2 weeks ago

@lachieh Yes, my thoughts are very similar to yours. Would you like to try this solution?

lachieh commented 2 weeks ago

Added a PR (#396) which probably needs some closer attention. I wanted to try to use the TS program to resolve the paths and see if they had a better way, but in this solution I just used the existsSync.