aleclarson / vite-tsconfig-paths

Support for TypeScript's path mapping in Vite
MIT License
1.24k stars 43 forks source link

Suggestion: Less magic project lookup to increase the developer experience #50

Closed skovhus closed 1 year ago

skovhus commented 2 years ago

First, thanks for a great plugin. 👏

Here comes a suggestion. :)

Context

Background: When I recently moved my index.html file to a subfolder (i.e. changing the root folder configuration), I wasted some time with a failing build due to Vite being unable to resolve path alias... This was due to the rather magical lookup behaviour provided by vite-tsconfig-paths.

As documented in the README, the plugin currently automatically finds all tsconfig files in the same folder as the vite root. It means that it usually works for simple setup, but can cause confusing and rather error-prone scenarios as shown above. Another surprise was that it located multiple tsconfig files (some that we use for other parts of the codebase that isn't relevant for Vite). And the plugin silently failed to resolve.

Pain points:

Workaround

Being explicit about the tsconfig projects seems like the safest way to use the plugin:

tsconfigPaths({projects: [path.resolve(__dirname, 'tsconfig.json')]})

Suggestion

This would be breaking change, but I believe a better and safer API would be to always let the user specify the tsconfig projects and never automatically resolve them. This basically means getting rid of root and extensions, and a user would need to specify the projects when using the plugin (as mentioned in the workaround).

And if the file resolution fails then the plugin should crash.

If we want a no config behaviour (i.e. tsconfigPaths()) then I suggest defaulting to a tsconfig.json file – but fail if the file cannot be resolved.

aleclarson commented 2 years ago

Another option would be to provide a hint when a resolution fails. The hint could say which tsconfig.json file was used and give a link to a wiki page that goes into Troubleshooting that the community can build up together.

edit: I've created the wiki page: https://github.com/aleclarson/vite-tsconfig-paths/wiki/Troubleshooting
Anyone can edit it.

jasperdunn commented 2 years ago

Thanks so much @skovhus, saved me some time! I came across this right away and it solved my issue.

skovhus commented 2 years ago

Another option would be to provide a hint when a resolution fails. The hint could say which tsconfig.json file was used and give a link to a wiki page that goes into Troubleshooting that the community can build up together.

Personally I would also rather fail than log an error (which people might not see). But yes a hint would be better than silently failing. Although we might need the wiki entry if we changed the message and behavior of the plug-in.

Note that adding a hint only solves the second pain point, not the part about multiple tsconfig being resolved. Wondering what the use case is here?

skovhus commented 2 years ago

Thanks so much @skovhus, saved me some time! I came across this right away and it solved my issue.

Happy to hear. Curious to know what you would prefer regarding the suggested solution.

jasperdunn commented 2 years ago

Yeah I agree with failing with a descriptive message. Maybe even adding an CLI option to add that config in the create vite with ts options?

AndrewSmithDev commented 2 years ago

This caused a lot of headache for me since I was using a file named tsconfig.base.json instead of tsconfig.json. Eventually I got it working by specifying the path in projects.

import { defineConfig } from 'vite';
import reactRefresh from '@vitejs/plugin-react-refresh';
import tsconfigPaths from 'vite-tsconfig-paths';
import { resolve } from 'path';

// https://vitejs.dev/config/
export default defineConfig({
  plugins: [
    tsconfigPaths({
      projects: [resolve(__dirname, '../../tsconfig.base.json')],
    }),
    reactRefresh(),
  ],
});
skovhus commented 2 years ago

@AndrewSmithDev Crashing in case tsconfig path resolution fails would have helped a lot, I assume? Any thoughts on the suggested solution?

aleclarson commented 2 years ago

@AndrewSmithDev Hi, do you have no tsconfig.json files anywhere that extend the tsconfig.base.json file? As I understand it, TypeScript won't use tsconfig.base.json unless it's extended by a tsconfig.json file somewhere in the project, so this plugin behaves as expected. I would suggest opening another issue with a minimal reproduction, in case there's a bug here. Your issue might be related to https://github.com/aleclarson/vite-tsconfig-paths/issues/12#issuecomment-1075342431.

skovhus commented 2 years ago

As I understand it, TypeScript won't use tsconfig.base.json unless it's extended by a tsconfig.json file somewhere in the project, so this plugin behaves as expected.

That depends. Using the TS_NODE_PROJECT environment variable you can specify a custom tsconfig file. Besides, one might have the file in sub folder and use the -p compiler option. And some frameworks like nx the root tsconfig is by default tsconfig.base.json.

Are you against being defensive and simply fail if a tsconfig file cannot be located as suggested here?

aleclarson commented 2 years ago

@skovhus This plugin (intentionally) does not support ts-node environment variables or tsc command line options. All configuration is defined by the Vite config and tsconfig.json files.

Are you against being defensive and simply fail if a tsconfig file cannot be located as suggested here?

This plugin cannot know the intention of any particular import, so we can't reliably know when a tsconfig.json should have been found or whether its paths should have been used.

I don't like the idea of a plugin crashing the dev server or build. I think the better choice would be to augment the resolution error with a list of tsconfig.json files that were tried. That should help remind users that vite-tsconfig-paths might be misconfigured. And that augmented resolution error will appear in the browser via Vite's error overlay feature.

skovhus commented 2 years ago

This plugin (intentionally) does not support ts-node environment variables or tsc command line options

I don’t suggest that it should, but I’m highlighting that the idea that a project always has a tsconfig.json file in the root isn’t always the case.

This plugin cannot know the intention of any particular import, so we can't reliably know when a tsconfig.json should have been found or whether its paths should have been used.

If this plugin is used, one can assume that a tsconfig file should be somewhere in the project. I’m arguing that the plugin cannot know which file to pick, so why not remove the clever implicit behaviour (and the root And extensions config options)? And if the implicit behaviour is kept then at least fail if no tsconfig file is found.

But it seems a design decision here is to be clever/implicit and not explicit (e.g. by forcing the user to provide a valid projects option).

And we lean on more logging instead of being defensive (e.g. crash).

… More logging would be better than nothing, and it would have saved me and others a lot of debug time.

Let me know if I understand your point of view correctly. I’m up for doing a PR.

aleclarson commented 2 years ago

I disagree with the idea that the current behavior is "clever". It's exactly how TypeScript behaves in the IDE, in the sense that we both crawl the project in search of tsconfig.json files.

And if the implicit behaviour is kept then at least fail if no tsconfig file is found.

I'm open to this (PR welcome). Include the link to the Troubleshooting wiki in the error please.