You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Feb 15, 2023. It is now read-only.
I'm opening this issue for discussion, as I see there are different ways to implement this. I'd be happy to submit a PR if there's a consensus.
The problem
Currently, setupTypeScript.js adds a validate npm script to run svelte-check, and this catches type errors in .svelte files. However, it does not type check .ts files, and it wasn't until I noticed I had type errors on my main branch (despite CI running build and validate commands) that I realized this was the case. Admittedly, the docs for svelte-check make this pretty obvious in hindsight, but I had assumed that the default template for a TypeScript Svelte project would implement full type checking.
There are a couple of simple options which could be added to the setupTypeScript.js script which would fix this.
Option 1: Configure TypeScript plugin to fail on errors
The generated Rollup configuration does show warnings for type errors in .ts files, which hints at one solution to this: to add a noEmitOnError: true option to the Rollup TypeScript plugin config. However, with this approach, one ends up with type checking for different file types occurring during different phases of the build or CI: validate type checks .svelte files, and build type checks .ts files. I think this would also be confusing to new users.
Option 2: Add type checking to "validate" command
Perhaps the simplest fix is to instead modify the validate command like so:
"validate": "svelte-check && tsc --noEmit"
Then, the same command type checks everything.
I can see that there are pros and cons to doing this: the scope of svelte-check is more than simply type checking, so it could be argued that this approach starts to conflate concerns. However, the naming of the validate script is fairly generic, which is why I assumed it would validate my entire project, and type checking is the minimum level of validation I would expect from this command for a TypeScript project. Modifying the validate command like this is also very obvious and easy for users to change if they wish to take a different approach.
Other questions
Is there a "recommended" solution to this problem from Svelte? (One of the above, or something I'm missing?)
Is there any reason not to amend the setup script to adopt one of these potential fixes? I'd be happy to submit a PR if it's helpful.
Thanks!
The text was updated successfully, but these errors were encountered:
I'm opening this issue for discussion, as I see there are different ways to implement this. I'd be happy to submit a PR if there's a consensus.
The problem
Currently,
setupTypeScript.js
adds avalidate
npm script to runsvelte-check
, and this catches type errors in.svelte
files. However, it does not type check.ts
files, and it wasn't until I noticed I had type errors on my main branch (despite CI runningbuild
andvalidate
commands) that I realized this was the case. Admittedly, the docs forsvelte-check
make this pretty obvious in hindsight, but I had assumed that the default template for a TypeScript Svelte project would implement full type checking.There are a couple of simple options which could be added to the
setupTypeScript.js
script which would fix this.Option 1: Configure TypeScript plugin to fail on errors
The generated Rollup configuration does show warnings for type errors in
.ts
files, which hints at one solution to this: to add anoEmitOnError: true
option to the Rollup TypeScript plugin config. However, with this approach, one ends up with type checking for different file types occurring during different phases of the build or CI:validate
type checks.svelte
files, andbuild
type checks.ts
files. I think this would also be confusing to new users.Option 2: Add type checking to "validate" command
Perhaps the simplest fix is to instead modify the
validate
command like so:Then, the same command type checks everything.
I can see that there are pros and cons to doing this: the scope of
svelte-check
is more than simply type checking, so it could be argued that this approach starts to conflate concerns. However, the naming of thevalidate
script is fairly generic, which is why I assumed it would validate my entire project, and type checking is the minimum level of validation I would expect from this command for a TypeScript project. Modifying thevalidate
command like this is also very obvious and easy for users to change if they wish to take a different approach.Other questions
Thanks!
The text was updated successfully, but these errors were encountered: