-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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 support for a virtual filesystem #48497
Conversation
Except for the new test I added.
I should have done this as two commits. Sorry.
I'm pretty sure I need to delete and re-number some of the rest though.
Plus fix associated mistakes in the code found by the test.
Shortly to be undone, but whatever.
Doesn't quite build yet, uploading so I can use github to view the diff.
But editorServices needs some fixes. At least, createDirectory needs a recursive version.
Next: Make a way to create a vfs server from the start.
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, @amcasey, @mjbvz, @minestarks for you. Feel free to loop in other consumers/maintainers if necessary |
tests/baselines/reference/types.asyncGenerators.es2018.2.errors.txt
Outdated
Show resolved
Hide resolved
I'm not sure why this fails locally, I suspect I need to `npm ci`.
This removes fshost usage in most places except for construction of objects.
Instead, pass in additional configuration from TestServerHost
1. Move VirtualServerHost code to correct vfs/harness location. 2. Serialise watch and timeout information. 3. Improve test by modifying an unopened file. 4. Test can run arbitrary actions in addition to commands.
@@ -1748,7 +1747,16 @@ namespace ts.server { | |||
useCaseSensitiveFileNames: this.program.useCaseSensitiveFileNames(), | |||
}; | |||
} | |||
return this.projectService.host; | |||
return { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not needed either ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put this here to provide a ModuleResolutionHost which uses the projectService's [fs]host for fileExists, directoryExists, realpath, useCaseSensitiveFileNames instead of this.program.fileExists/directoryExists/realpath/useCaseSensitiveFileNames
. Maybe it's not needed? I don't know.
1. Reimplement recursive deleting in VirtualServerHost.deleteFile 2. Save a private fshost, then all VirtualServerHost methods directly. 3. Switch from asserts to logs when fshost isn't a valid VirtualServerHost.
This is obsoleted by the virtual filesystem written by the vscode team, and the tsserver host that I wrote for vscode to use it. |
@sandersn can you provide links to both? Would be interesting to see how the solution was implemented. Thanks :) |
The tsserver host is in vscode's repo at extensions/typescript-language-features/web/tsServer.ts |
Edit: Remaining work
npm install
on a local disk. It'll have to be substantially rewritten.Partly fixes #47600 -- there is probably more work needed here and definitely in vs code.