Copy webServer from Typescript to VS Code #165771
Conversation
Not working. Also not correctly formatted, I'll do that later.
It works now
|
@mjbvz @jakebailey I think you are the interested parties here. |
|
This is the related TS issue for the package size savings: microsoft/TypeScript#51440 |
extensions/typescript-language-features/extension-browser.webpack.config.js
Outdated
Show resolved
Hide resolved
extensions/typescript-language-features/src/tsServer/webServer.ts
Outdated
Show resolved
Hide resolved
extensions/typescript-language-features/extension-browser.webpack.config.js
Outdated
Show resolved
Hide resolved
extensions/typescript-language-features/src/tsServer/webServer.ts
Outdated
Show resolved
Hide resolved
And prepare for getting rid of dynamicImportCompat.js hack
Instead of using CopyPlugin. 1. Shipping multiple entrypoints in a single file required fixes to build code. 2. There are a couple of warnings from `require` calls in tsserverlibrary.js. Those are not relevant since they're in non-web code, but I haven't figured how to turn them off; they are fully dynamic so `externals` didn't work.
| const config = typeof configOrFn === 'function' ? configOrFn({}, {}) : configOrFn; | ||
| if (outputRoot) { | ||
| config.output.path = path.join(outputRoot, path.relative(path.dirname(configPath), config.output.path)); | ||
| } | ||
| webpackConfigs.push(config); |
There was a problem hiding this comment.
I checked the webpack configs. None of them pass functions, which is good, because this code double-pushed function configs.
Nothing sets outputRoot that I could see, so the if (outputRoot) code is just-in-case. I left it because I don't know what it's supposed to do.
| // packaging depends on that and this must always be like it | ||
| filename: '[name].js', | ||
| path: path.join(__dirname, 'dist', 'browser'), | ||
| libraryTarget: undefined, |
There was a problem hiding this comment.
withBrowserDefaults sets libraryTarget: commonjs, which is not correct for tsserver.web.js. I don't know if it's correct for any callers, but there are lots of them so I didn't change it.
There was a problem hiding this comment.
It's correct for all callers because they should all be extensions, which even on web are CJS (they are executed in a funky eval environment).
| transform: async (content) => { | ||
| const dynamicImportCompatPath = path.join(__dirname, '..', 'node_modules', 'typescript', 'lib', 'dynamicImportCompat.js'); | ||
| const prefix = fs.existsSync(dynamicImportCompatPath) ? fs.readFileSync(dynamicImportCompatPath) : undefined; | ||
| const output = await Terser.minify(content.toString()); |
There was a problem hiding this comment.
withBrowserDefaults doesn't minify because it sets mode: none, but there's a comment that says mode: production will minify. So I didn't include a minify entry in tsserver.web.js' config.
|
@mjbvz I addressed your comments, and switched away from CopyPlugin in webpack, so can you take a look at the new commits and the follow-up comments I posted with them? |
|
Thank you @sandersn! Let's merge this and test it out further in the next insiders |
Previously, Typescript packaged the host/session combo needed for running vscode on the web into tsserver.js. tsserver.js would only execute normally if
processwas defined globally. Otherwise it would assume that it was running in a webworker and listen for a startup message. However, after Typescript's module conversion, building tsserver is much simpler if it remains a pure node executable (@jakebailey can explain). The code for the web is a completely separate 500-line chunk that was appended after the bulk of tsserver, so it's easily separable.This change copies webServer to the typescript language extension and switches vscode to package tsserverlibrary.js for the web instead of tsserver.js. We'll follow it up with a change that removes webServer.ts from tsserver.js in Typescript 5.0, which will slim down our build considerably. It also prepares the way for my update of the web host to use vscode-wasm as a virtual filesystem. It's structured exactly the same way--so it's likely that I'll eventually put the code from my update into the new webServer.ts file from this PR.
Note: Reads better with whitespace ignored.
Some notes:
5a.
getLogLevelis only available on tsserver.js, so I copied its content over. I copied a few trivial things likeconst noop = () => {}.ts.server.prefixes where needed. Vscode's build settings are stricter than typescript's so I had change a couple of places because of that.