Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
3 tasks done
k-i-m opened this issue May 16, 2024 · 20 comments · Fixed by #376 or #396
Closed
3 tasks done
Labels
bug Something isn't working

Comments

@k-i-m
Copy link

k-i-m commented May 16, 2024

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:
e8827cb

Reproduction

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

Steps to reproduce

  • create sample app (react based for example), with vite + vite-plugin-dts + vite-tsconfig-paths
  • create any subfolder (like "components") inside src, and there create any other folder containing some sample component:
src
- components
-- Sample
--- Sample.tsx
--- index.ts

where index.ts is only reexporting the component

export * from './Sample';
  • import the component in App.tsx: import { Sample } from 'components/Sample';

  • set the project's configuration:

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),
    },
  },
});
  • run vite build command
  • in dist/types folder you will find d.ts files with invalid import paths

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

@qmhc qmhc added the bug Something isn't working label May 16, 2024
@williamp-osttra
Copy link

williamp-osttra commented Aug 20, 2024

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

@giuvincenzi
Copy link

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

@kristof-mattei
Copy link

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 added a commit to lachieh/vite-plugin-dts that referenced this issue Aug 28, 2024
qmhc pushed a commit that referenced this issue Sep 1, 2024
* test(transform): add failing integration test for transformCode

* fix(utils): correct issue with failing integration test

* test(utils): add tests for parseTsAliases

* test(transform): refactor process aliases test with helper and descriptions, add more cases

* test: adds test cases from issue and fixes #330

* test(utils): handle windows path
@qmhc qmhc closed this as completed in #376 Sep 1, 2024
@lachieh
Copy link
Contributor

lachieh commented Sep 3, 2024

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
Copy link

@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
Copy link
Contributor

lachieh commented Sep 7, 2024

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

Could you post your tsconfig?

@kristof-mattei
Copy link

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
Copy link
Contributor

lachieh commented Sep 9, 2024

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
Copy link

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
Copy link
Author

k-i-m commented Sep 30, 2024

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
Copy link
Contributor

lachieh commented Sep 30, 2024

@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:

'*': ['src/utils/*']

@lachieh
Copy link
Contributor

lachieh commented Oct 1, 2024

I added a PR to hopefully fix the issue

@qmhc qmhc reopened this Oct 8, 2024
lachieh added a commit to lachieh/vite-plugin-dts that referenced this issue Oct 8, 2024
lachieh added a commit to lachieh/vite-plugin-dts that referenced this issue Oct 8, 2024
lachieh added a commit to lachieh/vite-plugin-dts that referenced this issue Oct 8, 2024
lachieh added a commit to lachieh/vite-plugin-dts that referenced this issue Oct 8, 2024
lachieh added a commit to lachieh/vite-plugin-dts that referenced this issue Oct 8, 2024
lachieh added a commit to lachieh/vite-plugin-dts that referenced this issue Oct 8, 2024
lachieh added a commit to lachieh/vite-plugin-dts that referenced this issue Oct 8, 2024
@qmhc qmhc closed this as completed in dc3cbfe Oct 11, 2024
@lachieh
Copy link
Contributor

lachieh commented Oct 11, 2024

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
Copy link
Author

k-i-m commented Oct 14, 2024

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
Copy link
Contributor

lachieh commented Oct 14, 2024

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 qmhc reopened this Oct 14, 2024
@qmhc
Copy link
Owner

qmhc commented Oct 14, 2024

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

@efriandika
Copy link

Just confirmed this problem still exists with version 4.3.0

@lachieh
Copy link
Contributor

lachieh commented Nov 12, 2024

@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
Copy link
Owner

qmhc commented Nov 13, 2024

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

@lachieh
Copy link
Contributor

lachieh commented Nov 16, 2024

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.

lachieh added a commit to lachieh/vite-plugin-dts that referenced this issue Nov 16, 2024
lachieh added a commit to lachieh/vite-plugin-dts that referenced this issue Nov 16, 2024
lachieh added a commit to lachieh/vite-plugin-dts that referenced this issue Nov 16, 2024
lachieh added a commit to lachieh/vite-plugin-dts that referenced this issue Dec 12, 2024
lachieh added a commit to lachieh/vite-plugin-dts that referenced this issue Dec 12, 2024
@qmhc qmhc closed this as completed in #396 Dec 17, 2024
@qmhc qmhc closed this as completed in bb4650d Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
7 participants