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

tsserver throws when creating new .vue files as typescript.tsserver.experimental.useVsCodeWatcher defaults to true #214226

Closed
davidmatter opened this issue Jun 4, 2024 · 17 comments
Assignees
Labels
*caused-by-extension Issue identified to be caused by an extension

Comments

@davidmatter
Copy link

davidmatter commented Jun 4, 2024

Type: Bug

The new default == true of typescript.tsserver.experimental.useVsCodeWatcher (introduced in 1.89) https://code.visualstudio.com/updates/v1_89#_file-watching-handled-by-vs-code-core breaks the Volar extension.

  1. Install the volar extension
  2. pnpm create vite@latest
  3. Create a vue project
  4. pnpm install
  5. Create a new vue SFC file
  6. Try to import any dependency in the script section. Intellisense won't work and tsserver throws an error:
image

VS Code version: Code 1.89.1 (Universal) (dc96b83, 2024-05-07T05:14:24.611Z)
OS version: Darwin arm64 23.3.0
Modes:

System Info
Item Value
CPUs Apple M1 Pro (8 x 24)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: enabled_on
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_graphite: disabled_off
video_decode: enabled
video_encode: enabled
webgl: enabled
webgl2: enabled
webgpu: enabled
Load (avg) 4, 3, 3
Memory (System) 16.00GB (0.07GB free)
Process Argv --crash-reporter-id 86b723b3-271c-4c32-86b7-c446f0c21093
Screen Reader no
VM 0%
Extensions (13)
Extension Author (truncated) Version
vscode-tailwindcss bra 0.10.5
vscode-eslint dba 2.4.4
prettier-vscode esb 10.4.0
copilot Git 1.198.0
copilot-chat Git 0.15.2024043005
vscode-tsconfig-helper joh 0.4.3
dotenv mik 1.0.1
live-server ms- 0.4.13
vscode-typescript-next ms- 5.5.20240505
rose-pine mvl 2.9.0
vscode-coverage-gutters rya 2.11.1
vim vsc 1.27.3
volar Vue 2.0.19

(3 theme extensions excluded)

A/B Experiments
vsliv368:30146709
vspor879:30202332
vspor708:30202333
vspor363:30204092
vscod805cf:30301675
binariesv615:30325510
vsaa593cf:30376535
py29gd2263:31024239
c4g48928:30535728
azure-dev_surveyone:30548225
962ge761:30959799
pythongtdpath:30769146
welcomedialogc:30910334
pythonidxpt:30866567
pythonnoceb:30805159
asynctok:30898717
pythontestfixt:30902429
pythonregdiag2:30936856
pythonmypyd1:30879173
pythoncet0:30885854
h48ei257:31000450
pythontbext0:30879054
accentitlementsc:30995553
dsvsc016:30899300
dsvsc017:30899301
dsvsc018:30899302
cppperfnew:31000557
dsvsc020:30976470
pythonait:31006305
jchc7451:31066414
showvideoc:31016891
chatpanelc:31048052
dsvsc021:30996838
01bff139:31013167
pythoncenvpt:31062603
a69g1124:31058053
dvdeprecation:31061160
pythonprt:31056678
dwnewjupytercf:31046870
26j00206:31048877

@davidmatter davidmatter changed the title tsserver throws when creating new .vue files because typescript.tsserver.experimental.useVsCodeWatcher defaults to true tsserver throws when creating new .vue files as typescript.tsserver.experimental.useVsCodeWatcher defaults to true Jun 4, 2024
@johnsoncodehk
Copy link
Contributor

Thanks for your investigation!

@sheetalkamat I found that this change caused the TS plugin to never receive getExternalFiles calls with update level >= 1.

@mjbvz
Copy link
Collaborator

mjbvz commented Jun 17, 2024

Closing as caused by extension/plugin. Please file an issue against them

@mjbvz mjbvz closed this as completed Jun 17, 2024
@mjbvz mjbvz added the *caused-by-extension Issue identified to be caused by an extension label Jun 17, 2024
@johnsoncodehk
Copy link
Contributor

@mjbvz I don't think this is caused by the Vue extension, it seems more like an issue with communication between tsserver and typescript-language-features, or an internal state issue with tsserver when --canUseWatchEvents is enabled for .vue file change events. Can you reopen this issue?

I think VSCode needs to wait for tsserver to support the use case of using --canUseWatchEvents with the TS plugin before enabling typescript.tsserver.experimental.useVsCodeWatcher by default.

@mjbvz
Copy link
Collaborator

mjbvz commented Jul 16, 2024

Out of the box we won't watch vue files. That only happens if you have a vue extension/plugin installed

Also --canUseWatchEvents is the command line option to tsserver that the typescript.tsserver.experimental.useVsCodeWatcher setting enables. They are essentially the same thing

@johnsoncodehk
Copy link
Contributor

