feat: support vite8 resolve.tsconfigPaths#494
Conversation
commit: |
|
/bigbonk review |
There was a problem hiding this comment.
Clean, well-scoped change. The approach is correct:
- Plugin gating:
vite-tsconfig-pathsis excluded from the plugin array on Vite 8+, and the nativeresolve.tsconfigPathsis enabled instead. - User override respected: Checking
=== undefinedcorrectly distinguishes "user didn't set it" from "user explicitly set it tofalse". - Cached version lookup: Hoisting
getViteMajorVersion()into a singleconst viteMajorVersionat the top ofvinext()is a good cleanup — it was being called redundantly before (line 1263 in the old code). - Test coverage: All three important cases are covered — Vite 7 keeps the plugin, Vite 8 enables native paths, and explicit
tsconfigPaths: falseis not overridden. - Peer dep range:
^8.0.0-beta.0is appropriate for early beta support.
Two minor observations (non-blocking):
|
|
||
| expect(findNamedPlugin(plugins, "vite-tsconfig-paths")).toBeDefined(); | ||
|
|
||
| fs.rmSync(root, { recursive: true, force: true }); |
There was a problem hiding this comment.
Nit: The temp directory cleanup (fs.rmSync) is at the end of each test body, but if an assertion fails mid-test, it won't run. Since afterEach already restores process.cwd(), consider also moving rmSync into afterEach (or using try/finally within each test) so temp dirs don't leak on failure.
Non-blocking — the leaked dirs are in os.tmpdir() so they'll be cleaned up by the OS eventually.
| } | ||
| } | ||
|
|
||
| type UserResolveConfigWithTsconfigPaths = NonNullable<UserConfig["resolve"]> & { |
There was a problem hiding this comment.
Nit: Once Vite 8 ships a stable release with tsconfigPaths in its public UserConfig["resolve"] type, this custom type can be removed and you can use Vite's type directly. Worth a TODO comment so it doesn't get forgotten.
Non-blocking.
|
Review submitted. Here's a summary: Approved — this is a clean, well-scoped change that correctly adds Vite 8 What the PR does:
Two non-blocking nits:
|
|
Nice one! |
https://main.vite.dev/guide/features#paths
Vite will also add a warning for this.
vite-tsconfig-pathsplugin is detected vitejs/vite#21781