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
feat(create-vite): use "solution" tsconfig so that vite.config.ts is type checked #15913
base: main
Are you sure you want to change the base?
Conversation
Run & review this pull request in StackBlitz Codeflow. |
75a6af5
to
ed63247
Compare
ec14e59
to
2be3660
Compare
The change in 2aa6462 is resulting in a vite.config.js and vite.config.d.ts being generated which we don't want. The work around, to be able to have type checking of both src files and vite.config.ts during build is to create a dedicated tsconfig.app.json and tsconfig.node.json. See: * vitejs/vite#15913 * https://github.com/vuejs/create-vue/blob/f75fd9813a624b8e44b012608335721901aba00b/template/tsconfig/base/tsconfig.json
The change in 2aa6462 is resulting in a vite.config.js and vite.config.d.ts being generated which we don't want. The work around, to be able to have type checking of both src files and vite.config.ts during build is to create a dedicated tsconfig.app.json and tsconfig.node.json. See: * vitejs/vite#15913 * https://github.com/vuejs/create-vue/blob/f75fd9813a624b8e44b012608335721901aba00b/template/tsconfig/base/tsconfig.json
2be3660
to
27988cb
Compare
The change in 2aa6462 is resulting in a vite.config.js and vite.config.d.ts being generated which we don't want. The work around, to be able to have type checking of both src files and vite.config.ts during build is to create a dedicated tsconfig.app.json and tsconfig.node.json. See: * vitejs/vite#15913 * https://github.com/vuejs/create-vue/blob/f75fd9813a624b8e44b012608335721901aba00b/template/tsconfig/base/tsconfig.json
The change in 2aa6462 is resulting in a vite.config.js and vite.config.d.ts being generated which we don't want. The work-around. To fix this, lets do the "solution" tsconfig approach as used by the create-vue project. This results in things like the following, all of which aren't enforced currently: * tsc allows "process.cwd()" in vite.config.ts and sum.test.ts but not in any src files such as app.ts * tsc allows document.createElement() in src files such as app.ts and in sum.test.ts (because jsdom exists there) but not in vite.config.ts which is just pure node environment. * Can't import test files into src files * Allows using new features such as "".replaceAll() in vite.config.ts which should be allowed because we're using node 20. See: * vitejs/vite#15913 * https://github.com/vuejs/create-vue/blob/12bf2889b9ca981bcfed894a7c24fa9db5e7bad5/template/tsconfig/base/tsconfig.app.json * https://github.com/vuejs/create-vue/blob/12bf2889b9ca981bcfed894a7c24fa9db5e7bad5/template/tsconfig/base/tsconfig.node.json * https://github.com/vuejs/create-vue/blob/12bf2889b9ca981bcfed894a7c24fa9db5e7bad5/template/tsconfig/vitest/tsconfig.vitest.json
The change in 2aa6462 is resulting in a vite.config.js and vite.config.d.ts being generated which we don't want. To fix this, lets do the "solution" tsconfig approach as used by the create-vue project. This results in things like the following, all of which aren't enforced currently: * tsc allows "process.cwd()" in vite.config.ts and sum.test.ts but not in any src files such as app.ts * tsc allows document.createElement() in src files such as app.ts and in sum.test.ts (because jsdom exists there) but not in vite.config.ts which is just pure node environment. * Can't import test files into src files * Allows using new features such as "".replaceAll() in vite.config.ts which should be allowed because we're using node 20. See: * vitejs/vite#15913 (comment) * https://github.com/vuejs/create-vue/blob/12bf2889b9ca981bcfed894a7c24fa9db5e7bad5/template/tsconfig/base/tsconfig.app.json * https://github.com/vuejs/create-vue/blob/12bf2889b9ca981bcfed894a7c24fa9db5e7bad5/template/tsconfig/base/tsconfig.node.json * https://github.com/vuejs/create-vue/blob/12bf2889b9ca981bcfed894a7c24fa9db5e7bad5/template/tsconfig/vitest/tsconfig.vitest.json
27988cb
to
7576086
Compare
One thing we might need to consider is that this might make more users to encounter #15870. |
Currently when running "pnpm run build" on a fresh Vite project using any of these templates, tsc doesn't fail when there are type errors in vite.config.ts, such as something obvious like: const foo: string = 123; Passing -b makes it run for all project references, meaning that vite.config.ts starts to be type checked too during build. In order to not generate a vite.config.js and vite.config.d.ts we need to make the main tsconfig.json have nothing but references to tsconfig.node.json and a new tsconfig.app.json, as is done in the create-vue project, see: https://github.com/vuejs/create-vue/blob/f75fd9813a624b8e44b012608335721901aba00b/template/tsconfig/base/tsconfig.json
7576086
to
07358e4
Compare
Let's wait for a tsconfck update before merging this one. |
@patak-dev sounds good - just out of interest, which update are we waiting for and what's the reason? |
I think we can now move forward with this one. @dominikg I'll let it open a few more days in case you'd like to check it out too. |
why is this turning on composite and storing a file in node_modules/.tmp ? This feels opinionated cc @dummdidumm regarding svelte-check, iirc it does check all files, not just .svelte. It should not be needed to chain tsc after it in svelte templates |
in general I'd like to challenge the use of solution style tsconfig in create-vite. My understanding is that these are minimalist setups. Is there a way to make this work that does not require additional files? |
It is to run type check on
AFAIK when using project references, TypeScript outputs tsbuildinfo files and defaults to the same directory with
We can merge |
@dominikg chaining looks fine, assuming the vite config does not import any Svelte code. It's fine because the tsconfigs have separate includes, so |
you can use references without turning on composite (composite makes incremental true by default https://www.typescriptlang.org/tsconfig#incremental , which generates these files) calling it tsconfig.node.json is also slightly controversial for users that use other environments such as deno or bun. If you have to add a second config file, i'd use a more neutral name and not use composite. Maybe not even use references but instead add a separate script that just typechecks vite.config.ts with a tsconfig.viteconfig.json. |
to expand on the separate command part, |
enabling composite/incremental can also slow down CI build times and it's not clear if the |
I didn't know about that. Then, I think we should remove
tsconfig.node.json was there for more than two years. So I want to split that discussion in a different PR. Maybe removing |
Description
Currently when running
pnpm run build
on a fresh Vite project using any of these templates,tsc
doesn't fail when there are type errors invite.config.ts
, such as something obvious like:Passing
-b
makes it run for all project references, meaning thatvite.config.ts
starts to be type checked too during build.In order to not generate a
vite.config.js
andvite.config.d.ts
we need to make the maintsconfig.json
have nothing but references totsconfig.node.json
and a newtsconfig.app.json
, as is done in the create-vue project, see:https://github.com/vuejs/create-vue/blob/f75fd9813a624b8e44b012608335721901aba00b/template/tsconfig/base/tsconfig.json
Note: I am unfamiliar with Svelte which is the only template that I wasn't sure how to apply this to, its script looks like this:
I checked the docs of the
svelte-check
project but I couldn't see anyway to pass options through totsc
, if you know of a way to do that then let me know and I'll update the PR.close #12590
close #11396
What is the purpose of this pull request?