Sorry if we didn't make it clear. The Vue extension does not watch Vue files by default, but enables tsserver to recognize Vue files through the Vue TS plugin, and tsserver will watch Vue files on its own. When typescript.tsserver.experimental.useVsCodeWatcher is not enabled, tsserver works well with its own file watcher logic and Vue files, but after enabling typescript.tsserver.experimental.useVsCodeWatcher, tsserver no longer recognizes newly created Vue files.

Also --canUseWatchEvents is the command line option to tsserver that the typescript.tsserver.experimental.useVsCodeWatcher setting enables. They are essentially the same thing

What I mean is that if tsserver does not support --canUseWatchEvents to work with TS plugins for now, VSCode should not enable typescript.tsserver.experimental.useVsCodeWatcher by default, because it will break Vue/MDX extensions which using TS pugins to recognize extra file extensions in tsserver.

@mjbvz
Copy link
Collaborator

mjbvz commented Jul 16, 2024

The Vue extension does not watch Vue files by default, but enables tsserver to recognize Vue files through the Vue TS plugin, and tsserver will watch Vue files on its own

Yes and the problem is that the Vue extension needs to support the new file watching logic. They can follow up with TypeScript if they think there's some larger issues but the issue should start on the Vue side

@johnsoncodehk
Copy link
Contributor

I just want to reiterate that this issue is not specific to the Vue extension, as far as I know it also breaks the MDX extension, and will break the Glint extension in the next version (it will move to the TS plugin).

If you mean this issue should be reported to TypeScript by the Vue team, we can do that. However, the fix may not be released until 5.6, and before the tsdk built into VSCode is upgraded to that fixed version, typescript.tsserver.experimental.useVsCodeWatcher should be disabled by default. Since the Intellisense not working in new files is a significant issue, this problem is causing many complaints for the Vue extension, we are in a passive position as the new file watcher logic is not something the Vue extension can participate in.

@mjbvz
Copy link
Collaborator

mjbvz commented Jul 16, 2024

If you mean this issue should be reported to TypeScript by the Vue team, we can do that.

yes please do and provide them a detailed explanation of the issue

The new file watching has been enabled since the May VS Code release and was enabled for VS Code insiders as a test experiment before that. This should have been caught much earlier on the Vue side and reported to TS back when there was still time to get a quick fix in or hold off on enabling the feature by default. We're past that point now but could possibly get a fix into a TS 5.5.x release if the fix is relatively simple

@machty
Copy link

machty commented Jul 17, 2024

This is impacting Glint's TS Plugin as well.

We're past that point now

I don't think there's a strong argument against restoring the value of an experimental flag back to false until all known bugs impacting large communities like Vue are resolved.

@mjbvz
Copy link
Collaborator

mjbvz commented Jul 17, 2024

Has anyone actually opened an issue against TS yet?

You can downvote me all you want and think I'm being very unreasonable but I've told you what will move the issue forward. Please do it. It's really not difficult

I don't think there's a strong argument against restoring the value of an experimental flag back to false until all known bugs impacting large communities like Vue are resolved.

The file watching switch was made because file watching issues were impacting our users. We feel it is the best change for all of our js/ts users, which is a much, much larger set of folks than those using these various plugins

@Hoishin
Copy link

Hoishin commented Jul 17, 2024

You can say whatever you want, but

which is a much, much larger set of folks than those using these various plugins

isn't particularly great comment to make about community-run OSSs.

Also, as someone who haven't used Vue for ages, I think it's VSCode team's responsibility to test new versions against various popular plugins, especially something like Vue.

@yyx990803
Copy link

yyx990803 commented Jul 17, 2024

@mjbvz

The issue at hand

  • This experimental feature "improves" watching reliability for existing TS users, but all JS/TS features were working before this was enabled by default

  • At the same time enabling it by default literally breaks the essential workflow for users of the Vue extension (more than 1 million users).

  • This breakage relies on a fix in tsserver, and that might take a while.

Implications of whether to rollback the default

