Merge pull request #294716 from microsoft/dev/mjbvz/more-esbuild-ext-cleanup

Clean up esbuilding of extensions

build/gulpfile.extensions.ts

@@ -166,7 +166,7 @@ const tasks = compilations.map(function (tsconfigFile) {
 	const compileTask = task.define(`compile-extension:${name}`, task.series(cleanTask, async () => {
 		const nonts = gulp.src(src, srcOpts).pipe(filter(['**', '!**/*.ts'], { dot: true }));
 		const copyNonTs = util.streamToPromise(nonts.pipe(gulp.dest(out)));
-		const tsgo = spawnTsgo(absolutePath, { reporterId: 'extensions' }, () => rewriteTsgoSourceMappingUrlsIfNeeded(false, out, baseUrl));
+		const tsgo = spawnTsgo(absolutePath, { taskName: 'extensions' }, () => rewriteTsgoSourceMappingUrlsIfNeeded(false, out, baseUrl));
 
 		await Promise.all([copyNonTs, tsgo]);
 	}));
@@ -175,7 +175,7 @@ const tasks = compilations.map(function (tsconfigFile) {
 		const nonts = gulp.src(src, srcOpts).pipe(filter(['**', '!**/*.ts'], { dot: true }));
 		const watchInput = watcher(src, { ...srcOpts, ...{ readDelay: 200 } });
 		const watchNonTs = watchInput.pipe(filter(['**', '!**/*.ts'], { dot: true })).pipe(gulp.dest(out));
-		const tsgoStream = watchInput.pipe(util.debounce(() => createTsgoStream(absolutePath, { reporterId: 'extensions' }, () => rewriteTsgoSourceMappingUrlsIfNeeded(false, out, baseUrl)), 200));
+		const tsgoStream = watchInput.pipe(util.debounce(() => createTsgoStream(absolutePath, { taskName: 'extensions' }, () => rewriteTsgoSourceMappingUrlsIfNeeded(false, out, baseUrl)), 200));
 		const watchStream = es.merge(nonts.pipe(gulp.dest(out)), watchNonTs, tsgoStream);
 
 		return watchStream;
@@ -276,9 +276,9 @@ gulp.task(watchWebExtensionsTask);
 async function buildWebExtensions(isWatch: boolean): Promise<void> {
 	const extensionsPath = path.join(root, 'extensions');
 
-	// Find all esbuild-browser.ts files
+	// Find all esbuild.browser.mts files
 	const esbuildConfigLocations = await nodeUtil.promisify(glob)(
-		path.join(extensionsPath, '**', 'esbuild-browser.ts'),
+		path.join(extensionsPath, '**', 'esbuild.browser.mts'),
 		{ ignore: ['**/node_modules'] }
 	);
 
@@ -293,7 +293,11 @@ async function buildWebExtensions(isWatch: boolean): Promise<void> {
 
 	// Esbuild for extensions
 	if (esbuildConfigLocations.length > 0) {
-		promises.push(ext.esbuildExtensions('packaging web extension (esbuild)', isWatch, esbuildConfigLocations.map(script => ({ script }))));
+		promises.push(
+			ext.esbuildExtensions('packaging web extension (esbuild)', isWatch, esbuildConfigLocations.map(script => ({ script }))),
+			// Also run type check on extensions
+			...esbuildConfigLocations.map(script => ext.typeCheckExtension(path.dirname(script), true))
+		);
 	}
 
 	// Run webpack for remaining extensions

build/lib/extensions.ts

@@ -25,6 +25,7 @@ import { getProductionDependencies } from './dependencies.ts';
 import { type IExtensionDefinition, getExtensionStream } from './builtInExtensions.ts';
 import { getVersion } from './getVersion.ts';
 import { fetchUrls, fetchGithub } from './fetch.ts';
+import { createTsgoStream, spawnTsgo } from './tsgo.ts';
 import vzip from 'gulp-vinyl-zip';
 
 import { createRequire } from 'module';
@@ -67,23 +68,27 @@ function updateExtensionPackageJSON(input: Stream, update: (data: any) => any):
 function fromLocal(extensionPath: string, forWeb: boolean, disableMangle: boolean): Stream {
 
 	const esbuildConfigFileName = forWeb
-		? 'esbuild-browser.ts'
-		: 'esbuild.ts';
+		? 'esbuild.browser.mts'
+		: 'esbuild.mts';
 
 	const webpackConfigFileName = forWeb
 		? `extension-browser.webpack.config.js`
 		: `extension.webpack.config.js`;
 
 	const hasEsbuild = fs.existsSync(path.join(extensionPath, esbuildConfigFileName));
-	const isWebPacked = fs.existsSync(path.join(extensionPath, webpackConfigFileName));
+	const hasWebpack = fs.existsSync(path.join(extensionPath, webpackConfigFileName));
 
 	let input: Stream;
 	let isBundled = false;
 
 	if (hasEsbuild) {
-		input = fromLocalEsbuild(extensionPath, esbuildConfigFileName);
+		// Unlike webpack, esbuild only does bundling so we still want to run a separate type check step
+		input = es.merge(
+			fromLocalEsbuild(extensionPath, esbuildConfigFileName),
+			typeCheckExtensionStream(extensionPath, forWeb),
+		);
 		isBundled = true;
-	} else if (isWebPacked) {
+	} else if (hasWebpack) {
 		input = fromLocalWebpack(extensionPath, webpackConfigFileName, disableMangle);
 		isBundled = true;
 	} else {
@@ -105,6 +110,17 @@ function fromLocal(extensionPath: string, forWeb: boolean, disableMangle: boolea
 	return input;
 }
 
+export function typeCheckExtension(extensionPath: string, forWeb: boolean): Promise<void> {
+	const tsconfigFileName = forWeb ? 'tsconfig.browser.json' : 'tsconfig.json';
+	const tsconfigPath = path.join(extensionPath, tsconfigFileName);
+	return spawnTsgo(tsconfigPath, { taskName: 'typechecking extension (tsgo)', noEmit: true });
+}
+
+export function typeCheckExtensionStream(extensionPath: string, forWeb: boolean): Stream {
+	const tsconfigFileName = forWeb ? 'tsconfig.browser.json' : 'tsconfig.json';
+	const tsconfigPath = path.join(extensionPath, tsconfigFileName);
+	return createTsgoStream(tsconfigPath, { taskName: 'typechecking extension (tsgo)', noEmit: true });
+}
 
 function fromLocalWebpack(extensionPath: string, webpackConfigFileName: string, disableMangle: boolean): Stream {
 	const vsce = require('@vscode/vsce') as typeof import('@vscode/vsce');
@@ -267,6 +283,7 @@ function fromLocalEsbuild(extensionPath: string, esbuildConfigFileName: string):
 			if (error) {
 				return reject(error);
 			}
+
 			const matches = (stderr || '').match(/\> (.+): error: (.+)?/g);
 			fancyLog(`Bundled extension: ${ansiColors.yellow(path.join(path.basename(extensionPath), esbuildConfigFileName))} with ${matches ? matches.length : 0} errors.`);
 			for (const match of matches || []) {
@@ -632,17 +649,6 @@ export function translatePackageJSON(packageJSON: string, packageNLSPath: string
 
 const extensionsPath = path.join(root, 'extensions');
 
-// Additional projects to run esbuild on. These typically build code for webviews
-const esbuildMediaScripts = [
-	'ipynb/esbuild.mjs',
-	'markdown-language-features/esbuild-notebook.mjs',
-	'markdown-language-features/esbuild-preview.mjs',
-	'markdown-math/esbuild.mjs',
-	'mermaid-chat-features/esbuild-chat-webview.mjs',
-	'notebook-renderers/esbuild.mjs',
-	'simple-browser/esbuild-preview.mjs',
-];
-
 export async function webpackExtensions(taskName: string, isWatch: boolean, webpackConfigLocations: { configPath: string; outputRoot?: string }[]) {
 	const webpack = require('webpack') as typeof import('webpack');
 
@@ -742,6 +748,18 @@ export async function esbuildExtensions(taskName: string, isWatch: boolean, scri
 	await Promise.all(tasks);
 }
 
+
+// Additional projects to run esbuild on. These typically build code for webviews
+const esbuildMediaScripts = [
+	'ipynb/esbuild.notebook.mts',
+	'markdown-language-features/esbuild.notebook.mts',
+	'markdown-language-features/esbuild.webview.mts',
+	'markdown-math/esbuild.notebook.mts',
+	'mermaid-chat-features/esbuild.webview.mts',
+	'notebook-renderers/esbuild.notebook.mts',
+	'simple-browser/esbuild.webview.mts',
+];
+
 export function buildExtensionMedia(isWatch: boolean, outputRoot?: string): Promise<void> {
 	return esbuildExtensions('esbuilding extension media', isWatch, esbuildMediaScripts.map(p => ({
 		script: path.join(extensionsPath, p),

build/lib/tsgo.ts

@@ -3,37 +3,31 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import ansiColors from 'ansi-colors';
 import * as cp from 'child_process';
 import es from 'event-stream';
+import fancyLog from 'fancy-log';
 import * as path from 'path';
-import { createReporter } from './reporter.ts';
 
 const root = path.dirname(path.dirname(import.meta.dirname));
 const npx = process.platform === 'win32' ? 'npx.cmd' : 'npx';
 const ansiRegex = /[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g;
 
-export function spawnTsgo(projectPath: string, config: { reporterId: string }, onComplete?: () => Promise<void> | void): Promise<void> {
-	const reporter = createReporter(config.reporterId);
-	let report: NodeJS.ReadWriteStream | undefined;
-
-	const beginReport = (emitError: boolean) => {
-		if (report) {
-			report.end();
+export function spawnTsgo(projectPath: string, config: { taskName: string; noEmit?: boolean }, onComplete?: () => Promise<void> | void): Promise<void> {
+	function reporter(stdError: string) {
+		const matches = (stdError || '').match(/^error \w+: (.+)?/g);
+		fancyLog(`Finished ${ansiColors.green(config.taskName)} ${projectPath} with ${matches ? matches.length : 0} errors.`);
+		for (const match of matches || []) {
+			fancyLog.error(match);
 		}
-		report = reporter.end(emitError);
-	};
+	}
 
-	const endReport = () => {
-		if (!report) {
-			return;
-		}
-		report.end();
-		report = undefined;
-	};
-
-	beginReport(false);
-
-	const args = ['tsgo', '--project', projectPath, '--pretty', 'false', '--sourceMap', '--inlineSources'];
+	const args = ['tsgo', '--project', projectPath, '--pretty', 'false'];
+	if (config.noEmit) {
+		args.push('--noEmit');
+	} else {
+		args.push('--sourceMap', '--inlineSources');
+	}
 	const child = cp.spawn(npx, args, {
 		cwd: root,
 		stdio: ['ignore', 'pipe', 'pipe'],
@@ -47,23 +41,13 @@ export function spawnTsgo(projectPath: string, config: { reporterId: string }, o
 			return;
 		}
 		if (/Starting compilation|File change detected/i.test(trimmed)) {
-			beginReport(false);
 			return;
 		}
 		if (/Compilation complete/i.test(trimmed)) {
-			endReport();
 			return;
 		}
 
-		const match = /(.*\(\d+,\d+\): )(.*: )(.*)/.exec(trimmed);
-
-		if (match) {
-			const fullpath = path.isAbsolute(match[1]) ? match[1] : path.join(root, match[1]);
-			const message = match[3];
-			reporter(fullpath + message);
-		} else {
-			reporter(trimmed);
-		}
+		reporter(trimmed);
 	};
 
 	const handleData = (data: Buffer) => {
@@ -84,7 +68,7 @@ export function spawnTsgo(projectPath: string, config: { reporterId: string }, o
 				handleLine(buffer);
 				buffer = '';
 			}
-			endReport();
+
 			if (code === 0) {
 				Promise.resolve(onComplete?.()).then(() => resolve(), reject);
 			} else {
@@ -93,15 +77,13 @@ export function spawnTsgo(projectPath: string, config: { reporterId: string }, o
 		});
 
 		child.on('error', err => {
-			endReport();
 			reject(err);
 		});
 	});
 }
 
-export function createTsgoStream(projectPath: string, config: { reporterId: string }, onComplete?: () => Promise<void> | void): NodeJS.ReadWriteStream {
+export function createTsgoStream(projectPath: string, config: { taskName: string; noEmit?: boolean }, onComplete?: () => Promise<void> | void): NodeJS.ReadWriteStream {
 	const stream = es.through();
-
 	spawnTsgo(projectPath, config, onComplete).then(() => {
 		stream.emit('end');
 	}).catch(() => {