Skip to content

Commit dce8b93

Browse files
authored
Merge pull request #298894 from microsoft/joh/source-map-fixes
Enhance source map handling in NLS plugin and related components
2 parents a437b74 + 7bcb255 commit dce8b93

File tree

8 files changed

+458
-110
lines changed

8 files changed

+458
-110
lines changed

build/gulpfile.vscode.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,9 @@ function runTsGoTypeCheck(): Promise<void> {
237237
}
238238

239239
const sourceMappingURLBase = `https://main.vscode-cdn.net/sourcemaps/${commit}`;
240+
const isCI = !!process.env['CI'] || !!process.env['BUILD_ARTIFACTSTAGINGDIRECTORY'] || !!process.env['GITHUB_WORKSPACE'];
241+
const useCdnSourceMapsForPackagingTasks = isCI;
242+
const stripSourceMapsInPackagingTasks = isCI;
240243
const minifyVSCodeTask = task.define('minify-vscode', task.series(
241244
bundleVSCodeTask,
242245
util.rimraf('out-vscode-min'),
@@ -349,8 +352,11 @@ function packageTask(platform: string, arch: string, sourceFolderName: string, d
349352

350353
const extensions = gulp.src(['.build/extensions/**', ...platformSpecificBuiltInExtensionsExclusions], { base: '.build', dot: true });
351354

355+
const sourceFilterPattern = stripSourceMapsInPackagingTasks
356+
? ['**', '!**/*.{js,css}.map']
357+
: ['**'];
352358
const sources = es.merge(src, extensions)
353-
.pipe(filter(['**', '!**/*.{js,css}.map'], { dot: true }));
359+
.pipe(filter(sourceFilterPattern, { dot: true }));
354360

355361
let version = packageJson.version;
356362
const quality = (product as { quality?: string }).quality;
@@ -420,8 +426,13 @@ function packageTask(platform: string, arch: string, sourceFolderName: string, d
420426
const productionDependencies = getProductionDependencies(root);
421427
const dependenciesSrc = productionDependencies.map(d => path.relative(root, d)).map(d => [`${d}/**`, `!${d}/**/{test,tests}/**`]).flat().concat('!**/*.mk');
422428

429+
const depFilterPattern = ['**', `!**/${config.version}/**`, '!**/bin/darwin-arm64-87/**', '!**/package-lock.json', '!**/yarn.lock'];
430+
if (stripSourceMapsInPackagingTasks) {
431+
depFilterPattern.push('!**/*.{js,css}.map');
432+
}
433+
423434
const deps = gulp.src(dependenciesSrc, { base: '.', dot: true })
424-
.pipe(filter(['**', `!**/${config.version}/**`, '!**/bin/darwin-arm64-87/**', '!**/package-lock.json', '!**/yarn.lock', '!**/*.{js,css}.map']))
435+
.pipe(filter(depFilterPattern))
425436
.pipe(util.cleanNodeModules(path.join(import.meta.dirname, '.moduleignore')))
426437
.pipe(util.cleanNodeModules(path.join(import.meta.dirname, `.moduleignore.${process.platform}`)))
427438
.pipe(jsFilter)
@@ -701,7 +712,13 @@ BUILD_TARGETS.forEach(buildTarget => {
701712
if (useEsbuildTranspile) {
702713
const esbuildBundleTask = task.define(
703714
`esbuild-bundle${dashed(platform)}${dashed(arch)}${dashed(minified)}`,
704-
() => runEsbuildBundle(sourceFolderName, !!minified, true, 'desktop', minified ? `${sourceMappingURLBase}/core` : undefined)
715+
() => runEsbuildBundle(
716+
sourceFolderName,
717+
!!minified,
718+
true,
719+
'desktop',
720+
minified && useCdnSourceMapsForPackagingTasks ? `${sourceMappingURLBase}/core` : undefined
721+
)
705722
);
706723
vscodeTask = task.define(`vscode${dashed(platform)}${dashed(arch)}${dashed(minified)}`, task.series(
707724
copyCodiconsTask,

build/gulpfile.vscode.web.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const quality = (product as { quality?: string }).quality;
3333
const version = (quality && quality !== 'stable') ? `${packageJson.version}-${quality}` : packageJson.version;
3434

3535
// esbuild-based bundle for standalone web
36-
function runEsbuildBundle(outDir: string, minify: boolean, nls: boolean): Promise<void> {
36+
function runEsbuildBundle(outDir: string, minify: boolean, nls: boolean, sourceMapBaseUrl?: string): Promise<void> {
3737
return new Promise((resolve, reject) => {
3838
const scriptPath = path.join(REPO_ROOT, 'build/next/index.ts');
3939
const args = [scriptPath, 'bundle', '--out', outDir, '--target', 'web'];
@@ -44,6 +44,9 @@ function runEsbuildBundle(outDir: string, minify: boolean, nls: boolean): Promis
4444
if (nls) {
4545
args.push('--nls');
4646
}
47+
if (sourceMapBaseUrl) {
48+
args.push('--source-map-base-url', sourceMapBaseUrl);
49+
}
4750

4851
const proc = cp.spawn(process.execPath, args, {
4952
cwd: REPO_ROOT,
@@ -164,8 +167,9 @@ const minifyVSCodeWebTask = task.define('minify-vscode-web-OLD', task.series(
164167
gulp.task(minifyVSCodeWebTask);
165168

166169
// esbuild-based tasks (new)
170+
const sourceMappingURLBase = `https://main.vscode-cdn.net/sourcemaps/${commit}`;
167171
const esbuildBundleVSCodeWebTask = task.define('esbuild-vscode-web', () => runEsbuildBundle('out-vscode-web', false, true));
168-
const esbuildBundleVSCodeWebMinTask = task.define('esbuild-vscode-web-min', () => runEsbuildBundle('out-vscode-web-min', true, true));
172+
const esbuildBundleVSCodeWebMinTask = task.define('esbuild-vscode-web-min', () => runEsbuildBundle('out-vscode-web-min', true, true, `${sourceMappingURLBase}/core`));
169173

170174
function packageTask(sourceFolderName: string, destinationFolderName: string) {
171175
const destination = path.join(BUILD_ROOT, destinationFolderName);

build/next/index.ts

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,13 @@ ${tslib}`,
897897
const mangleStats: { file: string; result: ConvertPrivateFieldsResult }[] = [];
898898
// Map from JS file path to pre-mangle content + edits, for source map adjustment
899899
const mangleEdits = new Map<string, { preMangleCode: string; edits: readonly import('./private-to-property.ts').TextEdit[] }>();
900+
// Map from JS file path to pre-NLS content + edits, for source map adjustment
901+
const nlsEdits = new Map<string, { preNLSCode: string; edits: readonly import('./private-to-property.ts').TextEdit[] }>();
902+
// Defer .map files until all .js files are processed, because esbuild may
903+
// emit the .map file in a different build result than the .js file (e.g.
904+
// code-split chunks), and we need the NLS/mangle edits from the .js pass
905+
// to be available when adjusting the .map.
906+
const deferredMaps: { path: string; text: string; contents: Uint8Array }[] = [];
900907
for (const { result } of buildResults) {
901908
if (!result.outputFiles) {
902909
continue;
@@ -925,7 +932,12 @@ ${tslib}`,
925932

926933
// Apply NLS post-processing if enabled (JS only)
927934
if (file.path.endsWith('.js') && doNls && indexMap.size > 0) {
928-
content = postProcessNLS(content, indexMap, preserveEnglish);
935+
const preNLSCode = content;
936+
const nlsResult = postProcessNLS(content, indexMap, preserveEnglish);
937+
content = nlsResult.code;
938+
if (nlsResult.edits.length > 0) {
939+
nlsEdits.set(file.path, { preNLSCode, edits: nlsResult.edits });
940+
}
929941
}
930942

931943
// Rewrite sourceMappingURL to CDN URL if configured
@@ -943,16 +955,8 @@ ${tslib}`,
943955

944956
await fs.promises.writeFile(file.path, content);
945957
} else if (file.path.endsWith('.map')) {
946-
// Source maps may need adjustment if private fields were mangled
947-
const jsPath = file.path.replace(/\.map$/, '');
948-
const editInfo = mangleEdits.get(jsPath);
949-
if (editInfo) {
950-
const mapJson = JSON.parse(file.text);
951-
const adjusted = adjustSourceMap(mapJson, editInfo.preMangleCode, editInfo.edits);
952-
await fs.promises.writeFile(file.path, JSON.stringify(adjusted));
953-
} else {
954-
await fs.promises.writeFile(file.path, file.contents);
955-
}
958+
// Defer .map processing until all .js files have been handled
959+
deferredMaps.push({ path: file.path, text: file.text, contents: file.contents });
956960
} else {
957961
// Write other files (assets, etc.) as-is
958962
await fs.promises.writeFile(file.path, file.contents);
@@ -961,6 +965,27 @@ ${tslib}`,
961965
bundled++;
962966
}
963967

968+
// Second pass: process deferred .map files now that all mangle/NLS edits
969+
// have been collected from .js processing above.
970+
for (const mapFile of deferredMaps) {
971+
const jsPath = mapFile.path.replace(/\.map$/, '');
972+
const mangle = mangleEdits.get(jsPath);
973+
const nls = nlsEdits.get(jsPath);
974+
975+
if (mangle || nls) {
976+
let mapJson = JSON.parse(mapFile.text);
977+
if (mangle) {
978+
mapJson = adjustSourceMap(mapJson, mangle.preMangleCode, mangle.edits);
979+
}
980+
if (nls) {
981+
mapJson = adjustSourceMap(mapJson, nls.preNLSCode, nls.edits);
982+
}
983+
await fs.promises.writeFile(mapFile.path, JSON.stringify(mapJson));
984+
} else {
985+
await fs.promises.writeFile(mapFile.path, mapFile.contents);
986+
}
987+
}
988+
964989
// Log mangle-privates stats
965990
if (doManglePrivates && mangleStats.length > 0) {
966991
let totalClasses = 0, totalFields = 0, totalEdits = 0, totalElapsed = 0;

build/next/nls-plugin.ts

Lines changed: 80 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
analyzeLocalizeCalls,
1313
parseLocalizeKeyOrValue
1414
} from '../lib/nls-analysis.ts';
15+
import type { TextEdit } from './private-to-property.ts';
1516

1617
// ============================================================================
1718
// Types
@@ -148,12 +149,13 @@ export async function finalizeNLS(
148149

149150
/**
150151
* Post-processes a JavaScript file to replace NLS placeholders with indices.
152+
* Returns the transformed code and the edits applied (for source map adjustment).
151153
*/
152154
export function postProcessNLS(
153155
content: string,
154156
indexMap: Map<string, number>,
155157
preserveEnglish: boolean
156-
): string {
158+
): { code: string; edits: readonly TextEdit[] } {
157159
return replaceInOutput(content, indexMap, preserveEnglish);
158160
}
159161

@@ -244,7 +246,7 @@ function generateNLSSourceMap(
244246
const generator = new SourceMapGenerator();
245247
generator.setSourceContent(filePath, originalSource);
246248

247-
const lineCount = originalSource.split('\n').length;
249+
const lines = originalSource.split('\n');
248250

249251
// Group edits by line
250252
const editsByLine = new Map<number, NLSEdit[]>();
@@ -257,7 +259,7 @@ function generateNLSSourceMap(
257259
arr.push(edit);
258260
}
259261

260-
for (let line = 0; line < lineCount; line++) {
262+
for (let line = 0; line < lines.length; line++) {
261263
const smLine = line + 1; // source maps use 1-based lines
262264

263265
// Always map start of line
@@ -273,7 +275,8 @@ function generateNLSSourceMap(
273275

274276
let cumulativeShift = 0;
275277

276-
for (const edit of lineEdits) {
278+
for (let i = 0; i < lineEdits.length; i++) {
279+
const edit = lineEdits[i];
277280
const origLen = edit.endCol - edit.startCol;
278281

279282
// Map start of edit: the replacement begins at the same original position
@@ -285,12 +288,20 @@ function generateNLSSourceMap(
285288

286289
cumulativeShift += edit.newLength - origLen;
287290

288-
// Map content after edit: columns resume with the shift applied
289-
generator.addMapping({
290-
generated: { line: smLine, column: edit.endCol + cumulativeShift },
291-
original: { line: smLine, column: edit.endCol },
292-
source: filePath,
293-
});
291+
// Source maps don't interpolate columns — each query resolves to the
292+
// last segment with generatedColumn <= queryColumn. A single mapping
293+
// at edit-end would cause every subsequent column on this line to
294+
// collapse to that one original position. Add per-column identity
295+
// mappings from edit-end to the next edit (or end of line) so that
296+
// esbuild's source-map composition preserves fine-grained accuracy.
297+
const nextBound = i + 1 < lineEdits.length ? lineEdits[i + 1].startCol : lines[line].length;
298+
for (let origCol = edit.endCol; origCol < nextBound; origCol++) {
299+
generator.addMapping({
300+
generated: { line: smLine, column: origCol + cumulativeShift },
301+
original: { line: smLine, column: origCol },
302+
source: filePath,
303+
});
304+
}
294305
}
295306
}
296307
}
@@ -302,63 +313,80 @@ function replaceInOutput(
302313
content: string,
303314
indexMap: Map<string, number>,
304315
preserveEnglish: boolean
305-
): string {
306-
// Replace all placeholders in a single pass using regex
307-
// Two types of placeholders:
308-
// - %%NLS:moduleId#key%% for localize() - message replaced with null
309-
// - %%NLS2:moduleId#key%% for localize2() - message preserved
310-
// Note: esbuild may use single or double quotes, so we handle both
316+
): { code: string; edits: readonly TextEdit[] } {
317+
// Collect all matches first, then apply from back to front so that byte
318+
// offsets remain valid. Each match becomes a TextEdit in terms of the
319+
// ORIGINAL content offsets, which is what adjustSourceMap expects.
320+
321+
interface PendingEdit { start: number; end: number; replacement: string }
322+
const pending: PendingEdit[] = [];
311323

312324
if (preserveEnglish) {
313-
// Just replace the placeholder with the index (both NLS and NLS2)
314-
return content.replace(/["']%%NLS2?:([^%]+)%%["']/g, (match, inner) => {
315-
// Try NLS first, then NLS2
325+
const re = /["']%%NLS2?:([^%]+)%%["']/g;
326+
let m: RegExpExecArray | null;
327+
while ((m = re.exec(content)) !== null) {
328+
const inner = m[1];
316329
let placeholder = `%%NLS:${inner}%%`;
317330
let index = indexMap.get(placeholder);
318331
if (index === undefined) {
319332
placeholder = `%%NLS2:${inner}%%`;
320333
index = indexMap.get(placeholder);
321334
}
322335
if (index !== undefined) {
323-
return String(index);
336+
pending.push({ start: m.index, end: m.index + m[0].length, replacement: String(index) });
324337
}
325-
// Placeholder not found in map, leave as-is (shouldn't happen)
326-
return match;
327-
});
338+
}
328339
} else {
329-
// For NLS (localize): replace placeholder with index AND replace message with null
330-
// For NLS2 (localize2): replace placeholder with index, keep message
331-
// Note: Use (?:[^"\\]|\\.)* to properly handle escaped quotes like \" or \\
332-
// Note: esbuild may use single or double quotes, so we handle both
333-
334-
// First handle NLS (localize) - replace both key and message
335-
content = content.replace(
336-
/["']%%NLS:([^%]+)%%["'](\s*,\s*)(?:"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'|`(?:[^`\\]|\\.)*`)/g,
337-
(match, inner, comma) => {
338-
const placeholder = `%%NLS:${inner}%%`;
339-
const index = indexMap.get(placeholder);
340-
if (index !== undefined) {
341-
return `${index}${comma}null`;
342-
}
343-
return match;
340+
// NLS (localize): replace placeholder with index AND replace message with null
341+
const reNLS = /["']%%NLS:([^%]+)%%["'](\s*,\s*)(?:"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'|`(?:[^`\\]|\\.)*`)/g;
342+
let m: RegExpExecArray | null;
343+
while ((m = reNLS.exec(content)) !== null) {
344+
const inner = m[1];
345+
const comma = m[2];
346+
const placeholder = `%%NLS:${inner}%%`;
347+
const index = indexMap.get(placeholder);
348+
if (index !== undefined) {
349+
pending.push({ start: m.index, end: m.index + m[0].length, replacement: `${index}${comma}null` });
344350
}
345-
);
346-
347-
// Then handle NLS2 (localize2) - replace only key, keep message
348-
content = content.replace(
349-
/["']%%NLS2:([^%]+)%%["']/g,
350-
(match, inner) => {
351-
const placeholder = `%%NLS2:${inner}%%`;
352-
const index = indexMap.get(placeholder);
353-
if (index !== undefined) {
354-
return String(index);
355-
}
356-
return match;
351+
}
352+
353+
// NLS2 (localize2): replace only key, keep message
354+
const reNLS2 = /["']%%NLS2:([^%]+)%%["']/g;
355+
while ((m = reNLS2.exec(content)) !== null) {
356+
const inner = m[1];
357+
const placeholder = `%%NLS2:${inner}%%`;
358+
const index = indexMap.get(placeholder);
359+
if (index !== undefined) {
360+
pending.push({ start: m.index, end: m.index + m[0].length, replacement: String(index) });
357361
}
358-
);
362+
}
363+
}
359364

360-
return content;
365+
if (pending.length === 0) {
366+
return { code: content, edits: [] };
361367
}
368+
369+
// Sort by offset ascending, then apply back-to-front to keep offsets valid
370+
pending.sort((a, b) => a.start - b.start);
371+
372+
// Build TextEdit[] (in original-content coordinates) and apply edits
373+
const edits: TextEdit[] = [];
374+
for (const p of pending) {
375+
edits.push({ start: p.start, end: p.end, newText: p.replacement });
376+
}
377+
378+
// Apply edits using forward-scanning parts array — O(N+K) instead of
379+
// O(N*K) from repeated substring concatenation on large strings.
380+
const parts: string[] = [];
381+
let lastEnd = 0;
382+
for (const p of pending) {
383+
parts.push(content.substring(lastEnd, p.start));
384+
parts.push(p.replacement);
385+
lastEnd = p.end;
386+
}
387+
parts.push(content.substring(lastEnd));
388+
389+
return { code: parts.join(''), edits };
362390
}
363391

364392
// ============================================================================

0 commit comments

Comments
 (0)