Refactor build system: replace createFileContentMapper with fileContentMapperPlugin for better integration with esbuild

This commit is contained in:
Johannes
2026-02-05 18:38:36 +01:00
parent 1ab650b351
commit 772a2d094e
2 changed files with 181 additions and 54 deletions

View File

@@ -430,53 +430,6 @@ function readISODate(outDir: string): string {
}
}
/**
* Creates a file content mapper for builds.
* This transforms JS files to inject build-time configuration.
* The placeholders get bundled into all entry points that import these modules.
*/
function createFileContentMapper(outDir: string, target: BuildTarget): (filePath: string, content: string) => string {
// Cache the replacement strings (computed once)
let productConfigReplacement: string | undefined;
let builtinExtensionsReplacement: string | undefined;
return (_filePath: string, content: string): string => {
// Inject product configuration (placeholder gets bundled into many files)
if (content.includes('/*BUILD->INSERT_PRODUCT_CONFIGURATION*/')) {
if (productConfigReplacement === undefined) {
// For server-web, remove webEndpointUrlTemplate
const productForTarget = target === 'server-web'
? { ...product, webEndpointUrlTemplate: undefined }
: product;
const productConfiguration = JSON.stringify({
...productForTarget,
version,
commit,
date: readISODate(outDir)
});
// Remove the outer braces since the placeholder is inside an object literal
productConfigReplacement = productConfiguration.substring(1, productConfiguration.length - 1);
}
content = content.replace('/*BUILD->INSERT_PRODUCT_CONFIGURATION*/', () => productConfigReplacement!);
}
// Inject built-in extensions list (placeholder gets bundled into files importing the scanner)
if (content.includes('/*BUILD->INSERT_BUILTIN_EXTENSIONS*/')) {
if (builtinExtensionsReplacement === undefined) {
// Web target uses .build/web/extensions (from compileWebExtensionsBuildTask)
// Other targets use .build/extensions
const extensionsRoot = target === 'web' ? '.build/web/extensions' : '.build/extensions';
const builtinExtensions = JSON.stringify(scanBuiltinExtensions(extensionsRoot));
// Remove the outer brackets since the placeholder is inside an array literal
builtinExtensionsReplacement = builtinExtensions.substring(1, builtinExtensions.length - 1);
}
content = content.replace('/*BUILD->INSERT_BUILTIN_EXTENSIONS*/', () => builtinExtensionsReplacement!);
}
return content;
};
}
/**
* Only used to make encoding tests happy. The source files don't have a BOM but the
* tests expect one... so we add it here.
@@ -641,6 +594,74 @@ function cssExternalPlugin(): esbuild.Plugin {
};
}
/**
* esbuild plugin that transforms source files to inject build-time configuration.
* This runs during onLoad so the transformation happens before esbuild processes the content,
* ensuring placeholders like `/*BUILD->INSERT_PRODUCT_CONFIGURATION* /` are replaced
* before esbuild strips them as non-legal comments.
*/
function fileContentMapperPlugin(outDir: string, target: BuildTarget): esbuild.Plugin {
// Cache the replacement strings (computed once)
let productConfigReplacement: string | undefined;
let builtinExtensionsReplacement: string | undefined;
return {
name: 'file-content-mapper',
setup(build) {
build.onLoad({ filter: /\.ts$/ }, async (args) => {
// Skip .d.ts files
if (args.path.endsWith('.d.ts')) {
return undefined;
}
let contents = await fs.promises.readFile(args.path, 'utf-8');
let modified = false;
// Inject product configuration
if (contents.includes('/*BUILD->INSERT_PRODUCT_CONFIGURATION*/')) {
if (productConfigReplacement === undefined) {
// For server-web, remove webEndpointUrlTemplate
const productForTarget = target === 'server-web'
? { ...product, webEndpointUrlTemplate: undefined }
: product;
const productConfiguration = JSON.stringify({
...productForTarget,
version,
commit,
date: readISODate(outDir)
});
// Remove the outer braces since the placeholder is inside an object literal
productConfigReplacement = productConfiguration.substring(1, productConfiguration.length - 1);
}
contents = contents.replace('/*BUILD->INSERT_PRODUCT_CONFIGURATION*/', () => productConfigReplacement!);
modified = true;
}
// Inject built-in extensions list
if (contents.includes('/*BUILD->INSERT_BUILTIN_EXTENSIONS*/')) {
if (builtinExtensionsReplacement === undefined) {
// Web target uses .build/web/extensions (from compileWebExtensionsBuildTask)
// Other targets use .build/extensions
const extensionsRoot = target === 'web' ? '.build/web/extensions' : '.build/extensions';
const builtinExtensions = JSON.stringify(scanBuiltinExtensions(extensionsRoot));
// Remove the outer brackets since the placeholder is inside an array literal
builtinExtensionsReplacement = builtinExtensions.substring(1, builtinExtensions.length - 1);
}
contents = contents.replace('/*BUILD->INSERT_BUILTIN_EXTENSIONS*/', () => builtinExtensionsReplacement!);
modified = true;
}
if (modified) {
return { contents, loader: 'ts' };
}
// No modifications, let esbuild handle normally
return undefined;
});
},
};
}
// ============================================================================
// Transpile (Goal 1: TS → JS using esbuild.transform for maximum speed)
// ============================================================================
@@ -741,6 +762,9 @@ ${tslib}`,
// Collect all build results (with write: false)
const buildResults: { outPath: string; result: esbuild.BuildResult }[] = [];
// Create the file content mapper plugin (injects product config, builtin extensions)
const contentMapperPlugin = fileContentMapperPlugin(outDir, target);
// Bundle each entry point directly from TypeScript source
await Promise.all(allEntryPoints.map(async (entryPoint) => {
const entryPath = path.join(REPO_ROOT, SRC_DIR, `${entryPoint}.ts`);
@@ -748,6 +772,8 @@ ${tslib}`,
// Use CSS external plugin for entry points that don't need bundled CSS
const plugins: esbuild.Plugin[] = bundleCssEntryPoints.has(entryPoint) ? [] : [cssExternalPlugin()];
// Add content mapper plugin to inject product config and builtin extensions
plugins.push(contentMapperPlugin);
if (doNls) {
plugins.unshift(nlsPlugin({
baseDir: path.join(REPO_ROOT, SRC_DIR),
@@ -807,7 +833,7 @@ ${tslib}`,
const outPath = path.join(REPO_ROOT, outDir, `${entry}.js`);
const bootstrapPlugins: esbuild.Plugin[] = [inlineMinimistPlugin()];
const bootstrapPlugins: esbuild.Plugin[] = [inlineMinimistPlugin(), contentMapperPlugin];
if (doNls) {
bootstrapPlugins.unshift(nlsPlugin({
baseDir: path.join(REPO_ROOT, SRC_DIR),
@@ -852,9 +878,6 @@ ${tslib}`,
indexMap = nlsResult.indexMap;
}
// Create file content mapper for web builds (injects product config, builtin extensions)
const fileContentMapper = createFileContentMapper(outDir, target);
// Post-process and write all output files
let bundled = 0;
for (const { result } of buildResults) {
@@ -873,9 +896,6 @@ ${tslib}`,
content = postProcessNLS(content, indexMap, preserveEnglish);
}
// Apply file content mapping (product config, builtin extensions)
content = fileContentMapper(file.path, content);
await fs.promises.writeFile(file.path, content);
} else {
// Write other files (source maps, etc.) as-is

107
build/next/working.md Normal file
View File

@@ -0,0 +1,107 @@
# Working Notes: New esbuild-based Build System
> These notes are for AI agents to help with context in new or summarized sessions.
## Important: Validating Changes
**The `VS Code - Build` task is NOT needed to validate changes in the `build/` folder!**
Build scripts in `build/` are TypeScript files that run directly with `tsx` (e.g., `npx tsx build/next/index.ts`). They are not compiled by the main VS Code build.
To test changes:
```bash
# Test transpile
npx tsx build/next/index.ts transpile --out out-test
# Test bundle (server-web target to test the auth fix)
npx tsx build/next/index.ts bundle --nls --target server-web --out out-vscode-reh-web-test
# Verify product config was injected
grep -l "serverLicense" out-vscode-reh-web-test/vs/code/browser/workbench/workbench.js
```
---
## Architecture Overview
### Files
- **[index.ts](index.ts)** - Main build orchestrator
- `transpile` command: Fast TS → JS using `esbuild.transform()`
- `bundle` command: TS → bundled JS using `esbuild.build()`
- **[nls-plugin.ts](nls-plugin.ts)** - NLS (localization) esbuild plugin
### Integration with Old Build
In [gulpfile.vscode.ts](../gulpfile.vscode.ts#L228-L242), the `core-ci` task uses these new scripts:
- `runEsbuildTranspile()` → transpile command
- `runEsbuildBundle()` → bundle command
Old gulp-based bundling renamed to `core-ci-OLD`.
---
## Key Learnings
### 1. Comment Stripping by esbuild
**Problem:** esbuild strips comments like `/*BUILD->INSERT_PRODUCT_CONFIGURATION*/` during bundling.
**Solution:** Use an `onLoad` plugin to transform source files BEFORE esbuild processes them. See `fileContentMapperPlugin()` in index.ts.
**Why post-processing doesn't work:** By the time we post-process the bundled output, the comment placeholder has already been stripped.
### 2. Authorization Error: "Unauthorized client refused"
**Root cause:** Missing product configuration in browser bundle.
**Flow:**
1. Browser loads with empty product config (placeholder was stripped)
2. `productService.serverLicense` is empty/undefined
3. Browser's `SignService.vsda()` can't decrypt vsda WASM (needs serverLicense as key)
4. Browser's `sign()` returns original challenge instead of signed value
5. Server validates signature → fails
6. Server is in built mode (no `VSCODE_DEV`) → rejects connection
**Fix:** The `fileContentMapperPlugin` now runs during `onLoad`, replacing placeholders before esbuild strips them.
### 3. Build-Time Placeholders
Two placeholders that need injection:
| Placeholder | Location | Purpose |
|-------------|----------|---------|
| `/*BUILD->INSERT_PRODUCT_CONFIGURATION*/` | `src/vs/platform/product/common/product.ts` | Product config (commit, version, serverLicense, etc.) |
| `/*BUILD->INSERT_BUILTIN_EXTENSIONS*/` | `src/vs/workbench/services/extensionManagement/browser/builtinExtensionsScannerService.ts` | List of built-in extensions |
### 4. Server-web Target Specifics
- Removes `webEndpointUrlTemplate` from product config (see `tweakProductForServerWeb` in old build)
- Uses `.build/extensions` for builtin extensions (not `.build/web/extensions`)
---
## Testing the Fix
```bash
# Build server-web with new system
npx tsx build/next/index.ts bundle --nls --target server-web --out out-vscode-reh-web-min
# Package it (uses gulp task)
npm run gulp vscode-reh-web-darwin-arm64-min
# Run server
./vscode-server-darwin-arm64-web/bin/code-server-oss --connection-token dev-token
# Open browser - should connect without "Unauthorized client refused"
```
---
## Open Items / Future Work
1. **`BUILD_INSERT_PACKAGE_CONFIGURATION`** - Server bootstrap files ([bootstrap-meta.ts](../../src/bootstrap-meta.ts)) have this marker for package.json injection. Currently handled by [inlineMeta.ts](../lib/inlineMeta.ts) in the old build's packaging step.
2. **Mangling** - The new build doesn't do TypeScript-based mangling yet. Old `core-ci` with mangling is now `core-ci-OLD`.
3. **Entry point duplication** - Entry points are duplicated between [buildfile.ts](../buildfile.ts) and [index.ts](index.ts). Consider consolidating.