-
Notifications
You must be signed in to change notification settings - Fork 440
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
type-check reports errors twice #437
Comments
@sodatea @unshame I have an idea what the cause may be. The The solution could be changing the type check script in the package.json to: |
@sjihdazii yeah, but that defeats the purpose of having two configs. The .vitest.json config has additional globals defined which are only available in the .test files. |
Hi there, I just stumbled over this, too. Why does In other words, how about changing
Note that this requires setting something like
in EDIT: I uploaded some example code here. |
I was under the impression that vue-tsc didn't work with noEmit: false Edit: oh, yeah, that works, cool Edit2: seems not to work when importing .vue files in tests - I get a weird And I'm unable to include just the .vue files with Edit3: this issue has been fixed in volar 2.0.26 vuejs/language-tools#3526 so looks like this solution works 🎉 |
@unshame I'm having trouble following you but I'm glad you found a fix! For everyone else, I uploaded an example project the other day where I had |
@codethief I was updating the comment as I was trying to implement your suggestion, so it came out a bit incomprehensible. Your approach works with vue-tsc version 2 or later. That was my main point. It also works for npm/pnpm workspaces as I found out - I've been able to utilize ts project references properly by putting the following in the package.json of each workspace:
I never realized I could just reference the compiled types while still referencing the uncompiled code. And this works in vscode & webstorm without needing to pre-compile anything. This, with the vitest tsconfig optimization, has cut down typechecking times for my project significantly (to one minute from seven). So huge thanks! |
@cexbrayat would you consider using the approach @codethief proposed for this repo? I have some free time, so I can submit a PR. |
Sure, we would be glad to get a PR and see if that improves the situation! 👍 |
@cexbrayat what's the reason for putting e2e tsconfigs in the e2e folder and not including them in the references of the main tsconfig? Is it because of this same issue by any chance? |
One thing I'm trying to experiment with now is to:
With this configuration, files not imported into a |
If you do that, test code will no longer be able to import production code. The solution then is to either
I know the situation with project references is confusing and not at all well-documented – god knows they confused me for the longest time, too. However, I would suggest you take a closer look at the comments above, in particular, at the example code I shared and at @unshame's PR, since they already solve the present issue and use project references in the way they were intended. |
Strictly speaking, the code imported to the spec files runs in the test environment, not the app code's browser environment. IMO, for components being unit-tested, it's not a bug for the errors to be reported twice. Because it means the types are wrong in both the browser and test environments. |
That is very true. However, given that regular Vue code typically runs in a browser environment and test runners like Vitest emulate such an environment using JSDOM & friends, I'd say using project references in the way I outlined is a decent compromise for now. See also microsoft/TypeScript#37053 for the general discussion. N.B. If you ask me, the underlying issue is the presence of a global scope in JS to begin with. If JS/TS had monads or algebraic effects to encode side effects, then we could save ourselves this gigantic pile of pain. (For instance, importing production code in test code would lead the type checker to verify automatically whether the test environment provides the appropriate "global" scope that the production code needs.) |
I just came up with a somewhat contrived example before going to bed. I'll write it down here and continue the discussion later. Say the user is developing a web app utilizing the OPFS API, like https://github.com/diffusionstudio/vits-web Then, the following code, if present in
|
We are only using `references` in a solution-style tsconfig. According to discussions at microsoft/TypeScript#60465, such usage doesn't require `composite: true` to be set in sub-configs. Removing this field loosens the constraints on these configs that all files to be explicitly listed in `files` or `includes`. After this change, type errors in source code would only be reported twice if they're also imported by unit test specs, in constrast to always be reported twice prior to the change. I know this is not ideal yet, but it's still an improvement, and might help catch some edge cases such as vuejs#437 (comment) In the long run, we should still keep an eye on vuejs#549 (pending vuejs/language-tools#4750). Cross-referencing might be a more intuitive configuration, and should be the desirable configuration if we opt into Vitest Browser Mode.
We are only using `references` in a solution-style tsconfig. According to discussions at microsoft/TypeScript#60465, such usage doesn't require `composite: true` to be set in sub-configs. Removing this field loosens the constraints on these configs that all files to be explicitly listed in `files` or `includes`. After this change, type errors in source code would only be reported twice if they're also imported by unit test specs, in contrast to always be reported twice prior to the change. I know this is not ideal yet, but it's still an improvement, and might help catch some edge cases such as vuejs#437 (comment) In the long run, we should still keep an eye on vuejs#549 (pending vuejs/language-tools#4750). Cross-referencing might be a more intuitive configuration, and should be the desirable configuration if we opt into Vitest Browser Mode.
Creating a project with typescript and vitest
making a mistake somewhere and then running
type-check
script reports the errors twice:This is due to the fact that "src" is included in both
tsconfig.app.json
andtsconfig.vitest.json
. This was introduced with #274Not sure how to fix this. It's nice to have a separate tsconfig for vitest, but not sure it's worth it if it double reports the errors because of it.
The text was updated successfully, but these errors were encountered: