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

Copy webServer from Typescript to VS Code #165771

Merged
merged 10 commits into from Nov 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions build/gulpfile.extensions.js
Expand Up @@ -64,6 +64,7 @@ const compilations = [
'references-view/tsconfig.json',
'simple-browser/tsconfig.json',
'typescript-language-features/test-workspace/tsconfig.json',
'typescript-language-features/web/tsconfig.json',
'typescript-language-features/tsconfig.json',
'vscode-api-tests/tsconfig.json',
'vscode-colorize-tests/tsconfig.json',
Expand Down
17 changes: 6 additions & 11 deletions build/lib/extensions.js
Expand Up @@ -414,19 +414,14 @@ async function webpackExtensions(taskName, isWatch, webpackConfigLocations) {
const webpackConfigs = [];
for (const { configPath, outputRoot } of webpackConfigLocations) {
const configOrFnOrArray = require(configPath);
function addConfig(configOrFn) {
let config;
if (typeof configOrFn === 'function') {
config = configOrFn({}, {});
function addConfig(configOrFnOrArray) {
for (const configOrFn of Array.isArray(configOrFnOrArray) ? configOrFnOrArray : [configOrFnOrArray]) {
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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
else {
config = configOrFn;
}
if (outputRoot) {
config.output.path = path.join(outputRoot, path.relative(path.dirname(configPath), config.output.path));
}
webpackConfigs.push(configOrFn);
}
addConfig(configOrFnOrArray);
}
Expand Down
18 changes: 6 additions & 12 deletions build/lib/extensions.ts
Expand Up @@ -506,20 +506,14 @@ export async function webpackExtensions(taskName: string, isWatch: boolean, webp

for (const { configPath, outputRoot } of webpackConfigLocations) {
const configOrFnOrArray = require(configPath);
function addConfig(configOrFn: webpack.Configuration | Function) {
let config;
if (typeof configOrFn === 'function') {
config = (configOrFn as Function)({}, {});
function addConfig(configOrFnOrArray: webpack.Configuration | ((env: unknown,args: unknown) => webpack.Configuration) | webpack.Configuration[]) {
for (const configOrFn of Array.isArray(configOrFnOrArray) ? configOrFnOrArray : [configOrFnOrArray]) {
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);
} else {
config = configOrFn;
}

if (outputRoot) {
config.output.path = path.join(outputRoot, path.relative(path.dirname(configPath), config.output.path));
}

webpackConfigs.push(configOrFn);
}
addConfig(configOrFnOrArray);
}
Expand Down
3 changes: 3 additions & 0 deletions extensions/.vscodeignore
@@ -0,0 +1,3 @@
node_modules/typescript/lib/tsc.js
node_modules/typescript/lib/typescriptServices.js
node_modules/typescript/lib/tsserverlibrary.js
1 change: 0 additions & 1 deletion extensions/postinstall.mjs
Expand Up @@ -26,7 +26,6 @@ function processRoot() {
function processLib() {
const toDelete = new Set([
'tsc.js',
'tsserverlibrary.js',
sandersn marked this conversation as resolved.
Show resolved Hide resolved
'typescriptServices.js',
]);

Expand Down
1 change: 1 addition & 0 deletions extensions/typescript-language-features/.vscodeignore
@@ -1,5 +1,6 @@
build/**
src/**
web/**
test/**
test-workspace/**
out/**
Expand Down
Expand Up @@ -7,8 +7,6 @@

'use strict';
const CopyPlugin = require('copy-webpack-plugin');
const Terser = require('terser');
const fs = require('fs');
const path = require('path');

const defaultConfig = require('../shared.webpack.config');
Expand All @@ -30,8 +28,7 @@ const languages = [
'tr',
'zh-cn',
];

module.exports = withBrowserDefaults({
module.exports = [withBrowserDefaults({
context: __dirname,
entry: {
extension: './src/extension.browser.ts',
Expand Down Expand Up @@ -60,30 +57,21 @@ module.exports = withBrowserDefaults({
}))
],
}),
// @ts-ignore
new CopyPlugin({
patterns: [
{
from: '../node_modules/typescript/lib/tsserver.js',
to: 'typescript/tsserver.web.js',
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());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (!output.code) {
throw new Error('Terser returned undefined code');
}

if (prefix) {
return prefix.toString() + '\n' + output.code;
}
return output.code;
},
transformPath: (targetPath) => {
return targetPath.replace('tsserver.js', 'tsserver.web.js');
}
}
],
}),
],
});
}), withBrowserDefaults({
context: __dirname,
entry: {
'typescript/tsserver.web': './web/webServer.ts'
},
ignoreWarnings: [/Critical dependency: the request of a dependency is an expression/],
output: {
// all output goes into `dist`.
// packaging depends on that and this must always be like it
filename: '[name].js',
path: path.join(__dirname, 'dist', 'browser'),
libraryTarget: undefined,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

},
externals: {
'perf_hooks': 'commonjs perf_hooks',
}
})];
15 changes: 15 additions & 0 deletions extensions/typescript-language-features/web/tsconfig.json
@@ -0,0 +1,15 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"outDir": "../../out",
"module": "nodenext",
"moduleDetection": "legacy",
"experimentalDecorators": true,
"types": [
"node"
]
},
"files": [
"webServer.ts"
]
}