Allow loading custom typescript.tsdk settings on web hosts#138211
Allow loading custom typescript.tsdk settings on web hosts#138211weswigham wants to merge 4 commits intomicrosoft:mainfrom
Conversation
| return [ | ||
| new TypeScriptVersion(source, | ||
| serverPath.toString(), | ||
| /*DiskTypeScriptVersionProvider.getApiVersion(serverPath)*/ API.fromVersionString('4.4.1'), // TODO: Pull version form URL if possible |
There was a problem hiding this comment.
Right now this API version is hardcoded - we could try and pull it from the URL (which could actually be anything, even a blob, so looking for a x.y.z in the url would be heuristic at best) or prefetch the document and search it for .versionMajorMinor = and pull out the version that way or assume the URL is a full directory structure and walk up to where we'd expect the package.json to be and fetch that. Unsure what we want to do - in either case we'd probably need to asyncify a few getters.
There was a problem hiding this comment.
I have a version that doesn't break any of the internal APIs by using a sync XMLHttpRequest to inspect the server's text contents for the version string. Could be OK - definitely would want to know if that or async'ing all these getters would be preferable.
There was a problem hiding this comment.
How much work would it be to use an async call? I worry that the sync request will stall all other extension operations
There was a problem hiding this comment.
Alternative: could it say "Downloading..." and then show the ts sdk version from the API when it's finished loading and easy to access?
There was a problem hiding this comment.
How much work would it be to use an async call? I worry that the sync request will stall all other extension operations
All the getters for different versions need to become getters for promises, and all of the functions using those getters need to become async. So pretty much everything version selector related. So long as that restructuring sounds fine, it's doable - just a large structural change for an otherwise pretty simple change. I'll finish up my local version of that for comparison - I originally bailed on it and prepped the sync request one (which shouldn't be that much worse than the sync FS calls in the non browser version) once I got to needing to refactor the getters types and thus existing APIs.
There was a problem hiding this comment.
So, I have an implementation with asyncness and lemme tell you: async is toxic. It had to flow out to everywhere.
271a10f to
e4125d3
Compare
extensions/typescript-language-features/src/utils/configuration.browser.ts
Outdated
Show resolved
Hide resolved
extensions/typescript-language-features/src/extension.browser.ts
Outdated
Show resolved
Hide resolved
| return [ | ||
| new TypeScriptVersion(source, | ||
| serverPath.toString(), | ||
| /*DiskTypeScriptVersionProvider.getApiVersion(serverPath)*/ API.fromVersionString('4.4.1'), // TODO: Pull version form URL if possible |
There was a problem hiding this comment.
How much work would it be to use an async call? I worry that the sync request will stall all other extension operations
|
A solution for the display name: We thought that the URL passed could contain a query string which includes the user-presentable display name. E.g. instead of It could be: Any CDN will ignore the query params, and it can be safely given with the source |
|
Technically it's more than just the display name - it also governs what features are enabled in the LS. It being in the user provided string as an override is pretty much the same as the user supplying it via the api version override option. |
|
Closing as out of date but let me know if this is something we want to revisit |
This allow you to set, eg,
{ "typescript.tsdk": "https://unpkg.com/typescript@4.5.2/lib" }in
vscode.devand get a functioning custom tsdk version (for any version of TS published after microsoft/TypeScript#39656 was pulled in) - very useful for debugging custom versions and PRs invscode.dev.cc @orta @mattbierner