VS Code builtin tsserver/TypeScript support picks tsconfig.vitest.json instead of tsconfig.app.json for ts files

segevfiner commented 2 years ago

Follow up to talking to myself in https://github.com/vuejs/create-vue/pull/151 😛

Create a new create-vue project with everything but tsx selected. In this configuration, VS Code's builtin tsserver/TypeScript support seems to pick tsconfig.vitest.json, instead of tsconfig.app.json. It seems the logic to pick them is different between Volar and tsserver, as changing their order in tsconfig.json causes Volar to be the one to select tsconfig.vitest.json instead of tsconfig.app.json.

Adding the following to tsconfig.vitest.json:

  "references": [
    { "path": "./tsconfig.app.json" }
  ]

Seems to make VS Code select tsconfig.app.json for ts files properly, but this is some pure guess work and I'm not sure where or how the logic to select an appropriate tsconfig works.

In addition:

We are using vue-tsc --noEmit -p tsconfig.vitest.json --composite false to type check, but in case the user wants to add additional projects to the project references config, this won't type check them. Apparently --build, composite, and project references don't support noEmit, so we need to use emitDeclarationOnly, declarationDir instead to make it emit types to some folder solely for the purpose of type checking. On the plus side, in this mode it reuses them when it can for faster type checking (It also enables incremental).
tsconfig.vitest.json has "lib": [], but this should probably be "lib": ["ES2016"] to match tsconfig.app.json just without DOM.

So basically the following changes to the generated project seems to fix those things for me:

diff --git a/.gitignore b/.gitignore
index 38adffa..b2e683b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -13,6 +13,8 @@ dist
 dist-ssr
 coverage
 *.local
+types
+*.tsbuildinfo

 /cypress/videos/
 /cypress/screenshots/
diff --git a/package.json b/package.json
index 4c8c275..1a7cbc7 100644
--- a/package.json
+++ b/package.json
@@ -9,7 +9,7 @@
     "test:e2e": "start-server-and-test preview http://localhost:4173/ 'cypress open --e2e'",
     "test:e2e:ci": "start-server-and-test preview http://localhost:4173/ 'cypress run --e2e'",
     "build-only": "vite build",
