Skip to content
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

tsconfig: Use "solution" tsconfig for more correct type checking #27

Merged
merged 4 commits into from
Feb 24, 2024

Conversation

philbates35
Copy link
Owner

@philbates35 philbates35 commented Feb 24, 2024

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:

@philbates35 philbates35 force-pushed the tsconfig-updates branch 2 times, most recently from ed6e058 to 3353f7e Compare February 24, 2024 10:59
As we're building a web app it makes sense to use jsdom environment
instead of the default node environment, so that we can do things
such as demonstrated in the test that was added (taken from
https://vitest.dev/config/#environment), which currently isn't possible.
Add a test proving that jsdom types are working as expected, the
node types will be used in a later commit.
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
@philbates35 philbates35 changed the title tsconfig: Introduce new tsconfig.app.json tsconfig: Use "solution" tsconfig for more correct type checking Feb 24, 2024
@philbates35 philbates35 marked this pull request as ready for review February 24, 2024 14:40
@philbates35 philbates35 merged commit 6f161b9 into main Feb 24, 2024
11 checks passed
@philbates35 philbates35 deleted the tsconfig-updates branch February 24, 2024 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant