Support using client watch in tsserver using events

[sheetalkamat]

This is alternative to #54012 wherein tsserver protocol supports changes to watch.

This is kind of like prototype 2 in that PR but instead of plugin that logic is in tsserver.

Tsserver uses createFileWatcher and createDirectoryWatcher events to let client know about which paths to watch or closeFileWatcher event to close the watcher. Each watcher is identified with id. Whenever client wants to notify server about the change, it does so using watchChange for that id and other details like file that got updated and whether it was create, delete or update

Vscode side changes are at sheetalkamat/vscode#2
microsoft/vscode#193848

[typescript-bot]

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @mjbvz, @zkat, and @joj for you. Feel free to loop in other consumers/maintainers if necessary

[jakebailey]

Just writing this down before I forget (I haven't read the full PR yet), but one thing we may want to consider is preemptively modeling this after LSP, specifically, moving the glob calculation to the server and sending that as part of the request instead. Right now, that's done over on the VS Code side (per https://github.com/sheetalkamat/vscode/pull/2/files), but in the LSP, there is only one file watching API and it accepts globs (https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspace_didChangeWatchedFiles).

This would mean that we would eventually have to pull the code back out of VS Code into tsserver anyway if we were to start using LSP.

[jakebailey]

Also of note is that LSP doesn't have watch IDs or anything; when changing watches, you have to send the entire list of desired watches. That feels expensive but I'm not sure what all we watch (I have yet to view a trace of the current PR set to see).

(maybe this isn't true but I think it is; my memory is foggy)

[sheetalkamat]

we are sending directory names to watch along with whether to watch recursively or not . I feel its better for API users to convert to whichever pattern they need it.

Ids are added because its easier and cheaper to figure out which Watcher to invoke instead of having to traffic around the complete data about paths and recursive etc info

[typescript-bot]

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

[jakebailey]

The PR seems fine, but did we reach a consensus on the method? I guess it was that we should do the protocol first to get VS Code fixed sooner? I recall from the meeting someone saying that we shouldn't modify protocol this soon before a release but we usually accept editor changes up until RC so I'm not against it if we are sure this is what we want to do. Perhaps as internal if it's the API we're worried about?

[DanielRosenwasser]

we shouldn't modify protocol this soon before a release

Likely before the actual release (or the RC)

[sheetalkamat]

@jakebailey chatted with daniel and it seems we can revert / change protocol changes if needed before 5.3 RC so this is good as is. Please review so i can get this in.

[jakebailey]

Okay, thanks!