-    "type-check": "vue-tsc --noEmit -p tsconfig.vitest.json --composite false",
+    "type-check": "vue-tsc --build",
     "lint": "eslint . --ext .vue,.js,.jsx,.cjs,.mjs,.ts,.tsx,.cts,.mts --fix --ignore-path .gitignore"
   },
   "dependencies": {
diff --git a/tsconfig.app.json b/tsconfig.app.json
index cdbea1d..0e1bcdc 100644
--- a/tsconfig.app.json
+++ b/tsconfig.app.json
@@ -3,6 +3,8 @@
   "include": ["env.d.ts", "src/**/*", "src/**/*.vue"],
   "exclude": ["src/**/__tests__/*"],
   "compilerOptions": {
+    "emitDeclarationOnly": true,
+    "declarationDir": "types",
     "composite": true,
     "baseUrl": ".",
     "paths": {
diff --git a/tsconfig.config.json b/tsconfig.config.json
index c2d3a30..8577b3c 100644
--- a/tsconfig.config.json
+++ b/tsconfig.config.json
@@ -2,6 +2,8 @@
   "extends": "@vue/tsconfig/tsconfig.node.json",
   "include": ["vite.config.*", "vitest.config.*", "cypress.config.*"],
   "compilerOptions": {
+    "emitDeclarationOnly": true,
+    "declarationDir": "types",
     "composite": true,
     "types": ["node"]
   }
diff --git a/tsconfig.vitest.json b/tsconfig.vitest.json
index d080d61..d0d8b87 100644
--- a/tsconfig.vitest.json
+++ b/tsconfig.vitest.json
@@ -2,8 +2,13 @@
   "extends": "./tsconfig.app.json",
   "exclude": [],
   "compilerOptions": {
+    "emitDeclarationOnly": true,
+    "declarationDir": "types",
     "composite": true,
-    "lib": [],
+    "lib": ["ES2016"],
     "types": ["node", "jsdom"]
-  }
+  },
+  "references": [
+    { "path": "./tsconfig.app.json" }
+  ]
 }

@sodatea

haoqunjiang commented 2 years ago

For the tsconfig selection, there's an issue in TypeScript repo tracking that: https://github.com/microsoft/TypeScript/issues/48058

As of now, the vue-tsc behavior is deterministic, and opposite to the tsc behavior. We chose to follow its behavior as it's a Vue project after all.

I'm still not sure what's the best way to type-check the source files when vitest is present. Will think more about that later.

2. tsconfig.vitest.json has "lib": [], but this should probably be "lib": ["ES2016"] to match tsconfig.app.json just without DOM.

This looks like a bug to me, needs to be fixed.

segevfiner commented 2 years ago

@sodatea Do try my proposed changes, especially in some existing project, let me know if it seems to work better or not. (Not sure if there is a conventional name for the directory used for declarationDir for this configuration)

segevfiner commented 2 years ago

Let me know if you want me to send a PR with any of those changes.

haoqunjiang commented 2 years ago

I don't have the time to try it out now. @xiaoxiangmoe could you help take a look?

segevfiner commented 2 years ago

Argh. Just hit this again with it picking up tsconfig.vitest.json giving me an error about stuff that comes from DOM.Iterable not being defined.

xiaoxiangmoe commented 2 years ago

@segevfiner Can you provide me a minimal reproduce demo to show the difference?

segevfiner commented 2 years ago

Try: https://github.com/segevfiner/create-vue-159. It errors in main.ts and when running type-check but is OK in VS Code when opening App.vue as there is uses tsconfig.app.json. While during type check and viewing ts files it is using tsconfig.vitest.json.

JoostKersjes commented 2 years ago

I added your proposed changes to a semi-large project which includes Vitest, Cypress & Storybook. My painpoints:

Any tsconfig.*.json file that extends tsconfig.app.json will also cause the script to parse the app files. At best that means type-check takes longer to run than needed, at worst it means errors pop up because the compilerOptions are different.
This parsing the app files multiple times also causes any errors to be repeated in the output multiple times, which is hard to read.
I personally dislike that it any generates files at all, even if they are ignored. The files could be a source of confusion when working in teams.
Running type-check twice results in a false positive. There are multiple errors on the first time it runs. Once the files are generated though, any subsequent run of type-check will pass without reporting the errors.

Because the project is using Vitest, Cypress & Storybook it would be a blessing to get a good solution for this. I might look into this some more, but I'm probably unlikely to succeed.

segevfiner commented 2 years ago

Yeah. It's likely not perfect. I wish TS itself would have better handling for this... But the current situation is actively giving me false errors or allowing to use node where I should not, so we should find at least a solution to that...

haoqunjiang commented 2 years ago

tsconfig.vitest.json has "lib": [], but this should probably be "lib": ["ES2016"] to match tsconfig.app.json just without DOM.

I just checked that @types/node has already referenced the corresponding ES lib, e.g. https://github.com/DefinitelyTyped/DefinitelyTyped/blob/8d5e1a106b1b18250623e09a2a8469dc7ee52bca/types/node/index.d.ts#L73 So "lib": ["ES2016"] would be redundant.

Try: https://github.com/segevfiner/create-vue-159. It errors in main.ts and when running type-check but is OK in VS Code when opening App.vue as there is uses tsconfig.app.json. While during type check and viewing ts files it is using tsconfig.vitest.json.

It's a bug in @types/jsdom. I've submitted a PR: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/62769

Nevertheless, the type-checking experience could be better. I'll look further into it in the coming days.

segevfiner commented 1 year ago

TypeScript 5.0 now supports noEmit in with project references/composite projects! Allowing us to switch to just vue-tsc --build by adding noEmit to the tsconfig.*.json files!

But it also deprecates importsNotUsedAsValues & preserveValueImports in favor of verbatimModuleSyntax which needs to be modified in @vue/tsconfig, and support added to vue-tsc, as it seems to currently break with this option https://github.com/vuejs/language-tools/issues/2343#issuecomment-1478541728 and there seems to be other breakages as well as adding "ignoreDeprecations": "5.0" also gives other errors.

bmulholland commented 5 months ago

I've tried to wrap my head around these parts for the docs on this that I've written, but I've lost the current status. The project is now using --build so I'm not sure what applies anymore. If anyone here has a good understanding of the overlap of these two files, could you please help me finish https://github.com/vuejs/create-vue/issues/265#issuecomment-2165816042 ?

jemdiggity commented 2 months ago

In reference to tsconfig.vitest.json, it seems that including "lib": [], overrides the lib entry from tsconfig.app.json. For example if you include "lib": ["esnext", "dom"], in tsconfig.app.json, it doesn't have any effect, unless you remove "lib": [], from tsconfig.vitest.json.

vuejs / create-vue

VS Code builtin tsserver/TypeScript support picks tsconfig.vitest.json instead of tsconfig.app.json for ts files #159