feat(config): allow imports from local workspace tsconfig paths (#5370) #17286
Conversation
|
|
|
I'm not entirely sure why the windows-latest build is failing; doesn't seem to be related to my changes. Edit to add: It's running out of memory in the unit tests. I've run the tests on a Windows 10 machine and Node 20, and all tests pass. |
garydex
force-pushed
the
vite-config-resolve-tsconfig-paths
branch
2 times, most recently
from
41431a7 to
8c947cb
Compare
|
That's better - all tests passing now 🥳 It's worth pointing out that I have created an Nx workspace with my patched version of Vite, to demonstrate how a shared config file can be imported and extended - https://github.com/garydex/nx-vite-tsconfig-paths |
|
Vite doesn't respect the tsconfig paths today, hence I don't think we should fix the problem this way. Similarly in #6828 where we had discussed and decided that we don't want to have builtin support for it. Tsconfig paths describes the (Vite) runtime and doesn't define it. Sorry for not reviewing this earlier before. |
|
I appreciate the response. A few points:
The issue you mentioned is about resolving tsconfig paths across the entirety of Vite. This isn't trying to do that, or replicate/replace vite-tsconfig-paths or similar plugins. It's specific to config, and only executes if there's a tsconfig file that has paths in the codebase. The other advantage of using the tsconfig paths in this context is that it I actually think we may need both this and #17323 - while that PR may fix imports with npm/yarn workspaces, it still doesn't work in a repo that uses tsconfig paths, such as an Nx workspace. |
The config loading is part of the "experience" of using Vite, so that issue is relevant here too. We can't have tsconfig paths sometimes work here but not there when they work in a Vite project. |
|
Fair point, though I do think it's an equally jarring experience for users who can easily get tsconfig paths to work everywhere in their code except the config file. I understand it's via plugins and out of the scope of Vite itself - but ideally one would be able to at least opt-in for something like this. |
| resolvedPath: string, | ||
| ): Promise<string[]> { | ||
| const { tsconfig } = await parse(resolvedPath) | ||
| const paths = tsconfig?.compilerOptions?.paths || {} |
There was a problem hiding this comment.
How about using the ?? operator instead of ||?
I think ?? would be more appropriate in this context.
| if (!tsConfigPaths.length) { | ||
| return false | ||
| } |
There was a problem hiding this comment.
Since Array.prototype.some always returns false when called on an empty array, the tsConfigPaths.length check is unnecessary.
|
Related #16718 Closing this as it's not something we want to support in the config. |
Description
Fixes #5370 nrwl/nx#17019 remix-run/remix#9171
Uses existing
tsconfckdependency to loads the nearest tsconfig file if it exists, grabs anypathsfromcompilerOptionsand then makes sure these are not caught by theexternalise-depsplugin when resolving imports in the config file.This functionality is isolated to
loadConfigFromFileso does not need to replicate functionality fromtsconfig-pathsor similar, nor does it duplicate any existing plugins likevite-tsconfig-pathsor@esbuild-plugins/tsconfig-paths.