To me there two possible ways to handle this issue:

  1. Not rolling it back: JS/TS usage gets file watching improvements (it's not quantified in the release log and unnoticeable to me). In the meanwhile all Vue users are left with a broken DX until the upstream fix lands. Many of them have no idea of the underlying cause and spams us with 1 star reviews on VSCode marketplace, some with personal insults.

  2. Roll it back and wait for the fix to land: JS/TS usage stay on old watcher but doesn't affect normal usage; Vue users get normal DX in the meanwhile; No one gets mad during this period and everyone happy when the tsserver fix lands.

Isn't the ability to do (2) precisely the purpose of "experimental" features?


You can downvote me all you want and think I'm being very unreasonable but I've told you what will move the issue forward. Please do it.

Are you saying the VSCode team doesn't think it's their responsibility to ensure compatibility with tsserver, but rather it's the extension authors' responsibility to do so?

It's really not difficult

As already mentioned by @johnsoncodehk:

However, the fix may not be released until 5.6, and before the tsdk built into VSCode is upgraded to that fixed version, typescript.tsserver.experimental.useVsCodeWatcher should be disabled by default.

On the other hand - is it really that difficult to temporarily rollback the default value of a feature until it is actually stable?

@mjbvz
Copy link
Collaborator

mjbvz commented Jul 18, 2024

Repeating myself once again: open an issue against TypeScript that clearly explains that issue so this can be investigated and addressed. Ideally help them out too with a PR that fixes this

I even offered to try getting any TS side fix prioritized for a TS 5.5.x recovery release. That would be picked up by the next stable VS Code release, which is also when your proposed VS Code file watcher setting change would go in

@yyx990803
Copy link

  1. We are working on that in parallel.

  2. Getting the fix properly identified and eventually landed will take time. As I have explained: what we are asking is whether VSCode team is willing to provide a temporary default rollback to reduce the impact of this during this period. If VSCode team thinks the inconvenience of doing so justifies million+ of Vue users having broken DX for an unknown period of time, please state that clearly.

@Stanzilla
Copy link

as a workaround, could the Vue extension toggle the flag for the user? I know that's pretty bad in general but this case is special.

@myst729
Copy link

myst729 commented Jul 18, 2024

as a workaround, could the Vue extension toggle the flag for the user? I know that's pretty bad in general but this case is special.

This is so d**m "experimental" 😧

@mjbvz
Copy link
Collaborator

mjbvz commented Jul 19, 2024

I opened microsoft/TypeScript#59349 to track this. Initial investigation suggests the Vue plugin is using TS server apis in an unexpected way, although it's not yet clear if a fix is needed on the Vue or TS sides

Until there's a proper fix, here are a few possible workarounds to investigate:

  • The plugin code can try to disable the VS Code file watcher on the TS Server side. Not 100% sure it's possible but would be the best fix so it's worth investing

  • As @Stanzilla suggested, the Vue plugin can disable the new file watcher by changing user's VS Code setting. If you go this route, please reset the setting after the proper fix goes in so that future users can benefit from the new file watcher changes

I am now locking this issue as further discussion should happen on the issue TypeScript or in the Vue extension repos. Thank you to the folks helping more this forward, specifically to @johnsoncodehk plus @sheetalkamat and the TS team


At this point, I think it's worth recapping what happened so we can learn from it:

TypeScript server plugins like Vue are some of the most complicated and fragile parts of the VS Code ecosystem. The way they work is closer to patching our source code compared to our normal APIs. Given this, TS server plugin maintainers need to be extra diligent about testing on the latest VS Code releases and providing feedback to us early

VS Code shipped our file watching changes in early May with VS Code 1.89 (early April for VS Code insiders users). We documented these changes in our release notes and also rolled them out incrementally. If we had gotten a detailed issue report early in the rollout process, we could have prioritized a fix or potentially even delayed the rollout

An issue related to the new watcher was actually reported to Vue at the end of May (vuejs/language-tools#4424) It does not seem like this issue was investigated in depth on the Vue side at that time

This current issue was then reported to VS Code in early June. As the majority of extension specific issues are caused by extensions themselves, I delegated the issue back to Vue. After extension maintainers do a deeper investigation, if a bug is truly caused by VS Code, the maintainers can give us more detailed technical info and we can work together on a fix. Again though, the root cause is typically the extension code itself

I don't know why this issue blew up in the past few days. Even though I wish the issue has been investigated sooner, I also get it. Sometimes issues get lost or there's simply no time. When this happens, of course we still want to work towards finding a solution. But by mid July it was too late to revert the rollout of the new watcher. Not only would this regress the experience for non Vue users, rolling back to old code has its own risks

I am disappointed by how the issue has been handled since then. When issues like this happen, it is not ok to try shifting all of the blame to us or to come in and demand that we make changes without showing that you are willing to do the serious work to understand or address the root of the problem. This really should have been as simple as: "Here's the issue where we're tracking the bug on our side, it should be fixed in our next release. We considered all other approaches but think a workaround is needed on the VS Code side too, so until our fix goes out here's a PR that works around this for just the Vue users."

I tried to help in this issue by repeatedly giving specific instructions on how to get this issue moving forward. In response I was ignored and disrespected. This behavior has gone completely against good open source etiquette and it contributed nothing to getting the issue fixed

At the end of the day I know we are all trying to build the best tools for our users. It's good to be passionate but that passion needs to be channeled in a productive and professional way

As I said, it looks like we've got good momentum getting this addressed now. Hopefully this whole experience can also be a good learning moment so we can all do better going forward

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*caused-by-extension Issue identified to be caused by an extension
Projects
None yet
Development

No branches or pull requests

8 participants