From b396c83d5a61a84f2db5f5b9022c0e22bdae1cf9 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 7 Apr 2026 15:59:08 -0400 Subject: [PATCH 01/14] Fix relative sourcemap comment paths --- src/FileUtils.ts | 4 +- src/managers/ProjectManager.spec.ts | 269 ++++++++++++++++++++++++++-- src/managers/ProjectManager.ts | 168 ++++++++++++----- 3 files changed, 378 insertions(+), 63 deletions(-) diff --git a/src/FileUtils.ts b/src/FileUtils.ts index 3a04d1f9..324df38b 100644 --- a/src/FileUtils.ts +++ b/src/FileUtils.ts @@ -205,7 +205,7 @@ export class FileUtils { * force all drive letters to lower case (because that's what VSCode does sometimes so this makes it consistent) * @param thePath */ - public standardizePath(thePath: string) { + public standardizePath(thePath: string): string { if (!thePath) { return thePath; } @@ -330,7 +330,7 @@ export let fileUtils = new FileUtils(); /** * A tagged template literal function for standardizing the path. */ -export function standardizePath(stringParts, ...expressions: any[]) { +export function standardizePath(stringParts, ...expressions: any[]): string { let result = []; for (let i = 0; i < stringParts.length; i++) { result.push(stringParts[i], expressions[i]); diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index 5c43e11b..30ef4f9d 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -228,7 +228,7 @@ describe('ProjectManager', () => { // Simulate the full flow: // 1. compiler produces MainScene.brs + MainScene.brs.map in srcDir, with sources relative to srcDir // 2. prepublishToStaging copies them to stagingDir (recorded in fileMappings) - // 3. fixSourceMapSources rewrites the map's sources to be relative to stagingDir + // 3. preprocessStagingFiles rewrites the map's sources to be relative to stagingDir // 4. getSourceLocation('pkg:/source/MainScene.brs', 1) resolves back to rootDir/source/MainScene.bs const srcDir = s`${tempPath}/srcDir/source`; @@ -260,10 +260,10 @@ describe('ProjectManager', () => { { src: originalMapPath, dest: stagingMapPath } ]; - // fixSourceMapSources rewrites the on-disk map to use paths relative to stagingDir + // preprocessStagingFiles rewrites the on-disk map to use paths relative to stagingDir const project = new Project({ rootDir: rootDir, outDir: outDir, files: [], stagingDir: stagingDir, enhanceREPLCompletions: false }); project.fileMappings = fileMappings; - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); // Point the manager's mainProject at this project manager.mainProject = project as any; @@ -403,7 +403,7 @@ describe('Project', () => { }); }); - describe('fixSourceMapSources', () => { + describe('preprocessStagingFiles', () => { afterEach(() => { try { fsExtra.removeSync(tempPath); @@ -436,7 +436,7 @@ describe('Project', () => { { src: originalMapPath, dest: stagingMapPath } ]; - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); const updated = fsExtra.readJsonSync(stagingMapPath); // Resolve what the source path should be after rewriting @@ -457,7 +457,7 @@ describe('Project', () => { { src: mapPath, dest: mapPath } ]; - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); const unchanged = fsExtra.readJsonSync(mapPath); expect(unchanged.sources[0]).to.equal('../source/main.bs'); @@ -473,7 +473,7 @@ describe('Project', () => { // fileMappings does NOT include this map file project.fileMappings = []; - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); const unchanged = fsExtra.readJsonSync(mapPath); expect(unchanged.sources[0]).to.equal('../source/main.bs'); @@ -501,7 +501,7 @@ describe('Project', () => { { src: originalMapPath, dest: stagingMapPath } ]; - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); const updated = fsExtra.readJsonSync(stagingMapPath); const absoluteSource = path.resolve(path.dirname(originalMapPath), '../../rootDir/source/main.bs'); @@ -533,7 +533,7 @@ describe('Project', () => { { src: originalMapPath, dest: stagingMapPath } ]; - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); const updated = fsExtra.readJsonSync(stagingMapPath); const absoluteSource = path.resolve(path.dirname(originalMapPath), '../rootDir', 'source/main.bs'); @@ -566,7 +566,7 @@ describe('Project', () => { { src: originalMapPath, dest: stagingMapPath } ]; - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); const updated = fsExtra.readJsonSync(stagingMapPath); const absoluteSource = path.resolve(absoluteSourceRoot, 'source/main.bs'); @@ -592,7 +592,7 @@ describe('Project', () => { fsExtra.copySync(originalMapPath, stagingMapPath); project.fileMappings = [{ src: originalMapPath, dest: stagingMapPath }]; - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); const updated = fsExtra.readJsonSync(stagingMapPath); const stagingMapDir = path.dirname(stagingMapPath); @@ -619,7 +619,7 @@ describe('Project', () => { fsExtra.copySync(originalMapPath, stagingMapPath); project.fileMappings = [{ src: originalMapPath, dest: stagingMapPath }]; - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); const updated = fsExtra.readJsonSync(stagingMapPath); const absoluteSource = path.resolve(path.dirname(originalMapPath), '../../rootDir/source/main.bs'); @@ -645,7 +645,7 @@ describe('Project', () => { fsExtra.copySync(originalMapPath, stagingMapPath); project.fileMappings = [{ src: originalMapPath, dest: stagingMapPath }]; - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); const updated = fsExtra.readJsonSync(stagingMapPath); const absoluteSource = path.resolve(path.dirname(originalMapPath), '../../../../src/components/views/details/Details.bs'); @@ -674,7 +674,7 @@ describe('Project', () => { { src: originalMapPathA, dest: stagingMapPathA }, { src: originalMapPathB, dest: stagingMapPathB } ]; - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); const stagingMapDir = path.dirname(stagingMapPathA); const originalMapDir = path.dirname(originalMapPathA); @@ -699,7 +699,7 @@ describe('Project', () => { project.fileMappings = [{ src: originalMapPath, dest: stagingMapPath }]; // should not throw - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); // file should be unchanged const unchanged = fsExtra.readJsonSync(stagingMapPath); @@ -715,7 +715,244 @@ describe('Project', () => { project.fileMappings = [{ src: s`${tempPath}/srcDir/main.brs.map`, dest: stagingMapPath }]; // should not throw - await project['fixSourceMapSources'](); + await project['preprocessStagingFiles'](); + }); + + describe('fixSourceMapComment', () => { + /** + * Primary scenario: .map is OUTSIDE rootDir so it was never staged. + * + * /alpha/beta/charlie/rootDir/source/main.brs → comment: '../../../../../maps/main.brs.map' + * /alpha/maps/main.brs.map (not copied — outside rootDir) + * + * After staging: + * staging/source/main.brs → comment must be rewritten to an absolute-equivalent + * relative path from the new staging location back to + * the original (unstaged) map file. + */ + it('rewrites the comment to point at the original map when the map was not staged', async () => { + const rootDirSource = s`${tempPath}/alpha/beta/charlie/rootDir/source`; + const mapsDir = s`${tempPath}/alpha/maps`; + const stagingSourceDir = s`${stagingDir}/source`; + + fsExtra.ensureDirSync(rootDirSource); + fsExtra.ensureDirSync(mapsDir); + fsExtra.ensureDirSync(stagingSourceDir); + + const originalBrsPath = s`${rootDirSource}/main.brs`; + const originalMapPath = s`${mapsDir}/main.brs.map`; + const stagingBrsPath = s`${stagingSourceDir}/main.brs`; + + // The comment in the original file points relatively from rootDirSource → mapsDir + const originalRelative = s`${path.relative(rootDirSource, originalMapPath)}`; + fsExtra.writeFileSync(originalBrsPath, `sub main()\nend sub\n'//# sourceMappingURL=${originalRelative}`); + + // Only the .brs is staged — the .map is outside rootDir and never copied + fsExtra.copySync(originalBrsPath, stagingBrsPath); + + project.fileMappings = [ + { src: originalBrsPath, dest: stagingBrsPath } + // map intentionally absent + ]; + + await project['preprocessStagingFiles'](); + + const updatedContents = fsExtra.readFileSync(stagingBrsPath, 'utf8'); + // The new comment must resolve back to the same absolute map path + const commentMatch = /'\/\/# sourceMappingURL=(.+)$/.exec(updatedContents); + expect(commentMatch, 'sourceMappingURL comment should still be present').to.exist; + const resolvedMapPath = fileUtils.standardizePath( + path.resolve(path.dirname(stagingBrsPath), commentMatch[1]) + ); + expect(resolvedMapPath).to.equal(originalMapPath); + }); + + /** + * When the map WAS staged (at a different relative location), the comment + * should point at the staged copy, not the original. + */ + it('rewrites the comment to point at the staged map when the map was also staged', async () => { + const srcBrsDir = s`${tempPath}/src/components/views`; + const srcMapDir = s`${tempPath}/src/components/maps`; + const stagingSourceDir = s`${stagingDir}/source`; + + fsExtra.ensureDirSync(srcBrsDir); + fsExtra.ensureDirSync(srcMapDir); + fsExtra.ensureDirSync(stagingSourceDir); + + const originalBrsPath = s`${srcBrsDir}/main.brs`; + const originalMapPath = s`${srcMapDir}/main.brs.map`; + const stagingBrsPath = s`${stagingSourceDir}/main.brs`; + const stagingMapPath = s`${stagingSourceDir}/main.brs.map`; + + const originalRelative = s`${path.relative(srcBrsDir, originalMapPath)}`; + fsExtra.writeFileSync(originalBrsPath, `sub main()\nend sub\n'//# sourceMappingURL=${originalRelative}`); + fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); + + fsExtra.copySync(originalBrsPath, stagingBrsPath); + fsExtra.copySync(originalMapPath, stagingMapPath); + + project.fileMappings = [ + { src: originalBrsPath, dest: stagingBrsPath }, + { src: originalMapPath, dest: stagingMapPath } + ]; + + await project['preprocessStagingFiles'](); + + const updatedContents = fsExtra.readFileSync(stagingBrsPath, 'utf8'); + // Both were staged as siblings, so comment should just be the filename + expect(updatedContents).to.contain(`'//# sourceMappingURL=main.brs.map`); + }); + + it('does not rewrite the comment when the relative path is already correct after staging', async () => { + // .brs and .map are siblings in both source and staging — no change needed + const srcDir = s`${tempPath}/src/source`; + const stagingSourceDir = s`${stagingDir}/source`; + + fsExtra.ensureDirSync(srcDir); + fsExtra.ensureDirSync(stagingSourceDir); + + const originalBrsPath = s`${srcDir}/main.brs`; + const originalMapPath = s`${srcDir}/main.brs.map`; + const stagingBrsPath = s`${stagingSourceDir}/main.brs`; + const stagingMapPath = s`${stagingSourceDir}/main.brs.map`; + + const originalContents = `sub main()\nend sub\n'//# sourceMappingURL=main.brs.map`; + fsExtra.writeFileSync(originalBrsPath, originalContents); + fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); + + fsExtra.copySync(originalBrsPath, stagingBrsPath); + fsExtra.copySync(originalMapPath, stagingMapPath); + + project.fileMappings = [ + { src: originalBrsPath, dest: stagingBrsPath }, + { src: originalMapPath, dest: stagingMapPath } + ]; + + await project['preprocessStagingFiles'](); + + // File should be unchanged (no write needed) + expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); + }); + + it('rewrites the XML format comment ()', async () => { + const srcXmlDir = s`${tempPath}/src/components/views`; + const srcMapDir = s`${tempPath}/src/components/maps`; + const stagingSourceDir = s`${stagingDir}/source`; + + fsExtra.ensureDirSync(srcXmlDir); + fsExtra.ensureDirSync(srcMapDir); + fsExtra.ensureDirSync(stagingSourceDir); + + const originalXmlPath = s`${srcXmlDir}/MainScene.xml`; + const originalMapPath = s`${srcMapDir}/MainScene.xml.map`; + const stagingXmlPath = s`${stagingSourceDir}/MainScene.xml`; + const stagingMapPath = s`${stagingSourceDir}/MainScene.xml.map`; + + const originalRelative = s`${path.relative(srcXmlDir, originalMapPath)}`; + fsExtra.writeFileSync(originalXmlPath, `\n\n`); + fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); + + fsExtra.copySync(originalXmlPath, stagingXmlPath); + fsExtra.copySync(originalMapPath, stagingMapPath); + + project.fileMappings = [ + { src: originalXmlPath, dest: stagingXmlPath }, + { src: originalMapPath, dest: stagingMapPath } + ]; + + await project['preprocessStagingFiles'](); + + const updatedContents = fsExtra.readFileSync(stagingXmlPath, 'utf8'); + expect(updatedContents).to.contain(``); + }); + + it('injects a comment when there is none but a sidecar .map exists next to the original source', async () => { + // Scenario: user's files array omits map files, so only the .brs is staged. + // The .map exists next to the original .brs in rootDir but was never copied. + // We should inject a comment in the staged .brs pointing back at the original map. + const srcDir = s`${tempPath}/rootDir/source`; + const stagingSourceDir = s`${stagingDir}/source`; + + fsExtra.ensureDirSync(srcDir); + fsExtra.ensureDirSync(stagingSourceDir); + + const originalBrsPath = s`${srcDir}/main.brs`; + const originalMapPath = s`${srcDir}/main.brs.map`; // sidecar, never staged + const stagingBrsPath = s`${stagingSourceDir}/main.brs`; + + fsExtra.writeFileSync(originalBrsPath, `sub main()\nend sub`); + fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); + + // Only the .brs is staged — map was excluded from files array + fsExtra.copySync(originalBrsPath, stagingBrsPath); + + project.fileMappings = [ + { src: originalBrsPath, dest: stagingBrsPath } + ]; + + await project['preprocessStagingFiles'](); + + const updatedContents = fsExtra.readFileSync(stagingBrsPath, 'utf8'); + const commentMatch = /'\/\/# sourceMappingURL=(.+)$/.exec(updatedContents); + expect(commentMatch, 'sourceMappingURL comment should have been injected').to.exist; + // The injected path must resolve back to the original (unstaged) map file + const resolvedMapPath = fileUtils.standardizePath( + path.resolve(path.dirname(stagingBrsPath), commentMatch[1]) + ); + expect(resolvedMapPath).to.equal(originalMapPath); + }); + + it('does not inject a comment when there is none but the sidecar .map was staged alongside the .brs', async () => { + const srcDir = s`${tempPath}/rootDir/source`; + const stagingSourceDir = s`${stagingDir}/source`; + + fsExtra.ensureDirSync(srcDir); + fsExtra.ensureDirSync(stagingSourceDir); + + const originalBrsPath = s`${srcDir}/main.brs`; + const originalMapPath = s`${srcDir}/main.brs.map`; + const stagingBrsPath = s`${stagingSourceDir}/main.brs`; + const stagingMapPath = s`${stagingSourceDir}/main.brs.map`; + + const originalContents = `sub main()\nend sub`; + fsExtra.writeFileSync(originalBrsPath, originalContents); + fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); + + fsExtra.copySync(originalBrsPath, stagingBrsPath); + fsExtra.copySync(originalMapPath, stagingMapPath); + + project.fileMappings = [ + { src: originalBrsPath, dest: stagingBrsPath }, + { src: originalMapPath, dest: stagingMapPath } + ]; + + await project['preprocessStagingFiles'](); + + // No comment should have been injected — the map is already a sibling in staging + expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); + }); + + it('does not crash when .brs has no sourceMappingURL comment', async () => { + const srcDir = s`${tempPath}/src/source`; + const stagingSourceDir = s`${stagingDir}/source`; + + fsExtra.ensureDirSync(srcDir); + fsExtra.ensureDirSync(stagingSourceDir); + + const originalBrsPath = s`${srcDir}/main.brs`; + const stagingBrsPath = s`${stagingSourceDir}/main.brs`; + + const originalContents = `sub main()\nend sub\n`; + fsExtra.writeFileSync(originalBrsPath, originalContents); + fsExtra.copySync(originalBrsPath, stagingBrsPath); + + project.fileMappings = [{ src: originalBrsPath, dest: stagingBrsPath }]; + + await project['preprocessStagingFiles'](); + + expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); + }); }); }); diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index 561440a8..472da1fc 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -3,9 +3,7 @@ import * as fsExtra from 'fs-extra'; import * as path from 'path'; import { rokuDeploy, RokuDeploy, util as rokuDeployUtil } from 'roku-deploy'; import type { FileEntry } from 'roku-deploy'; -import * as glob from 'glob'; -import { promisify } from 'util'; -const globAsync = promisify(glob); +import * as fastGlob from 'fast-glob'; import type { BreakpointManager } from './BreakpointManager'; import { fileUtils, standardizePath as s } from '../FileUtils'; import type { LocationManager, SourceLocation } from './LocationManager'; @@ -298,10 +296,14 @@ export interface AddProjectParams { export class Project { constructor(params: AddProjectParams) { - assert(params?.rootDir, 'rootDir is required'); + if (!params?.rootDir) { + throw new Error('rootDir is required'); + } this.rootDir = fileUtils.standardizePath(params.rootDir); - assert(params?.outDir, 'outDir is required'); + if (!params?.outDir) { + throw new Error('outDir is required'); + } this.outDir = fileUtils.standardizePath(params.outDir); this.stagingDir = params.stagingDir ?? rokuDeploy.getOptions(this).stagingDir; this.bsConst = params.bsConst; @@ -368,7 +370,7 @@ export class Project { outDir: this.outDir }); - await this.fixSourceMapSources(); + await this.preprocessStagingFiles(); if (this.enhanceREPLCompletions) { //activate our background brighterscript ProgramBuilder now that the staging directory contains the final production project @@ -440,59 +442,135 @@ export class Project { } /** - * Find all .map files in the staging directory and update their `sources` paths to be - * relative to the staging map file location instead of the original source location. - * This ensures source maps work correctly when the stagingDir differs from the original - * source directory (e.g. when using sourceDirs or a customized stagingDir). + * Walk every staged file once and apply all necessary rewrites for files that were moved + * from a different source location: + * - .map files: rewrite `sources` paths to be relative to the new staging location + * - .brs/.xml files: rewrite the sourceMappingURL comment path to point to the staged map */ - private async fixSourceMapSources() { - // Build a lookup from staging dest path -> original src path - const stagingToSrcMap = new Map(); + private async preprocessStagingFiles() { + const srcToDestMap = new Map(); + const destToSrcMap = new Map(); for (const mapping of this.fileMappings) { - stagingToSrcMap.set(mapping.dest, mapping.src); + srcToDestMap.set(mapping.src, mapping.dest); + destToSrcMap.set(mapping.dest, mapping.src); } - // Find all .map files currently in the staging directory - const mapFiles = (await globAsync('**/*.map', { cwd: this.stagingDir, absolute: true })) - .map(f => fileUtils.standardizePath(f)); + //walk over every file + const stagedFiles: string[] = (await fastGlob('**/*', { cwd: this.stagingDir, absolute: true, onlyFiles: true })) + .map((f: string) => fileUtils.standardizePath(f)); - await Promise.all(mapFiles.map(async (stagingMapPath) => { - const originalMapPath = stagingToSrcMap.get(stagingMapPath); + await Promise.all(stagedFiles.map(async (stagingFilePath: string) => { + const originalSrcPath = destToSrcMap.get(stagingFilePath); - // If not in fileMappings or location is unchanged, no rewriting needed - if (!originalMapPath || originalMapPath === stagingMapPath) { + // Skip files not in fileMappings (e.g. generated after staging) + if (!originalSrcPath) { return; } - try { - const sourceMap = await fsExtra.readJsonSync(stagingMapPath) as SourceMapPayload; + const ext = path.extname(stagingFilePath).toLowerCase(); - if (!Array.isArray(sourceMap.sources) || sourceMap.sources.length === 0) { - return; - } + if (ext === '.map') { + await this.fixSourceMapSources(stagingFilePath, originalSrcPath); + } else if (ext === '.brs' || ext === '.xml') { + await this.fixSourceMapComment(stagingFilePath, originalSrcPath, srcToDestMap); + } + })); + } - // Resolve sources relative to original map's base dir (honoring sourceRoot if present) - const originalBaseDir = path.resolve( - //sourceRoot should resolve relative to originalMapDir, or keep it as-is if it's an absolute path - path.dirname(originalMapPath), - sourceMap.sourceRoot ?? '' - ); + /** + * Rewrite the `sources` paths in a staged .map file so they are relative to the map's + * new staging location rather than the original source directory. + */ + private async fixSourceMapSources(stagingMapPath: string, originalMapPath: string) { + try { + const sourceMap = await fsExtra.readJsonSync(stagingMapPath) as SourceMapPayload; + if (!Array.isArray(sourceMap.sources) || sourceMap.sources.length === 0) { + return; + } + // Resolve sources relative to original map's base dir (honoring sourceRoot if present) + const originalBaseDir = path.resolve( + //sourceRoot should resolve relative to originalMapDir, or keep as-is when absolute path + path.dirname(originalMapPath), + sourceMap.sourceRoot ?? '' + ); - const stagingMapDir = path.dirname(stagingMapPath); + const stagingMapDir = path.dirname(stagingMapPath); - sourceMap.sources = sourceMap.sources.map((source) => { - const absoluteSourcePath = path.resolve(originalBaseDir, source); - return fileUtils.standardizePath(path.relative(stagingMapDir, absoluteSourcePath)); - }); + sourceMap.sources = sourceMap.sources.map((source) => { + const absoluteSourcePath = path.resolve(originalBaseDir, source); + return fileUtils.standardizePath(path.relative(stagingMapDir, absoluteSourcePath)); + }); - // Clear sourceRoot since sources are now relative to the map file's new location - delete sourceMap.sourceRoot; + // Clear sourceRoot since sources are now relative to the map file's new location + delete sourceMap.sourceRoot; - await fsExtra.writeFile(stagingMapPath, JSON.stringify(sourceMap)); - } catch (e) { - this.logger.error(`Error updating source map sources for '${stagingMapPath}'`, e); + await fsExtra.writeFile(stagingMapPath, JSON.stringify(sourceMap)); + } catch (e) { + this.logger.error(`Error updating source map sources for '${stagingMapPath}'`, e); + } + } + + /** + * Rewrite the sourceMappingURL comment in a staged .brs or .xml file so the path points + * to the map file's new staging location. + * + * BRS format: '//# sourceMappingURL= + * XML format: + */ + private async fixSourceMapComment(stagingFilePath: string, originalSrcPath: string, srcToDestMap: Map) { + try { + let contents = await fsExtra.readFile(stagingFilePath, 'utf8'); + const commentMatch = /('|)?$/m.exec(contents); + const ext = path.extname(originalSrcPath).toLowerCase(); + const isXml = ext === '.xml'; + + let absoluteMapPath: string; + let originalCommentPath: string | undefined; + + if (commentMatch) { + originalCommentPath = commentMatch[2]; + absoluteMapPath = fileUtils.standardizePath( + path.resolve(path.dirname(originalSrcPath), originalCommentPath) + ); + } else { + // No comment — check if a sidecar map exists next to the original source file + const sidecarMapPath = fileUtils.standardizePath(originalSrcPath + '.map'); + if (!await fsExtra.pathExists(sidecarMapPath)) { + return; + } + // If the sidecar was also staged, the debugger will find it automatically — no comment needed + if (srcToDestMap.has(sidecarMapPath)) { + return; + } + absoluteMapPath = sidecarMapPath; } - })); + + // If the map was also staged, point at its new location; otherwise point directly + // at the absolute map path from the original source tree (e.g. map outside rootDir) + const mapTarget = srcToDestMap.get(absoluteMapPath) ?? absoluteMapPath; + const newRelativePath = fileUtils.standardizePath( + path.relative(path.dirname(stagingFilePath), mapTarget) + ); + if (newRelativePath === originalCommentPath) { + return; + } + + if (commentMatch) { + contents = contents.replace( + /('|)?$/m, + (_, open, close) => `${open}//# sourceMappingURL=${newRelativePath}${close ?? ''}` + ); + } else { + // Inject the comment at the end of the file + const comment = isXml + ? `\n` + : `\n'//# sourceMappingURL=${newRelativePath}`; + contents += comment; + } + await fsExtra.writeFile(stagingFilePath, contents, 'utf8'); + } catch (e) { + this.logger.error(`Error updating sourceMappingURL comment in '${stagingFilePath}'`, e); + } } /** @@ -601,10 +679,10 @@ export class Project { return; } try { - let files = await globAsync(`${this.rdbFilesBasePath}/**/*`, { + let files: string[] = await fastGlob(`${this.rdbFilesBasePath}/**/*`, { cwd: './', absolute: false, - follow: true + followSymbolicLinks: true }); for (let filePathAbsolute of files) { const promises = []; From b5dd09e557c75596787820bbc2efaaf655504ce9 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 7 Apr 2026 16:21:01 -0400 Subject: [PATCH 02/14] Fixes --- src/managers/ProjectManager.spec.ts | 118 +++++++++++++++++++--------- src/managers/ProjectManager.ts | 22 +++--- 2 files changed, 95 insertions(+), 45 deletions(-) diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index 30ef4f9d..0a22c268 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -771,22 +771,23 @@ describe('Project', () => { * When the map WAS staged (at a different relative location), the comment * should point at the staged copy, not the original. */ - it('rewrites the comment to point at the staged map when the map was also staged', async () => { - const srcBrsDir = s`${tempPath}/src/components/views`; - const srcMapDir = s`${tempPath}/src/components/maps`; - const stagingSourceDir = s`${stagingDir}/source`; - - fsExtra.ensureDirSync(srcBrsDir); - fsExtra.ensureDirSync(srcMapDir); - fsExtra.ensureDirSync(stagingSourceDir); - - const originalBrsPath = s`${srcBrsDir}/main.brs`; - const originalMapPath = s`${srcMapDir}/main.brs.map`; - const stagingBrsPath = s`${stagingSourceDir}/main.brs`; - const stagingMapPath = s`${stagingSourceDir}/main.brs.map`; - - const originalRelative = s`${path.relative(srcBrsDir, originalMapPath)}`; - fsExtra.writeFileSync(originalBrsPath, `sub main()\nend sub\n'//# sourceMappingURL=${originalRelative}`); + it('rewrites the comment to point at the staged map when the map was also staged (.brs)', async () => { + // Source layout: + // src/components/views/main.brs → comment: '../maps/main.brs.map' + // src/components/maps/main.brs.map + // Staging layout (both siblings in source/): + // staging/source/main.brs → comment should become: 'main.brs.map' + // staging/source/main.brs.map + const originalBrsPath = s`${tempPath}/src/components/views/main.brs`; + const originalMapPath = s`${tempPath}/src/components/maps/main.brs.map`; + const stagingBrsPath = s`${stagingDir}/source/main.brs`; + const stagingMapPath = s`${stagingDir}/source/main.brs.map`; + + fsExtra.ensureDirSync(path.dirname(originalBrsPath)); + fsExtra.ensureDirSync(path.dirname(originalMapPath)); + fsExtra.ensureDirSync(path.dirname(stagingBrsPath)); + + fsExtra.writeFileSync(originalBrsPath, `sub main()\nend sub\n'//# sourceMappingURL=../maps/main.brs.map`); fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); fsExtra.copySync(originalBrsPath, stagingBrsPath); @@ -799,9 +800,7 @@ describe('Project', () => { await project['preprocessStagingFiles'](); - const updatedContents = fsExtra.readFileSync(stagingBrsPath, 'utf8'); - // Both were staged as siblings, so comment should just be the filename - expect(updatedContents).to.contain(`'//# sourceMappingURL=main.brs.map`); + expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(`sub main()\nend sub\n'//# sourceMappingURL=main.brs.map`); }); it('does not rewrite the comment when the relative path is already correct after staging', async () => { @@ -836,21 +835,22 @@ describe('Project', () => { }); it('rewrites the XML format comment ()', async () => { - const srcXmlDir = s`${tempPath}/src/components/views`; - const srcMapDir = s`${tempPath}/src/components/maps`; - const stagingSourceDir = s`${stagingDir}/source`; - - fsExtra.ensureDirSync(srcXmlDir); - fsExtra.ensureDirSync(srcMapDir); - fsExtra.ensureDirSync(stagingSourceDir); - - const originalXmlPath = s`${srcXmlDir}/MainScene.xml`; - const originalMapPath = s`${srcMapDir}/MainScene.xml.map`; - const stagingXmlPath = s`${stagingSourceDir}/MainScene.xml`; - const stagingMapPath = s`${stagingSourceDir}/MainScene.xml.map`; - - const originalRelative = s`${path.relative(srcXmlDir, originalMapPath)}`; - fsExtra.writeFileSync(originalXmlPath, `\n\n`); + // Source layout: + // src/components/views/MainScene.xml → comment: '../maps/MainScene.xml.map' + // src/components/maps/MainScene.xml.map + // Staging layout (both siblings in source/): + // staging/source/MainScene.xml → comment should become: 'MainScene.xml.map' + // staging/source/MainScene.xml.map + const originalXmlPath = s`${tempPath}/src/components/views/MainScene.xml`; + const originalMapPath = s`${tempPath}/src/components/maps/MainScene.xml.map`; + const stagingXmlPath = s`${stagingDir}/source/MainScene.xml`; + const stagingMapPath = s`${stagingDir}/source/MainScene.xml.map`; + + fsExtra.ensureDirSync(path.dirname(originalXmlPath)); + fsExtra.ensureDirSync(path.dirname(originalMapPath)); + fsExtra.ensureDirSync(path.dirname(stagingXmlPath)); + + fsExtra.writeFileSync(originalXmlPath, `\n\n`); fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); fsExtra.copySync(originalXmlPath, stagingXmlPath); @@ -863,8 +863,7 @@ describe('Project', () => { await project['preprocessStagingFiles'](); - const updatedContents = fsExtra.readFileSync(stagingXmlPath, 'utf8'); - expect(updatedContents).to.contain(``); + expect(fsExtra.readFileSync(stagingXmlPath, 'utf8')).to.equal(`\n\n`); }); it('injects a comment when there is none but a sidecar .map exists next to the original source', async () => { @@ -933,6 +932,53 @@ describe('Project', () => { expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); }); + it('rewrites the comment in an arbitrary text-based file format', async () => { + // Source layout: + // src/components/views/main.md → comment: '../maps/main.md.map' + // src/components/maps/main.md.map + // Staging layout (both siblings in source/): + // staging/source/main.md → comment should become: 'main.md.map' + // staging/source/main.md.map + const originalFilePath = s`${tempPath}/src/components/views/main.md`; + const originalMapPath = s`${tempPath}/src/components/maps/main.md.map`; + const stagingFilePath = s`${stagingDir}/source/main.md`; + const stagingMapPath = s`${stagingDir}/source/main.md.map`; + + fsExtra.outputFileSync(originalFilePath, `# hello\n//# sourceMappingURL=../maps/main.md.map`); + fsExtra.outputJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); + + fsExtra.copySync(originalFilePath, stagingFilePath); + fsExtra.copySync(originalMapPath, stagingMapPath); + + project.fileMappings = [ + { src: originalFilePath, dest: stagingFilePath }, + { src: originalMapPath, dest: stagingMapPath } + ]; + + await project['preprocessStagingFiles'](); + + expect(fsExtra.readFileSync(stagingFilePath, 'utf8')).to.equal(`# hello\n//# sourceMappingURL=main.md.map`); + }); + + it('does not rewrite the comment when the path is absolute', async () => { + const originalBrsPath = s`${tempPath}/src/source/main.brs`; + const stagingBrsPath = s`${stagingDir}/source/main.brs`; + + fsExtra.ensureDirSync(path.dirname(originalBrsPath)); + fsExtra.ensureDirSync(path.dirname(stagingBrsPath)); + + const absoluteMapPath = s`${tempPath}/src/source/main.brs.map`; + const originalContents = `sub main()\nend sub\n'//# sourceMappingURL=${absoluteMapPath}`; + fsExtra.writeFileSync(originalBrsPath, originalContents); + fsExtra.copySync(originalBrsPath, stagingBrsPath); + + project.fileMappings = [{ src: originalBrsPath, dest: stagingBrsPath }]; + + await project['preprocessStagingFiles'](); + + expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); + }); + it('does not crash when .brs has no sourceMappingURL comment', async () => { const srcDir = s`${tempPath}/src/source`; const stagingSourceDir = s`${stagingDir}/source`; diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index 472da1fc..3420637f 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -471,7 +471,7 @@ export class Project { if (ext === '.map') { await this.fixSourceMapSources(stagingFilePath, originalSrcPath); - } else if (ext === '.brs' || ext === '.xml') { + } else { await this.fixSourceMapComment(stagingFilePath, originalSrcPath, srcToDestMap); } })); @@ -520,9 +520,7 @@ export class Project { private async fixSourceMapComment(stagingFilePath: string, originalSrcPath: string, srcToDestMap: Map) { try { let contents = await fsExtra.readFile(stagingFilePath, 'utf8'); - const commentMatch = /('|)?$/m.exec(contents); - const ext = path.extname(originalSrcPath).toLowerCase(); - const isXml = ext === '.xml'; + const commentMatch = /('|)?$/m.exec(contents); let absoluteMapPath: string; let originalCommentPath: string | undefined; @@ -557,14 +555,20 @@ export class Project { if (commentMatch) { contents = contents.replace( - /('|)?$/m, - (_, open, close) => `${open}//# sourceMappingURL=${newRelativePath}${close ?? ''}` + /('|)?$/m, + (_, open, close) => `${open ?? ''}//# sourceMappingURL=${newRelativePath}${close ?? ''}` ); } else { // Inject the comment at the end of the file - const comment = isXml - ? `\n` - : `\n'//# sourceMappingURL=${newRelativePath}`; + const ext = path.extname(stagingFilePath).toLowerCase(); + let comment: string; + if (ext === '.xml') { + comment = `\n`; + } else if (ext === '.brs') { + comment = `\n'//# sourceMappingURL=${newRelativePath}`; + } else { + comment = `\n//# sourceMappingURL=${newRelativePath}`; + } contents += comment; } await fsExtra.writeFile(stagingFilePath, contents, 'utf8'); From be7d10ffdca6df90fc79af30d7bd1da726df1d17 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 8 Apr 2026 08:03:46 -0400 Subject: [PATCH 03/14] Some small fixes --- src/managers/ProjectManager.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index 3420637f..69025109 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -356,7 +356,7 @@ export class Project { for (let fileMapping of this.fileMappings) { relativeFileMappings.push({ src: fileMapping.src, - dest: fileUtils.replaceCaseInsensitive(fileMapping.dest, this.stagingDir, '') + dest: fileUtils.replaceCaseInsensitive(fileMapping.dest, this.stagingDir, '').replace(/^[/\\]+/, '') }); } return Promise.resolve(relativeFileMappings); @@ -467,6 +467,11 @@ export class Project { return; } + // Skip files that were not moved (src === dest) — no paths need rewriting + if (originalSrcPath === stagingFilePath) { + return; + } + const ext = path.extname(stagingFilePath).toLowerCase(); if (ext === '.map') { @@ -543,6 +548,11 @@ export class Project { absoluteMapPath = sidecarMapPath; } + // If the original comment path was absolute, leave it as-is + if (originalCommentPath && path.isAbsolute(originalCommentPath)) { + return; + } + // If the map was also staged, point at its new location; otherwise point directly // at the absolute map path from the original source tree (e.g. map outside rootDir) const mapTarget = srcToDestMap.get(absoluteMapPath) ?? absoluteMapPath; From 96301fe9bef0cb9fa9fd2e11de9cc036f7ba82b2 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 8 Apr 2026 09:01:33 -0400 Subject: [PATCH 04/14] Fix some bugs --- src/managers/ProjectManager.spec.ts | 36 +++++++++++++++-------------- src/managers/ProjectManager.ts | 5 ---- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index 0a22c268..e8e1bd12 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -446,23 +446,6 @@ describe('Project', () => { expect(updated.sourceRoot).to.be.undefined; }); - it('does not modify a map file that was not moved (src === dest)', async () => { - fsExtra.ensureDirSync(stagingDir); - const mapPath = s`${stagingDir}/source/main.brs.map`; - fsExtra.ensureDirSync(path.dirname(mapPath)); - const originalSourceMap = { version: 3, sources: ['../source/main.bs'], mappings: '' }; - fsExtra.writeJsonSync(mapPath, originalSourceMap); - - project.fileMappings = [ - { src: mapPath, dest: mapPath } - ]; - - await project['preprocessStagingFiles'](); - - const unchanged = fsExtra.readJsonSync(mapPath); - expect(unchanged.sources[0]).to.equal('../source/main.bs'); - }); - it('does not modify a map file that is not in fileMappings (generated in staging)', async () => { fsExtra.ensureDirSync(stagingDir); const mapPath = s`${stagingDir}/source/main.brs.map`; @@ -999,6 +982,25 @@ describe('Project', () => { expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); }); + + it('leaves the file untouched when there is no comment and no sidecar map next to the original source', async () => { + const originalBrsPath = s`${tempPath}/src/source/main.brs`; + const stagingBrsPath = s`${stagingDir}/source/main.brs`; + + fsExtra.ensureDirSync(path.dirname(originalBrsPath)); + fsExtra.ensureDirSync(path.dirname(stagingBrsPath)); + + const originalContents = `sub main()\nend sub\n`; + fsExtra.writeFileSync(originalBrsPath, originalContents); + fsExtra.copySync(originalBrsPath, stagingBrsPath); + + // No .map file exists next to the original source + project.fileMappings = [{ src: originalBrsPath, dest: stagingBrsPath }]; + + await project['preprocessStagingFiles'](); + + expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); + }); }); }); diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index 69025109..e0c41b98 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -467,11 +467,6 @@ export class Project { return; } - // Skip files that were not moved (src === dest) — no paths need rewriting - if (originalSrcPath === stagingFilePath) { - return; - } - const ext = path.extname(stagingFilePath).toLowerCase(); if (ext === '.map') { From 5db6193e14fd0f0c079b172ab13b81b2e65b126e Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 8 Apr 2026 09:22:32 -0400 Subject: [PATCH 05/14] More flexible matching --- src/managers/ProjectManager.spec.ts | 91 +++++++++++++++++++++++++++++ src/managers/ProjectManager.ts | 32 ++++++++-- 2 files changed, 118 insertions(+), 5 deletions(-) diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index e8e1bd12..9fbdc9e2 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -849,6 +849,97 @@ describe('Project', () => { expect(fsExtra.readFileSync(stagingXmlPath, 'utf8')).to.equal(`\n\n`); }); + describe('legacy and variant comment forms', () => { + // Helper: set up a brs/xml/md file with a given comment, stage it, run preprocessStagingFiles, + // and return the updated staged file contents. + async function runWithComment(ext: string, commentLine: string) { + const originalPath = s`${tempPath}/src/source/main${ext}`; + const stagingPath = s`${stagingDir}/source/main${ext}`; + const originalMapPath = s`${tempPath}/src/maps/main${ext}.map`; + const stagingMapPath = s`${stagingDir}/source/main${ext}.map`; + + fsExtra.ensureDirSync(path.dirname(originalPath)); + fsExtra.ensureDirSync(path.dirname(originalMapPath)); + fsExtra.ensureDirSync(path.dirname(stagingPath)); + + fsExtra.writeFileSync(originalPath, `content\n${commentLine}`); + fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); + fsExtra.copySync(originalPath, stagingPath); + fsExtra.copySync(originalMapPath, stagingMapPath); + + project.fileMappings = [ + { src: originalPath, dest: stagingPath }, + { src: originalMapPath, dest: stagingMapPath } + ]; + + await project['preprocessStagingFiles'](); + return fsExtra.readFileSync(stagingPath, 'utf8'); + } + + // ── brs variants ────────────────────────────────────────────────────────── + it('brs: rewrites legacy @ form', async () => { + const result = await runWithComment('.brs', `'//@ sourceMappingURL=../maps/main.brs.map`); + expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + + it('brs: rewrites when // is omitted (\'# sourceMappingURL=...)', async () => { + const result = await runWithComment('.brs', `'# sourceMappingURL=../maps/main.brs.map`); + expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + + it('brs: rewrites when // is omitted with legacy @ (\'@ sourceMappingURL=...)', async () => { + const result = await runWithComment('.brs', `'@ sourceMappingURL=../maps/main.brs.map`); + expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + + it('brs: rewrites with whitespace between \' and //# (\' //# sourceMappingURL=...)', async () => { + const result = await runWithComment('.brs', `' //# sourceMappingURL=../maps/main.brs.map`); + expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + + it('brs: rewrites with whitespace and no // (\' # sourceMappingURL=...)', async () => { + const result = await runWithComment('.brs', `' # sourceMappingURL=../maps/main.brs.map`); + expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + + // ── xml variants ────────────────────────────────────────────────────────── + it('xml: rewrites legacy @ form ()', async () => { + const result = await runWithComment('.xml', ``); + expect(result).to.equal(`content\n`); + }); + + it('xml: rewrites when // is omitted ()', async () => { + const result = await runWithComment('.xml', ``); + expect(result).to.equal(`content\n`); + }); + + it('xml: rewrites with whitespace between )', async () => { + const result = await runWithComment('.xml', ``); + expect(result).to.equal(`content\n`); + }); + + it('xml: rewrites with whitespace and no // ()', async () => { + const result = await runWithComment('.xml', ``); + expect(result).to.equal(`content\n`); + }); + + // ── other text-based file variants ───────────────────────────────────────────── + it('other: rewrites legacy @ form (//@ sourceMappingURL=...)', async () => { + const result = await runWithComment('.md', `//@ sourceMappingURL=../maps/main.md.map`); + expect(result).to.equal(`content\n//# sourceMappingURL=main.md.map`); + }); + + it('other: rewrites with whitespace between // and # (// # sourceMappingURL=...)', async () => { + const result = await runWithComment('.md', `// # sourceMappingURL=../maps/main.md.map`); + expect(result).to.equal(`content\n//# sourceMappingURL=main.md.map`); + }); + + it('other: rewrites with whitespace between // and @ (// @ sourceMappingURL=...)', async () => { + const result = await runWithComment('.md', `// @ sourceMappingURL=../maps/main.md.map`); + expect(result).to.equal(`content\n//# sourceMappingURL=main.md.map`); + }); + }); + it('injects a comment when there is none but a sidecar .map exists next to the original source', async () => { // Scenario: user's files array omits map files, so only the .brs is staged. // The .map exists next to the original .brs in rootDir but was never copied. diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index e0c41b98..71134030 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -514,13 +514,25 @@ export class Project { * Rewrite the sourceMappingURL comment in a staged .brs or .xml file so the path points * to the map file's new staging location. * - * BRS format: '//# sourceMappingURL= - * XML format: + * Recognised comment forms (# and legacy @ are both accepted; // is optional for brs/xml): + * BRS: ' [//] [#|@] sourceMappingURL= + * XML: + * other: // \s* [#|@] sourceMappingURL= + * + * When rewriting, the canonical modern form is always written: + * BRS: '//# sourceMappingURL= + * XML: + * other: //# sourceMappingURL= */ private async fixSourceMapComment(stagingFilePath: string, originalSrcPath: string, srcToDestMap: Map) { try { let contents = await fsExtra.readFile(stagingFilePath, 'utf8'); - const commentMatch = /('|)?$/m.exec(contents); + + // Match brs: ' optionally followed by // then [#|@] sourceMappingURL= + // Match xml: + // Match other: // optionally followed by whitespace then [#|@] sourceMappingURL= + //https://regex101.com/r/5Wvsvt/1 + const commentMatch = /('[ \t]*(?:\/\/)?[ \t]*|)?$/m.exec(contents); let absoluteMapPath: string; let originalCommentPath: string | undefined; @@ -559,9 +571,19 @@ export class Project { } if (commentMatch) { + // Always rewrite to canonical modern form + const ext = path.extname(stagingFilePath).toLowerCase(); + let canonical: string; + if (ext === '.xml') { + canonical = ``; + } else if (ext === '.brs') { + canonical = `'//# sourceMappingURL=${newRelativePath}`; + } else { + canonical = `//# sourceMappingURL=${newRelativePath}`; + } contents = contents.replace( - /('|)?$/m, - (_, open, close) => `${open ?? ''}//# sourceMappingURL=${newRelativePath}${close ?? ''}` + /('[ \t]*(?:\/\/)?[ \t]*|)?$/m, + canonical ); } else { // Inject the comment at the end of the file From 303dbbf8a7dd049f5e6216ef8fc5b9752ad6fd51 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 8 Apr 2026 09:26:54 -0400 Subject: [PATCH 06/14] Honor existing newline char --- src/managers/ProjectManager.spec.ts | 41 +++++++++++++++++++++++++++++ src/managers/ProjectManager.ts | 7 ++--- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index 9fbdc9e2..2b418868 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -938,6 +938,27 @@ describe('Project', () => { const result = await runWithComment('.md', `// @ sourceMappingURL=../maps/main.md.map`); expect(result).to.equal(`content\n//# sourceMappingURL=main.md.map`); }); + + // ── no space between #/@ and sourceMappingURL ───────────────────────── + it('brs: matches when there is no space between # and sourceMappingURL', async () => { + const result = await runWithComment('.brs', `'//# sourceMappingURL=../maps/main.brs.map`); + expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + + it('brs: matches when there is no space between @ and sourceMappingURL (legacy)', async () => { + const result = await runWithComment('.brs', `'//@ sourceMappingURL=../maps/main.brs.map`); + expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + + it('xml: matches when there is no space between # and sourceMappingURL', async () => { + const result = await runWithComment('.xml', ``); + expect(result).to.equal(`content\n`); + }); + + it('other: matches when there is no space between # and sourceMappingURL', async () => { + const result = await runWithComment('.md', `//#sourceMappingURL=../maps/main.md.map`); + expect(result).to.equal(`content\n//# sourceMappingURL=main.md.map`); + }); }); it('injects a comment when there is none but a sidecar .map exists next to the original source', async () => { @@ -976,6 +997,26 @@ describe('Project', () => { expect(resolvedMapPath).to.equal(originalMapPath); }); + it('uses CRLF when injecting a comment into a CRLF file', async () => { + const originalBrsPath = s`${tempPath}/src/source/main.brs`; + const originalMapPath = s`${tempPath}/src/source/main.brs.map`; + const stagingBrsPath = s`${stagingDir}/source/main.brs`; + + fsExtra.ensureDirSync(path.dirname(originalBrsPath)); + fsExtra.ensureDirSync(path.dirname(stagingBrsPath)); + + fsExtra.writeFileSync(originalBrsPath, `sub main()\r\nend sub`); + fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); + fsExtra.copySync(originalBrsPath, stagingBrsPath); + + project.fileMappings = [{ src: originalBrsPath, dest: stagingBrsPath }]; + + await project['preprocessStagingFiles'](); + + const updatedContents = fsExtra.readFileSync(stagingBrsPath, 'utf8'); + expect(updatedContents).to.match(/\r\n'\/\/# sourceMappingURL=/); + }); + it('does not inject a comment when there is none but the sidecar .map was staged alongside the .brs', async () => { const srcDir = s`${tempPath}/rootDir/source`; const stagingSourceDir = s`${stagingDir}/source`; diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index 71134030..c99e4ff5 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -527,6 +527,7 @@ export class Project { private async fixSourceMapComment(stagingFilePath: string, originalSrcPath: string, srcToDestMap: Map) { try { let contents = await fsExtra.readFile(stagingFilePath, 'utf8'); + const newline = /\r?\n/.exec(contents)?.[0] ?? '\n'; // Match brs: ' optionally followed by // then [#|@] sourceMappingURL= // Match xml: @@ -590,11 +591,11 @@ export class Project { const ext = path.extname(stagingFilePath).toLowerCase(); let comment: string; if (ext === '.xml') { - comment = `\n`; + comment = `${newline}`; } else if (ext === '.brs') { - comment = `\n'//# sourceMappingURL=${newRelativePath}`; + comment = `${newline}'//# sourceMappingURL=${newRelativePath}`; } else { - comment = `\n//# sourceMappingURL=${newRelativePath}`; + comment = `${newline}//# sourceMappingURL=${newRelativePath}`; } contents += comment; } From 6895723b27951ac7b9f65d92772bf0d0ae96e9e0 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 8 Apr 2026 09:51:06 -0400 Subject: [PATCH 07/14] Simplify tests for sourcemap comments --- src/managers/ProjectManager.spec.ts | 517 +++++++++------------------- 1 file changed, 165 insertions(+), 352 deletions(-) diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index 2b418868..65146612 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -703,411 +703,167 @@ describe('Project', () => { describe('fixSourceMapComment', () => { /** - * Primary scenario: .map is OUTSIDE rootDir so it was never staged. + * Stage a source file (with a sourceMappingURL comment) and its map, run + * preprocessStagingFiles, and return the updated staged file contents. * - * /alpha/beta/charlie/rootDir/source/main.brs → comment: '../../../../../maps/main.brs.map' - * /alpha/maps/main.brs.map (not copied — outside rootDir) - * - * After staging: - * staging/source/main.brs → comment must be rewritten to an absolute-equivalent - * relative path from the new staging location back to - * the original (unstaged) map file. + * originalDir/main has the comment pointing at originalMapDir/main.map + * Both are staged to stagingDir/source/ */ - it('rewrites the comment to point at the original map when the map was not staged', async () => { - const rootDirSource = s`${tempPath}/alpha/beta/charlie/rootDir/source`; - const mapsDir = s`${tempPath}/alpha/maps`; - const stagingSourceDir = s`${stagingDir}/source`; - - fsExtra.ensureDirSync(rootDirSource); - fsExtra.ensureDirSync(mapsDir); - fsExtra.ensureDirSync(stagingSourceDir); - - const originalBrsPath = s`${rootDirSource}/main.brs`; - const originalMapPath = s`${mapsDir}/main.brs.map`; - const stagingBrsPath = s`${stagingSourceDir}/main.brs`; - - // The comment in the original file points relatively from rootDirSource → mapsDir - const originalRelative = s`${path.relative(rootDirSource, originalMapPath)}`; - fsExtra.writeFileSync(originalBrsPath, `sub main()\nend sub\n'//# sourceMappingURL=${originalRelative}`); - - // Only the .brs is staged — the .map is outside rootDir and never copied - fsExtra.copySync(originalBrsPath, stagingBrsPath); - - project.fileMappings = [ - { src: originalBrsPath, dest: stagingBrsPath } - // map intentionally absent - ]; - - await project['preprocessStagingFiles'](); - - const updatedContents = fsExtra.readFileSync(stagingBrsPath, 'utf8'); - // The new comment must resolve back to the same absolute map path - const commentMatch = /'\/\/# sourceMappingURL=(.+)$/.exec(updatedContents); - expect(commentMatch, 'sourceMappingURL comment should still be present').to.exist; - const resolvedMapPath = fileUtils.standardizePath( - path.resolve(path.dirname(stagingBrsPath), commentMatch[1]) - ); - expect(resolvedMapPath).to.equal(originalMapPath); - }); - - /** - * When the map WAS staged (at a different relative location), the comment - * should point at the staged copy, not the original. - */ - it('rewrites the comment to point at the staged map when the map was also staged (.brs)', async () => { - // Source layout: - // src/components/views/main.brs → comment: '../maps/main.brs.map' - // src/components/maps/main.brs.map - // Staging layout (both siblings in source/): - // staging/source/main.brs → comment should become: 'main.brs.map' - // staging/source/main.brs.map - const originalBrsPath = s`${tempPath}/src/components/views/main.brs`; - const originalMapPath = s`${tempPath}/src/components/maps/main.brs.map`; - const stagingBrsPath = s`${stagingDir}/source/main.brs`; - const stagingMapPath = s`${stagingDir}/source/main.brs.map`; - - fsExtra.ensureDirSync(path.dirname(originalBrsPath)); + async function stageFileWithComment(ext: string, commentLine: string, opts: { + originalDir?: string; + originalMapDir?: string; + stageMap?: boolean; + } = {}) { + const { + originalDir = s`${tempPath}/src/components/views`, + originalMapDir = s`${tempPath}/src/components/maps`, + stageMap = true + } = opts; + const originalPath = s`${originalDir}/main${ext}`; + const originalMapPath = s`${originalMapDir}/main${ext}.map`; + const stagingPath = s`${stagingDir}/source/main${ext}`; + const stagingMapPath = s`${stagingDir}/source/main${ext}.map`; + + fsExtra.ensureDirSync(path.dirname(originalPath)); fsExtra.ensureDirSync(path.dirname(originalMapPath)); - fsExtra.ensureDirSync(path.dirname(stagingBrsPath)); + fsExtra.ensureDirSync(path.dirname(stagingPath)); - fsExtra.writeFileSync(originalBrsPath, `sub main()\nend sub\n'//# sourceMappingURL=../maps/main.brs.map`); + fsExtra.writeFileSync(originalPath, `content\n${commentLine}`); fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); + fsExtra.copySync(originalPath, stagingPath); - fsExtra.copySync(originalBrsPath, stagingBrsPath); - fsExtra.copySync(originalMapPath, stagingMapPath); - - project.fileMappings = [ - { src: originalBrsPath, dest: stagingBrsPath }, - { src: originalMapPath, dest: stagingMapPath } - ]; + if (stageMap) { + fsExtra.copySync(originalMapPath, stagingMapPath); + project.fileMappings = [ + { src: originalPath, dest: stagingPath }, + { src: originalMapPath, dest: stagingMapPath } + ]; + } else { + project.fileMappings = [{ src: originalPath, dest: stagingPath }]; + } await project['preprocessStagingFiles'](); + return fsExtra.readFileSync(stagingPath, 'utf8'); + } - expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(`sub main()\nend sub\n'//# sourceMappingURL=main.brs.map`); - }); - - it('does not rewrite the comment when the relative path is already correct after staging', async () => { - // .brs and .map are siblings in both source and staging — no change needed - const srcDir = s`${tempPath}/src/source`; - const stagingSourceDir = s`${stagingDir}/source`; + /** + * Stage a source file with NO sourceMappingURL comment but with a sidecar .map + * next to the original, then run preprocessStagingFiles and return the staged + * file contents. + */ + async function stageFileWithSidecar(ext: string, opts: { stageMap?: boolean; crlf?: boolean } = {}) { + const { stageMap = false, crlf = false } = opts; + const srcDir = s`${tempPath}/rootDir/source`; + const originalPath = s`${srcDir}/main${ext}`; + const originalMapPath = s`${srcDir}/main${ext}.map`; + const stagingPath = s`${stagingDir}/source/main${ext}`; + const stagingMapPath = s`${stagingDir}/source/main${ext}.map`; fsExtra.ensureDirSync(srcDir); - fsExtra.ensureDirSync(stagingSourceDir); - - const originalBrsPath = s`${srcDir}/main.brs`; - const originalMapPath = s`${srcDir}/main.brs.map`; - const stagingBrsPath = s`${stagingSourceDir}/main.brs`; - const stagingMapPath = s`${stagingSourceDir}/main.brs.map`; - - const originalContents = `sub main()\nend sub\n'//# sourceMappingURL=main.brs.map`; - fsExtra.writeFileSync(originalBrsPath, originalContents); - fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); - - fsExtra.copySync(originalBrsPath, stagingBrsPath); - fsExtra.copySync(originalMapPath, stagingMapPath); - - project.fileMappings = [ - { src: originalBrsPath, dest: stagingBrsPath }, - { src: originalMapPath, dest: stagingMapPath } - ]; - - await project['preprocessStagingFiles'](); - - // File should be unchanged (no write needed) - expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); - }); - - it('rewrites the XML format comment ()', async () => { - // Source layout: - // src/components/views/MainScene.xml → comment: '../maps/MainScene.xml.map' - // src/components/maps/MainScene.xml.map - // Staging layout (both siblings in source/): - // staging/source/MainScene.xml → comment should become: 'MainScene.xml.map' - // staging/source/MainScene.xml.map - const originalXmlPath = s`${tempPath}/src/components/views/MainScene.xml`; - const originalMapPath = s`${tempPath}/src/components/maps/MainScene.xml.map`; - const stagingXmlPath = s`${stagingDir}/source/MainScene.xml`; - const stagingMapPath = s`${stagingDir}/source/MainScene.xml.map`; - - fsExtra.ensureDirSync(path.dirname(originalXmlPath)); - fsExtra.ensureDirSync(path.dirname(originalMapPath)); - fsExtra.ensureDirSync(path.dirname(stagingXmlPath)); + fsExtra.ensureDirSync(path.dirname(stagingPath)); - fsExtra.writeFileSync(originalXmlPath, `\n\n`); + fsExtra.writeFileSync(originalPath, crlf ? `sub main()\r\nend sub` : `sub main()\nend sub`); fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); + fsExtra.copySync(originalPath, stagingPath); - fsExtra.copySync(originalXmlPath, stagingXmlPath); - fsExtra.copySync(originalMapPath, stagingMapPath); - - project.fileMappings = [ - { src: originalXmlPath, dest: stagingXmlPath }, - { src: originalMapPath, dest: stagingMapPath } - ]; - - await project['preprocessStagingFiles'](); - - expect(fsExtra.readFileSync(stagingXmlPath, 'utf8')).to.equal(`\n\n`); - }); - - describe('legacy and variant comment forms', () => { - // Helper: set up a brs/xml/md file with a given comment, stage it, run preprocessStagingFiles, - // and return the updated staged file contents. - async function runWithComment(ext: string, commentLine: string) { - const originalPath = s`${tempPath}/src/source/main${ext}`; - const stagingPath = s`${stagingDir}/source/main${ext}`; - const originalMapPath = s`${tempPath}/src/maps/main${ext}.map`; - const stagingMapPath = s`${stagingDir}/source/main${ext}.map`; - - fsExtra.ensureDirSync(path.dirname(originalPath)); - fsExtra.ensureDirSync(path.dirname(originalMapPath)); - fsExtra.ensureDirSync(path.dirname(stagingPath)); - - fsExtra.writeFileSync(originalPath, `content\n${commentLine}`); - fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); - fsExtra.copySync(originalPath, stagingPath); + if (stageMap) { fsExtra.copySync(originalMapPath, stagingMapPath); - project.fileMappings = [ { src: originalPath, dest: stagingPath }, { src: originalMapPath, dest: stagingMapPath } ]; - - await project['preprocessStagingFiles'](); - return fsExtra.readFileSync(stagingPath, 'utf8'); + } else { + project.fileMappings = [{ src: originalPath, dest: stagingPath }]; } - // ── brs variants ────────────────────────────────────────────────────────── - it('brs: rewrites legacy @ form', async () => { - const result = await runWithComment('.brs', `'//@ sourceMappingURL=../maps/main.brs.map`); - expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); - }); - - it('brs: rewrites when // is omitted (\'# sourceMappingURL=...)', async () => { - const result = await runWithComment('.brs', `'# sourceMappingURL=../maps/main.brs.map`); - expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); - }); - - it('brs: rewrites when // is omitted with legacy @ (\'@ sourceMappingURL=...)', async () => { - const result = await runWithComment('.brs', `'@ sourceMappingURL=../maps/main.brs.map`); - expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); - }); - - it('brs: rewrites with whitespace between \' and //# (\' //# sourceMappingURL=...)', async () => { - const result = await runWithComment('.brs', `' //# sourceMappingURL=../maps/main.brs.map`); - expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); - }); - - it('brs: rewrites with whitespace and no // (\' # sourceMappingURL=...)', async () => { - const result = await runWithComment('.brs', `' # sourceMappingURL=../maps/main.brs.map`); - expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); - }); - - // ── xml variants ────────────────────────────────────────────────────────── - it('xml: rewrites legacy @ form ()', async () => { - const result = await runWithComment('.xml', ``); - expect(result).to.equal(`content\n`); - }); - - it('xml: rewrites when // is omitted ()', async () => { - const result = await runWithComment('.xml', ``); - expect(result).to.equal(`content\n`); - }); - - it('xml: rewrites with whitespace between )', async () => { - const result = await runWithComment('.xml', ``); - expect(result).to.equal(`content\n`); - }); - - it('xml: rewrites with whitespace and no // ()', async () => { - const result = await runWithComment('.xml', ``); - expect(result).to.equal(`content\n`); - }); + await project['preprocessStagingFiles'](); + return fsExtra.readFileSync(stagingPath, 'utf8'); + } - // ── other text-based file variants ───────────────────────────────────────────── - it('other: rewrites legacy @ form (//@ sourceMappingURL=...)', async () => { - const result = await runWithComment('.md', `//@ sourceMappingURL=../maps/main.md.map`); - expect(result).to.equal(`content\n//# sourceMappingURL=main.md.map`); - }); + // ── map not staged (outside rootDir) ────────────────────────────────────── + // The comment must be rewritten so it still resolves to the original map. + it('rewrites the comment to point at the original map when the map was not staged', async () => { + const rootDirSource = s`${tempPath}/alpha/beta/charlie/rootDir/source`; + const originalMapPath = s`${tempPath}/alpha/maps/main.brs.map`; + const stagingBrsPath = s`${stagingDir}/source/main.brs`; - it('other: rewrites with whitespace between // and # (// # sourceMappingURL=...)', async () => { - const result = await runWithComment('.md', `// # sourceMappingURL=../maps/main.md.map`); - expect(result).to.equal(`content\n//# sourceMappingURL=main.md.map`); + const originalRelative = s`${path.relative(rootDirSource, originalMapPath)}`; + const result = await stageFileWithComment('.brs', `'//# sourceMappingURL=${originalRelative}`, { + originalDir: rootDirSource, + originalMapDir: s`${tempPath}/alpha/maps`, + stageMap: false }); - it('other: rewrites with whitespace between // and @ (// @ sourceMappingURL=...)', async () => { - const result = await runWithComment('.md', `// @ sourceMappingURL=../maps/main.md.map`); - expect(result).to.equal(`content\n//# sourceMappingURL=main.md.map`); - }); + const commentMatch = /'\/\/# sourceMappingURL=(.+)$/.exec(result); + expect(commentMatch, 'sourceMappingURL comment should still be present').to.exist; + expect(fileUtils.standardizePath(path.resolve(path.dirname(stagingBrsPath), commentMatch[1]))).to.equal(originalMapPath); + }); - // ── no space between #/@ and sourceMappingURL ───────────────────────── - it('brs: matches when there is no space between # and sourceMappingURL', async () => { - const result = await runWithComment('.brs', `'//# sourceMappingURL=../maps/main.brs.map`); - expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); - }); + // ── map staged at a different relative location ─────────────────────────── + it('rewrites the comment to point at the staged map when the map was also staged (.brs)', async () => { + const result = await stageFileWithComment('.brs', `'//# sourceMappingURL=../maps/main.brs.map`); + expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); - it('brs: matches when there is no space between @ and sourceMappingURL (legacy)', async () => { - const result = await runWithComment('.brs', `'//@ sourceMappingURL=../maps/main.brs.map`); - expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); - }); + it('rewrites the XML format comment ()', async () => { + const result = await stageFileWithComment('.xml', ``); + expect(result).to.equal(`content\n`); + }); - it('xml: matches when there is no space between # and sourceMappingURL', async () => { - const result = await runWithComment('.xml', ``); - expect(result).to.equal(`content\n`); - }); + it('rewrites the comment in an arbitrary text-based file format', async () => { + const result = await stageFileWithComment('.md', `//# sourceMappingURL=../maps/main.md.map`); + expect(result).to.equal(`content\n//# sourceMappingURL=main.md.map`); + }); - it('other: matches when there is no space between # and sourceMappingURL', async () => { - const result = await runWithComment('.md', `//#sourceMappingURL=../maps/main.md.map`); - expect(result).to.equal(`content\n//# sourceMappingURL=main.md.map`); + it('does not rewrite the comment when the relative path is already correct after staging', async () => { + // brs and map are siblings in both source and staging — same relative path, no change needed + const result = await stageFileWithComment('.brs', `'//# sourceMappingURL=main.brs.map`, { + originalDir: s`${tempPath}/src/source`, + originalMapDir: s`${tempPath}/src/source` }); + expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); }); - it('injects a comment when there is none but a sidecar .map exists next to the original source', async () => { - // Scenario: user's files array omits map files, so only the .brs is staged. - // The .map exists next to the original .brs in rootDir but was never copied. - // We should inject a comment in the staged .brs pointing back at the original map. - const srcDir = s`${tempPath}/rootDir/source`; - const stagingSourceDir = s`${stagingDir}/source`; - - fsExtra.ensureDirSync(srcDir); - fsExtra.ensureDirSync(stagingSourceDir); - - const originalBrsPath = s`${srcDir}/main.brs`; - const originalMapPath = s`${srcDir}/main.brs.map`; // sidecar, never staged - const stagingBrsPath = s`${stagingSourceDir}/main.brs`; - - fsExtra.writeFileSync(originalBrsPath, `sub main()\nend sub`); - fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); - - // Only the .brs is staged — map was excluded from files array - fsExtra.copySync(originalBrsPath, stagingBrsPath); - - project.fileMappings = [ - { src: originalBrsPath, dest: stagingBrsPath } - ]; - - await project['preprocessStagingFiles'](); - - const updatedContents = fsExtra.readFileSync(stagingBrsPath, 'utf8'); - const commentMatch = /'\/\/# sourceMappingURL=(.+)$/.exec(updatedContents); - expect(commentMatch, 'sourceMappingURL comment should have been injected').to.exist; - // The injected path must resolve back to the original (unstaged) map file - const resolvedMapPath = fileUtils.standardizePath( - path.resolve(path.dirname(stagingBrsPath), commentMatch[1]) - ); - expect(resolvedMapPath).to.equal(originalMapPath); + it('does not rewrite the comment when the path is absolute', async () => { + const absoluteMapPath = s`${tempPath}/src/source/main.brs.map`; + const result = await stageFileWithComment('.brs', `'//# sourceMappingURL=${absoluteMapPath}`, { stageMap: false }); + expect(result).to.equal(`content\n'//# sourceMappingURL=${absoluteMapPath}`); }); - it('uses CRLF when injecting a comment into a CRLF file', async () => { - const originalBrsPath = s`${tempPath}/src/source/main.brs`; - const originalMapPath = s`${tempPath}/src/source/main.brs.map`; + // ── sidecar injection ───────────────────────────────────────────────────── + it('injects a comment when there is none but a sidecar .map exists next to the original source', async () => { const stagingBrsPath = s`${stagingDir}/source/main.brs`; + const originalMapPath = s`${tempPath}/rootDir/source/main.brs.map`; - fsExtra.ensureDirSync(path.dirname(originalBrsPath)); - fsExtra.ensureDirSync(path.dirname(stagingBrsPath)); + const result = await stageFileWithSidecar('.brs'); - fsExtra.writeFileSync(originalBrsPath, `sub main()\r\nend sub`); - fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); - fsExtra.copySync(originalBrsPath, stagingBrsPath); - - project.fileMappings = [{ src: originalBrsPath, dest: stagingBrsPath }]; - - await project['preprocessStagingFiles'](); - - const updatedContents = fsExtra.readFileSync(stagingBrsPath, 'utf8'); - expect(updatedContents).to.match(/\r\n'\/\/# sourceMappingURL=/); + const commentMatch = /'\/\/# sourceMappingURL=(.+)$/.exec(result); + expect(commentMatch, 'sourceMappingURL comment should have been injected').to.exist; + expect(fileUtils.standardizePath(path.resolve(path.dirname(stagingBrsPath), commentMatch[1]))).to.equal(originalMapPath); }); it('does not inject a comment when there is none but the sidecar .map was staged alongside the .brs', async () => { - const srcDir = s`${tempPath}/rootDir/source`; - const stagingSourceDir = s`${stagingDir}/source`; - - fsExtra.ensureDirSync(srcDir); - fsExtra.ensureDirSync(stagingSourceDir); - - const originalBrsPath = s`${srcDir}/main.brs`; - const originalMapPath = s`${srcDir}/main.brs.map`; - const stagingBrsPath = s`${stagingSourceDir}/main.brs`; - const stagingMapPath = s`${stagingSourceDir}/main.brs.map`; - - const originalContents = `sub main()\nend sub`; - fsExtra.writeFileSync(originalBrsPath, originalContents); - fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); - - fsExtra.copySync(originalBrsPath, stagingBrsPath); - fsExtra.copySync(originalMapPath, stagingMapPath); - - project.fileMappings = [ - { src: originalBrsPath, dest: stagingBrsPath }, - { src: originalMapPath, dest: stagingMapPath } - ]; - - await project['preprocessStagingFiles'](); - - // No comment should have been injected — the map is already a sibling in staging - expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); + const original = `sub main()\nend sub`; + const result = await stageFileWithSidecar('.brs', { stageMap: true }); + expect(result).to.equal(original); }); - it('rewrites the comment in an arbitrary text-based file format', async () => { - // Source layout: - // src/components/views/main.md → comment: '../maps/main.md.map' - // src/components/maps/main.md.map - // Staging layout (both siblings in source/): - // staging/source/main.md → comment should become: 'main.md.map' - // staging/source/main.md.map - const originalFilePath = s`${tempPath}/src/components/views/main.md`; - const originalMapPath = s`${tempPath}/src/components/maps/main.md.map`; - const stagingFilePath = s`${stagingDir}/source/main.md`; - const stagingMapPath = s`${stagingDir}/source/main.md.map`; - - fsExtra.outputFileSync(originalFilePath, `# hello\n//# sourceMappingURL=../maps/main.md.map`); - fsExtra.outputJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); - - fsExtra.copySync(originalFilePath, stagingFilePath); - fsExtra.copySync(originalMapPath, stagingMapPath); - - project.fileMappings = [ - { src: originalFilePath, dest: stagingFilePath }, - { src: originalMapPath, dest: stagingMapPath } - ]; - - await project['preprocessStagingFiles'](); - - expect(fsExtra.readFileSync(stagingFilePath, 'utf8')).to.equal(`# hello\n//# sourceMappingURL=main.md.map`); + it('uses CRLF when injecting a comment into a CRLF file', async () => { + const result = await stageFileWithSidecar('.brs', { crlf: true }); + expect(result).to.match(/\r\n'\/\/# sourceMappingURL=/); }); - it('does not rewrite the comment when the path is absolute', async () => { + // ── no comment, no sidecar ──────────────────────────────────────────────── + it('does not crash when .brs has no sourceMappingURL comment', async () => { const originalBrsPath = s`${tempPath}/src/source/main.brs`; const stagingBrsPath = s`${stagingDir}/source/main.brs`; fsExtra.ensureDirSync(path.dirname(originalBrsPath)); fsExtra.ensureDirSync(path.dirname(stagingBrsPath)); - const absoluteMapPath = s`${tempPath}/src/source/main.brs.map`; - const originalContents = `sub main()\nend sub\n'//# sourceMappingURL=${absoluteMapPath}`; - fsExtra.writeFileSync(originalBrsPath, originalContents); - fsExtra.copySync(originalBrsPath, stagingBrsPath); - - project.fileMappings = [{ src: originalBrsPath, dest: stagingBrsPath }]; - - await project['preprocessStagingFiles'](); - - expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); - }); - - it('does not crash when .brs has no sourceMappingURL comment', async () => { - const srcDir = s`${tempPath}/src/source`; - const stagingSourceDir = s`${stagingDir}/source`; - - fsExtra.ensureDirSync(srcDir); - fsExtra.ensureDirSync(stagingSourceDir); - - const originalBrsPath = s`${srcDir}/main.brs`; - const stagingBrsPath = s`${stagingSourceDir}/main.brs`; - const originalContents = `sub main()\nend sub\n`; fsExtra.writeFileSync(originalBrsPath, originalContents); fsExtra.copySync(originalBrsPath, stagingBrsPath); - project.fileMappings = [{ src: originalBrsPath, dest: stagingBrsPath }]; await project['preprocessStagingFiles'](); @@ -1133,6 +889,63 @@ describe('Project', () => { expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); }); + + // ── legacy and variant comment forms ────────────────────────────────────── + describe('legacy and variant comment forms', () => { + // brs variants + it('brs: rewrites legacy @ form', async () => { + expect(await stageFileWithComment('.brs', `'//@ sourceMappingURL=../maps/main.brs.map`)).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + it('brs: rewrites when // is omitted (\'# sourceMappingURL=...)', async () => { + expect(await stageFileWithComment('.brs', `'# sourceMappingURL=../maps/main.brs.map`)).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + it('brs: rewrites when // is omitted with legacy @ (\'@ sourceMappingURL=...)', async () => { + expect(await stageFileWithComment('.brs', `'@ sourceMappingURL=../maps/main.brs.map`)).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + it('brs: rewrites with whitespace between \' and //# (\' //# sourceMappingURL=...)', async () => { + expect(await stageFileWithComment('.brs', `' //# sourceMappingURL=../maps/main.brs.map`)).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + it('brs: rewrites with whitespace and no // (\' # sourceMappingURL=...)', async () => { + expect(await stageFileWithComment('.brs', `' # sourceMappingURL=../maps/main.brs.map`)).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + it('brs: no space between # and sourceMappingURL', async () => { + expect(await stageFileWithComment('.brs', `'//#sourceMappingURL=../maps/main.brs.map`)).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + it('brs: no space between @ and sourceMappingURL (legacy)', async () => { + expect(await stageFileWithComment('.brs', `'//@sourceMappingURL=../maps/main.brs.map`)).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + }); + + // xml variants + it('xml: rewrites legacy @ form ()', async () => { + expect(await stageFileWithComment('.xml', ``)).to.equal(`content\n`); + }); + it('xml: rewrites when // is omitted ()', async () => { + expect(await stageFileWithComment('.xml', ``)).to.equal(`content\n`); + }); + it('xml: rewrites with whitespace between )', async () => { + expect(await stageFileWithComment('.xml', ``)).to.equal(`content\n`); + }); + it('xml: rewrites with whitespace and no // ()', async () => { + expect(await stageFileWithComment('.xml', ``)).to.equal(`content\n`); + }); + it('xml: no space between # and sourceMappingURL', async () => { + expect(await stageFileWithComment('.xml', ``)).to.equal(`content\n`); + }); + + // other (markdown) variants + it('other: rewrites legacy @ form (//@ sourceMappingURL=...)', async () => { + expect(await stageFileWithComment('.md', `//@ sourceMappingURL=../maps/main.md.map`)).to.equal(`content\n//# sourceMappingURL=main.md.map`); + }); + it('other: rewrites with whitespace between // and # (// # sourceMappingURL=...)', async () => { + expect(await stageFileWithComment('.md', `// # sourceMappingURL=../maps/main.md.map`)).to.equal(`content\n//# sourceMappingURL=main.md.map`); + }); + it('other: rewrites with whitespace between // and @ (// @ sourceMappingURL=...)', async () => { + expect(await stageFileWithComment('.md', `// @ sourceMappingURL=../maps/main.md.map`)).to.equal(`content\n//# sourceMappingURL=main.md.map`); + }); + it('other: no space between # and sourceMappingURL', async () => { + expect(await stageFileWithComment('.md', `//#sourceMappingURL=../maps/main.md.map`)).to.equal(`content\n//# sourceMappingURL=main.md.map`); + }); + }); }); }); From c008f66cb1fd95893692cfa66e4b30c9c3ed70a0 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 8 Apr 2026 10:31:49 -0400 Subject: [PATCH 08/14] Filter sourcemap handling for media files --- src/managers/ProjectManager.spec.ts | 22 ++++++++++++++++++++++ src/managers/ProjectManager.ts | 24 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index 65146612..fd9d02b8 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -853,6 +853,28 @@ describe('Project', () => { expect(result).to.match(/\r\n'\/\/# sourceMappingURL=/); }); + it(`skips binary files`, async () => { + // ── binary / media files ────────────────────────────────────────────────── + // Iterate over every extension in Project.binaryExtensions and verify each is skipped. + // We write a fake "binary" payload (non-UTF-8 bytes) and assert the file is untouched. + for (const ext of Project.binaryExtensions) { + const originalPath = s`${tempPath}/src/source/file${ext}`; + const stagingPath = s`${stagingDir}/source/file${ext}`; + + fsExtra.ensureDirSync(path.dirname(originalPath)); + fsExtra.ensureDirSync(path.dirname(stagingPath)); + + const binaryContents = Buffer.from([0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10]); + fsExtra.writeFileSync(originalPath, binaryContents); + fsExtra.copySync(originalPath, stagingPath); + + project.fileMappings = [{ src: originalPath, dest: stagingPath }]; + await project['preprocessStagingFiles'](); + + expect(Buffer.compare(fsExtra.readFileSync(stagingPath), binaryContents)).to.equal(0); + } + }); + // ── no comment, no sidecar ──────────────────────────────────────────────── it('does not crash when .brs has no sourceMappingURL comment', async () => { const originalBrsPath = s`${tempPath}/src/source/main.brs`; diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index c99e4ff5..1e6b9d44 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -356,6 +356,7 @@ export class Project { for (let fileMapping of this.fileMappings) { relativeFileMappings.push({ src: fileMapping.src, + //build a relative path to the dest, and remove any leading slashes dest: fileUtils.replaceCaseInsensitive(fileMapping.dest, this.stagingDir, '').replace(/^[/\\]+/, '') }); } @@ -510,6 +511,26 @@ export class Project { } } + + public static readonly binaryExtensions = new Set([ + // images + '.jpg', '.jpeg', '.png', '.gif', '.bmp', '.webp', '.tiff', '.tif', '.ico', '.svg', + '.heic', '.heif', '.avif', '.raw', '.cr2', '.nef', '.arw', '.dng', + // video + '.mp4', '.mkv', '.mov', '.avi', '.wmv', '.flv', '.webm', '.m4v', '.mpg', '.mpeg', + '.m2v', '.ts', '.mts', '.m2ts', '.vob', '.ogv', '.3gp', '.3g2', + // audio + '.mp3', '.wav', '.aac', '.ogg', '.flac', '.m4a', '.wma', '.opus', '.aiff', '.aif', + // fonts + '.ttf', '.otf', '.woff', '.woff2', '.eot', + // archives / binary containers + '.zip', '.gz', '.tar', '.bz2', '.xz', '.7z', '.rar', '.pkg', '.exe', '.dll', '.so', + // documents / other binary formats + '.pdf', '.psd', '.ai', '.eps', '.indd', + // roku-specific + '.roku', '.rdb', '.squashfs' + ]); + /** * Rewrite the sourceMappingURL comment in a staged .brs or .xml file so the path points * to the map file's new staging location. @@ -526,6 +547,9 @@ export class Project { */ private async fixSourceMapComment(stagingFilePath: string, originalSrcPath: string, srcToDestMap: Map) { try { + if (Project.binaryExtensions.has(path.extname(stagingFilePath).toLowerCase())) { + return; + } let contents = await fsExtra.readFile(stagingFilePath, 'utf8'); const newline = /\r?\n/.exec(contents)?.[0] ?? '\n'; From 55eeecb8a18ce026bf947b72660d2247bd2b20ce Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 8 Apr 2026 10:35:52 -0400 Subject: [PATCH 09/14] Use term "colocate" --- src/managers/ProjectManager.spec.ts | 20 ++++++++++---------- src/managers/ProjectManager.ts | 12 ++++++------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index fd9d02b8..da6ef656 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -747,11 +747,11 @@ describe('Project', () => { } /** - * Stage a source file with NO sourceMappingURL comment but with a sidecar .map + * Stage a source file with NO sourceMappingURL comment but with a colocated .map * next to the original, then run preprocessStagingFiles and return the staged * file contents. */ - async function stageFileWithSidecar(ext: string, opts: { stageMap?: boolean; crlf?: boolean } = {}) { + async function stageFileWithColocatedMap(ext: string, opts: { stageMap?: boolean; crlf?: boolean } = {}) { const { stageMap = false, crlf = false } = opts; const srcDir = s`${tempPath}/rootDir/source`; const originalPath = s`${srcDir}/main${ext}`; @@ -830,26 +830,26 @@ describe('Project', () => { expect(result).to.equal(`content\n'//# sourceMappingURL=${absoluteMapPath}`); }); - // ── sidecar injection ───────────────────────────────────────────────────── - it('injects a comment when there is none but a sidecar .map exists next to the original source', async () => { + // ── colocated injection ───────────────────────────────────────────────────── + it('injects a comment when there is none but a colocated .map exists next to the original source', async () => { const stagingBrsPath = s`${stagingDir}/source/main.brs`; const originalMapPath = s`${tempPath}/rootDir/source/main.brs.map`; - const result = await stageFileWithSidecar('.brs'); + const result = await stageFileWithColocatedMap('.brs'); const commentMatch = /'\/\/# sourceMappingURL=(.+)$/.exec(result); expect(commentMatch, 'sourceMappingURL comment should have been injected').to.exist; expect(fileUtils.standardizePath(path.resolve(path.dirname(stagingBrsPath), commentMatch[1]))).to.equal(originalMapPath); }); - it('does not inject a comment when there is none but the sidecar .map was staged alongside the .brs', async () => { + it('does not inject a comment when there is none but the colocated .map was staged alongside the .brs', async () => { const original = `sub main()\nend sub`; - const result = await stageFileWithSidecar('.brs', { stageMap: true }); + const result = await stageFileWithColocatedMap('.brs', { stageMap: true }); expect(result).to.equal(original); }); it('uses CRLF when injecting a comment into a CRLF file', async () => { - const result = await stageFileWithSidecar('.brs', { crlf: true }); + const result = await stageFileWithColocatedMap('.brs', { crlf: true }); expect(result).to.match(/\r\n'\/\/# sourceMappingURL=/); }); @@ -875,7 +875,7 @@ describe('Project', () => { } }); - // ── no comment, no sidecar ──────────────────────────────────────────────── + // ── no comment, no colocated ──────────────────────────────────────────────── it('does not crash when .brs has no sourceMappingURL comment', async () => { const originalBrsPath = s`${tempPath}/src/source/main.brs`; const stagingBrsPath = s`${stagingDir}/source/main.brs`; @@ -893,7 +893,7 @@ describe('Project', () => { expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); }); - it('leaves the file untouched when there is no comment and no sidecar map next to the original source', async () => { + it('leaves the file untouched when there is no comment and no colocated map next to the original source', async () => { const originalBrsPath = s`${tempPath}/src/source/main.brs`; const stagingBrsPath = s`${stagingDir}/source/main.brs`; diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index 1e6b9d44..69d0e72e 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -568,16 +568,16 @@ export class Project { path.resolve(path.dirname(originalSrcPath), originalCommentPath) ); } else { - // No comment — check if a sidecar map exists next to the original source file - const sidecarMapPath = fileUtils.standardizePath(originalSrcPath + '.map'); - if (!await fsExtra.pathExists(sidecarMapPath)) { + // No comment — check if a colocated map exists next to the original source file + const colocatedMapPath = fileUtils.standardizePath(originalSrcPath + '.map'); + if (!await fsExtra.pathExists(colocatedMapPath)) { return; } - // If the sidecar was also staged, the debugger will find it automatically — no comment needed - if (srcToDestMap.has(sidecarMapPath)) { + // If the colocated was also staged, the debugger will find it automatically — no comment needed + if (srcToDestMap.has(colocatedMapPath)) { return; } - absoluteMapPath = sidecarMapPath; + absoluteMapPath = colocatedMapPath; } // If the original comment path was absolute, leave it as-is From f6870b49d35ea18378e9b9923d2520c3c407a3d5 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 8 Apr 2026 11:06:53 -0400 Subject: [PATCH 10/14] Always replace, even if same --- src/managers/ProjectManager.spec.ts | 11 +++--- src/managers/ProjectManager.ts | 53 ++++++++++------------------- 2 files changed, 25 insertions(+), 39 deletions(-) diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index da6ef656..df1e9685 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -815,8 +815,7 @@ describe('Project', () => { expect(result).to.equal(`content\n//# sourceMappingURL=main.md.map`); }); - it('does not rewrite the comment when the relative path is already correct after staging', async () => { - // brs and map are siblings in both source and staging — same relative path, no change needed + it('keeps the correct path when brs and map are siblings in both source and staging', async () => { const result = await stageFileWithComment('.brs', `'//# sourceMappingURL=main.brs.map`, { originalDir: s`${tempPath}/src/source`, originalMapDir: s`${tempPath}/src/source` @@ -824,10 +823,14 @@ describe('Project', () => { expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); }); - it('does not rewrite the comment when the path is absolute', async () => { + it('rewrites an absolute comment path to a relative path', async () => { + // The map is at tempPath/src/source/main.brs.map — not staged, so the comment + // should point back at it relatively from the staging location. const absoluteMapPath = s`${tempPath}/src/source/main.brs.map`; + const stagingBrsPath = s`${stagingDir}/source/main.brs`; const result = await stageFileWithComment('.brs', `'//# sourceMappingURL=${absoluteMapPath}`, { stageMap: false }); - expect(result).to.equal(`content\n'//# sourceMappingURL=${absoluteMapPath}`); + const expectedRelative = fileUtils.standardizePath(path.relative(path.dirname(stagingBrsPath), absoluteMapPath)); + expect(result).to.equal(`content\n'//# sourceMappingURL=${expectedRelative}`); }); // ── colocated injection ───────────────────────────────────────────────────── diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index 69d0e72e..ca5ebaa7 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -559,13 +559,15 @@ export class Project { //https://regex101.com/r/5Wvsvt/1 const commentMatch = /('[ \t]*(?:\/\/)?[ \t]*|)?$/m.exec(contents); - let absoluteMapPath: string; - let originalCommentPath: string | undefined; + const ext = path.extname(stagingFilePath).toLowerCase(); + let absoluteMapPath: string; if (commentMatch) { - originalCommentPath = commentMatch[2]; + const commentPath = commentMatch[2]; absoluteMapPath = fileUtils.standardizePath( - path.resolve(path.dirname(originalSrcPath), originalCommentPath) + path.isAbsolute(commentPath) + ? commentPath + : path.resolve(path.dirname(originalSrcPath), commentPath) ); } else { // No comment — check if a colocated map exists next to the original source file @@ -573,55 +575,36 @@ export class Project { if (!await fsExtra.pathExists(colocatedMapPath)) { return; } - // If the colocated was also staged, the debugger will find it automatically — no comment needed + // If the colocated map was also staged, the debugger will find it automatically — no comment needed if (srcToDestMap.has(colocatedMapPath)) { return; } absoluteMapPath = colocatedMapPath; } - // If the original comment path was absolute, leave it as-is - if (originalCommentPath && path.isAbsolute(originalCommentPath)) { - return; - } - - // If the map was also staged, point at its new location; otherwise point directly - // at the absolute map path from the original source tree (e.g. map outside rootDir) + // If the map was also staged, point at its new location; otherwise point back at the original const mapTarget = srcToDestMap.get(absoluteMapPath) ?? absoluteMapPath; const newRelativePath = fileUtils.standardizePath( path.relative(path.dirname(stagingFilePath), mapTarget) ); - if (newRelativePath === originalCommentPath) { - return; + + // Build the canonical comment for this file type + let canonical: string; + if (ext === '.xml') { + canonical = ``; + } else if (ext === '.brs') { + canonical = `'//# sourceMappingURL=${newRelativePath}`; + } else { + canonical = `//# sourceMappingURL=${newRelativePath}`; } if (commentMatch) { - // Always rewrite to canonical modern form - const ext = path.extname(stagingFilePath).toLowerCase(); - let canonical: string; - if (ext === '.xml') { - canonical = ``; - } else if (ext === '.brs') { - canonical = `'//# sourceMappingURL=${newRelativePath}`; - } else { - canonical = `//# sourceMappingURL=${newRelativePath}`; - } contents = contents.replace( /('[ \t]*(?:\/\/)?[ \t]*|)?$/m, canonical ); } else { - // Inject the comment at the end of the file - const ext = path.extname(stagingFilePath).toLowerCase(); - let comment: string; - if (ext === '.xml') { - comment = `${newline}`; - } else if (ext === '.brs') { - comment = `${newline}'//# sourceMappingURL=${newRelativePath}`; - } else { - comment = `${newline}//# sourceMappingURL=${newRelativePath}`; - } - contents += comment; + contents += `${newline}${canonical}`; } await fsExtra.writeFile(stagingFilePath, contents, 'utf8'); } catch (e) { From f35623b656bf62462bfbce09785879ebe038be43 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 8 Apr 2026 12:21:55 -0400 Subject: [PATCH 11/14] Fix some regex stuff --- src/managers/ProjectManager.spec.ts | 2 +- src/managers/ProjectManager.ts | 45 ++++++++++++++--------------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index df1e9685..48fac347 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -957,7 +957,7 @@ describe('Project', () => { expect(await stageFileWithComment('.xml', ``)).to.equal(`content\n`); }); - // other (markdown) variants + // other (markdown) variants — prefix is normalized (whitespace collapsed, @ → #, // ensured) it('other: rewrites legacy @ form (//@ sourceMappingURL=...)', async () => { expect(await stageFileWithComment('.md', `//@ sourceMappingURL=../maps/main.md.map`)).to.equal(`content\n//# sourceMappingURL=main.md.map`); }); diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index ca5ebaa7..9767a1cf 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -553,17 +553,17 @@ export class Project { let contents = await fsExtra.readFile(stagingFilePath, 'utf8'); const newline = /\r?\n/.exec(contents)?.[0] ?? '\n'; - // Match brs: ' optionally followed by // then [#|@] sourceMappingURL= - // Match xml: - // Match other: // optionally followed by whitespace then [#|@] sourceMappingURL= - //https://regex101.com/r/5Wvsvt/1 - const commentMatch = /('[ \t]*(?:\/\/)?[ \t]*|)?$/m.exec(contents); - + //https://regex101.com/r/FMRJNy/1 + const matches = [ + ...contents.matchAll(/^([ \t]*(?:'|)?/gm) + ]; + // in case there are multiple comments, use the last one since that's what tools typically do + const commentMatch = matches?.[matches?.length - 1]; const ext = path.extname(stagingFilePath).toLowerCase(); let absoluteMapPath: string; if (commentMatch) { - const commentPath = commentMatch[2]; + const commentPath = commentMatch[3]; absoluteMapPath = fileUtils.standardizePath( path.isAbsolute(commentPath) ? commentPath @@ -575,8 +575,8 @@ export class Project { if (!await fsExtra.pathExists(colocatedMapPath)) { return; } - // If the colocated map was also staged, the debugger will find it automatically — no comment needed - if (srcToDestMap.has(colocatedMapPath)) { + // If the colocated map was staged right next to this file, the debugger will find it automatically — no comment needed + if (srcToDestMap.get(colocatedMapPath) === stagingFilePath + '.map') { return; } absoluteMapPath = colocatedMapPath; @@ -588,23 +588,22 @@ export class Project { path.relative(path.dirname(stagingFilePath), mapTarget) ); - // Build the canonical comment for this file type - let canonical: string; - if (ext === '.xml') { - canonical = ``; - } else if (ext === '.brs') { - canonical = `'//# sourceMappingURL=${newRelativePath}`; - } else { - canonical = `//# sourceMappingURL=${newRelativePath}`; + //if we found a comment matching our exact pattern, do a replacement + if (commentMatch) { + const leadingWhitespaceAndCommentChars = commentMatch[1]; + const newComment = `${leadingWhitespaceAndCommentChars.trimEnd()}//# sourceMappingURL=${newRelativePath}`; + contents = contents.replace(commentMatch[0], newComment); + return; } - if (commentMatch) { - contents = contents.replace( - /('[ \t]*(?:\/\/)?[ \t]*|)?$/m, - canonical - ); + //At this point we HAVE a sourcemap and no comment. The sourcemap is NOT located in staging, + //so we need to write a sourcemap comment into the source file to reference the original map location + if (ext === '.brs') { + contents += `${newline}'//# sourceMappingURL=${newRelativePath}`; + } else if (ext === '.xml') { + contents += `${newline}`; } else { - contents += `${newline}${canonical}`; + contents += `${newline}//# sourceMappingURL=${newRelativePath}`; } await fsExtra.writeFile(stagingFilePath, contents, 'utf8'); } catch (e) { From b0a09578ae9222b88313e15765af06ace82f272c Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 8 Apr 2026 13:11:45 -0400 Subject: [PATCH 12/14] Fix logic --- src/managers/ProjectManager.spec.ts | 180 ++++++++++++++++++---------- src/managers/ProjectManager.ts | 19 ++- 2 files changed, 124 insertions(+), 75 deletions(-) diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index 48fac347..2dc364b8 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -706,23 +706,25 @@ describe('Project', () => { * Stage a source file (with a sourceMappingURL comment) and its map, run * preprocessStagingFiles, and return the updated staged file contents. * - * originalDir/main has the comment pointing at originalMapDir/main.map - * Both are staged to stagingDir/source/ + * originalDir/main has the comment pointing at originalMapDir/main.map. + * The source file is always staged to stagingDir/source/main. + * The map is staged to stagingMapDest (default: stagingDir/source/main.map). */ async function stageFileWithComment(ext: string, commentLine: string, opts: { originalDir?: string; originalMapDir?: string; stageMap?: boolean; + stagingMapDest?: string; } = {}) { const { originalDir = s`${tempPath}/src/components/views`, originalMapDir = s`${tempPath}/src/components/maps`, - stageMap = true + stageMap = true, + stagingMapDest = s`${stagingDir}/source/main${ext}.map` } = opts; const originalPath = s`${originalDir}/main${ext}`; const originalMapPath = s`${originalMapDir}/main${ext}.map`; const stagingPath = s`${stagingDir}/source/main${ext}`; - const stagingMapPath = s`${stagingDir}/source/main${ext}.map`; fsExtra.ensureDirSync(path.dirname(originalPath)); fsExtra.ensureDirSync(path.dirname(originalMapPath)); @@ -733,10 +735,11 @@ describe('Project', () => { fsExtra.copySync(originalPath, stagingPath); if (stageMap) { - fsExtra.copySync(originalMapPath, stagingMapPath); + fsExtra.ensureDirSync(path.dirname(stagingMapDest)); + fsExtra.copySync(originalMapPath, stagingMapDest); project.fileMappings = [ { src: originalPath, dest: stagingPath }, - { src: originalMapPath, dest: stagingMapPath } + { src: originalMapPath, dest: stagingMapDest } ]; } else { project.fileMappings = [{ src: originalPath, dest: stagingPath }]; @@ -748,16 +751,25 @@ describe('Project', () => { /** * Stage a source file with NO sourceMappingURL comment but with a colocated .map - * next to the original, then run preprocessStagingFiles and return the staged - * file contents. + * next to the original, run preprocessStagingFiles, and return the staged file contents. + * + * When stageMap is true the map is staged to stagingMapDest + * (default: right next to the source file, i.e. stagingDir/source/main.map). */ - async function stageFileWithColocatedMap(ext: string, opts: { stageMap?: boolean; crlf?: boolean } = {}) { - const { stageMap = false, crlf = false } = opts; + async function stageFileWithColocatedMap(ext: string, opts: { + stageMap?: boolean; + stagingMapDest?: string; + crlf?: boolean; + } = {}) { const srcDir = s`${tempPath}/rootDir/source`; const originalPath = s`${srcDir}/main${ext}`; const originalMapPath = s`${srcDir}/main${ext}.map`; const stagingPath = s`${stagingDir}/source/main${ext}`; - const stagingMapPath = s`${stagingDir}/source/main${ext}.map`; + const { + stageMap = false, + stagingMapDest = s`${stagingDir}/source/main${ext}.map`, + crlf = false + } = opts; fsExtra.ensureDirSync(srcDir); fsExtra.ensureDirSync(path.dirname(stagingPath)); @@ -767,10 +779,11 @@ describe('Project', () => { fsExtra.copySync(originalPath, stagingPath); if (stageMap) { - fsExtra.copySync(originalMapPath, stagingMapPath); + fsExtra.ensureDirSync(path.dirname(stagingMapDest)); + fsExtra.copySync(originalMapPath, stagingMapDest); project.fileMappings = [ { src: originalPath, dest: stagingPath }, - { src: originalMapPath, dest: stagingMapPath } + { src: originalMapPath, dest: stagingMapDest } ]; } else { project.fileMappings = [{ src: originalPath, dest: stagingPath }]; @@ -780,8 +793,7 @@ describe('Project', () => { return fsExtra.readFileSync(stagingPath, 'utf8'); } - // ── map not staged (outside rootDir) ────────────────────────────────────── - // The comment must be rewritten so it still resolves to the original map. + // ── comment rewrite: map not staged ────────────────────────────────────── it('rewrites the comment to point at the original map when the map was not staged', async () => { const rootDirSource = s`${tempPath}/alpha/beta/charlie/rootDir/source`; const originalMapPath = s`${tempPath}/alpha/maps/main.brs.map`; @@ -799,13 +811,13 @@ describe('Project', () => { expect(fileUtils.standardizePath(path.resolve(path.dirname(stagingBrsPath), commentMatch[1]))).to.equal(originalMapPath); }); - // ── map staged at a different relative location ─────────────────────────── - it('rewrites the comment to point at the staged map when the map was also staged (.brs)', async () => { + // ── comment rewrite: map staged ─────────────────────────────────────────── + it('rewrites the brs comment to point at the staged map', async () => { const result = await stageFileWithComment('.brs', `'//# sourceMappingURL=../maps/main.brs.map`); expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); }); - it('rewrites the XML format comment ()', async () => { + it('rewrites the xml comment to point at the staged map', async () => { const result = await stageFileWithComment('.xml', ``); expect(result).to.equal(`content\n`); }); @@ -824,8 +836,6 @@ describe('Project', () => { }); it('rewrites an absolute comment path to a relative path', async () => { - // The map is at tempPath/src/source/main.brs.map — not staged, so the comment - // should point back at it relatively from the staging location. const absoluteMapPath = s`${tempPath}/src/source/main.brs.map`; const stagingBrsPath = s`${stagingDir}/source/main.brs`; const result = await stageFileWithComment('.brs', `'//# sourceMappingURL=${absoluteMapPath}`, { stageMap: false }); @@ -833,8 +843,33 @@ describe('Project', () => { expect(result).to.equal(`content\n'//# sourceMappingURL=${expectedRelative}`); }); - // ── colocated injection ───────────────────────────────────────────────────── - it('injects a comment when there is none but a colocated .map exists next to the original source', async () => { + it('uses the last comment when multiple sourceMappingURL comments exist in one file', async () => { + // Only the last comment should be rewritten; the first should be left as-is. + const originalDir = s`${tempPath}/src/source`; + const originalMapDir = s`${tempPath}/src/source`; + const originalPath = s`${originalDir}/main.brs`; + const originalMapPath = s`${originalMapDir}/main.brs.map`; + const stagingPath = s`${stagingDir}/source/main.brs`; + const stagingMapPath = s`${stagingDir}/source/main.brs.map`; + + fsExtra.ensureDirSync(originalDir); + fsExtra.ensureDirSync(path.dirname(stagingPath)); + fsExtra.writeFileSync(originalPath, `line1\n'//# sourceMappingURL=first.brs.map\nline2\n'//# sourceMappingURL=main.brs.map`); + fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); + fsExtra.copySync(originalPath, stagingPath); + fsExtra.copySync(originalMapPath, stagingMapPath); + project.fileMappings = [ + { src: originalPath, dest: stagingPath }, + { src: originalMapPath, dest: stagingMapPath } + ]; + + await project['preprocessStagingFiles'](); + const result = fsExtra.readFileSync(stagingPath, 'utf8'); + expect(result).to.equal(`line1\n'//# sourceMappingURL=first.brs.map\nline2\n'//# sourceMappingURL=main.brs.map`); + }); + + // ── colocated injection ─────────────────────────────────────────────────── + it('injects a brs comment when there is none but a colocated .map exists next to the original source', async () => { const stagingBrsPath = s`${stagingDir}/source/main.brs`; const originalMapPath = s`${tempPath}/rootDir/source/main.brs.map`; @@ -845,41 +880,55 @@ describe('Project', () => { expect(fileUtils.standardizePath(path.resolve(path.dirname(stagingBrsPath), commentMatch[1]))).to.equal(originalMapPath); }); - it('does not inject a comment when there is none but the colocated .map was staged alongside the .brs', async () => { + it('injects an xml comment when there is none but a colocated .map exists next to the original source', async () => { + const stagingXmlPath = s`${stagingDir}/source/main.xml`; + const originalMapPath = s`${tempPath}/rootDir/source/main.xml.map`; + + const result = await stageFileWithColocatedMap('.xml'); + + const commentMatch = /$/.exec(result); + expect(commentMatch, 'sourceMappingURL comment should have been injected').to.exist; + expect(fileUtils.standardizePath(path.resolve(path.dirname(stagingXmlPath), commentMatch[1]))).to.equal(originalMapPath); + }); + + it('injects a //# comment for other file types when there is none but a colocated .map exists', async () => { + const stagingPath = s`${stagingDir}/source/main.md`; + const originalMapPath = s`${tempPath}/rootDir/source/main.md.map`; + + const result = await stageFileWithColocatedMap('.md'); + + const commentMatch = /\/\/# sourceMappingURL=(.+)$/.exec(result); + expect(commentMatch, 'sourceMappingURL comment should have been injected').to.exist; + expect(fileUtils.standardizePath(path.resolve(path.dirname(stagingPath), commentMatch[1]))).to.equal(originalMapPath); + }); + + it('does not inject a comment when the colocated .map was staged right next to the source file', async () => { const original = `sub main()\nend sub`; const result = await stageFileWithColocatedMap('.brs', { stageMap: true }); expect(result).to.equal(original); }); - it('uses CRLF when injecting a comment into a CRLF file', async () => { - const result = await stageFileWithColocatedMap('.brs', { crlf: true }); - expect(result).to.match(/\r\n'\/\/# sourceMappingURL=/); - }); - - it(`skips binary files`, async () => { - // ── binary / media files ────────────────────────────────────────────────── - // Iterate over every extension in Project.binaryExtensions and verify each is skipped. - // We write a fake "binary" payload (non-UTF-8 bytes) and assert the file is untouched. - for (const ext of Project.binaryExtensions) { - const originalPath = s`${tempPath}/src/source/file${ext}`; - const stagingPath = s`${stagingDir}/source/file${ext}`; + it('injects a comment when the colocated .map was staged but at a different relative location', async () => { + // Map is staged to a different subdirectory, not right next to the source — so the + // debugger cannot find it automatically and we must inject a comment. + const stagingBrsPath = s`${stagingDir}/source/main.brs`; + const mapStagedElsewhere = s`${stagingDir}/maps/main.brs.map`; - fsExtra.ensureDirSync(path.dirname(originalPath)); - fsExtra.ensureDirSync(path.dirname(stagingPath)); + const result = await stageFileWithColocatedMap('.brs', { stageMap: true, stagingMapDest: mapStagedElsewhere }); - const binaryContents = Buffer.from([0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10]); - fsExtra.writeFileSync(originalPath, binaryContents); - fsExtra.copySync(originalPath, stagingPath); - - project.fileMappings = [{ src: originalPath, dest: stagingPath }]; - await project['preprocessStagingFiles'](); + const commentMatch = /'\/\/# sourceMappingURL=(.+)$/.exec(result); + expect(commentMatch, 'sourceMappingURL comment should have been injected').to.exist; + // The injected path should resolve to the map's staged location + expect(fileUtils.standardizePath(path.resolve(path.dirname(stagingBrsPath), commentMatch[1]))).to.equal(mapStagedElsewhere); + }); - expect(Buffer.compare(fsExtra.readFileSync(stagingPath), binaryContents)).to.equal(0); - } + it('uses CRLF when injecting a comment into a CRLF file', async () => { + const result = await stageFileWithColocatedMap('.brs', { crlf: true }); + expect(result).to.match(/\r\n'\/\/# sourceMappingURL=/); }); - // ── no comment, no colocated ──────────────────────────────────────────────── - it('does not crash when .brs has no sourceMappingURL comment', async () => { + // ── no comment, no colocated map ────────────────────────────────────────── + it('leaves the file untouched when there is no comment and no colocated map', async () => { const originalBrsPath = s`${tempPath}/src/source/main.brs`; const stagingBrsPath = s`${stagingDir}/source/main.brs`; @@ -896,23 +945,24 @@ describe('Project', () => { expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); }); - it('leaves the file untouched when there is no comment and no colocated map next to the original source', async () => { - const originalBrsPath = s`${tempPath}/src/source/main.brs`; - const stagingBrsPath = s`${stagingDir}/source/main.brs`; - - fsExtra.ensureDirSync(path.dirname(originalBrsPath)); - fsExtra.ensureDirSync(path.dirname(stagingBrsPath)); + // ── binary files ────────────────────────────────────────────────────────── + it('skips binary files without modifying them', async () => { + for (const ext of Project.binaryExtensions) { + const originalPath = s`${tempPath}/src/source/file${ext}`; + const stagingPath = s`${stagingDir}/source/file${ext}`; - const originalContents = `sub main()\nend sub\n`; - fsExtra.writeFileSync(originalBrsPath, originalContents); - fsExtra.copySync(originalBrsPath, stagingBrsPath); + fsExtra.ensureDirSync(path.dirname(originalPath)); + fsExtra.ensureDirSync(path.dirname(stagingPath)); - // No .map file exists next to the original source - project.fileMappings = [{ src: originalBrsPath, dest: stagingBrsPath }]; + const binaryContents = Buffer.from([0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10]); + fsExtra.writeFileSync(originalPath, binaryContents); + fsExtra.copySync(originalPath, stagingPath); - await project['preprocessStagingFiles'](); + project.fileMappings = [{ src: originalPath, dest: stagingPath }]; + await project['preprocessStagingFiles'](); - expect(fsExtra.readFileSync(stagingBrsPath, 'utf8')).to.equal(originalContents); + expect(Buffer.compare(fsExtra.readFileSync(stagingPath), binaryContents)).to.equal(0, `${ext} file should be untouched`); + } }); // ── legacy and variant comment forms ────────────────────────────────────── @@ -921,16 +971,16 @@ describe('Project', () => { it('brs: rewrites legacy @ form', async () => { expect(await stageFileWithComment('.brs', `'//@ sourceMappingURL=../maps/main.brs.map`)).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); }); - it('brs: rewrites when // is omitted (\'# sourceMappingURL=...)', async () => { + it(`brs: rewrites when // is omitted ('# sourceMappingURL=...)`, async () => { expect(await stageFileWithComment('.brs', `'# sourceMappingURL=../maps/main.brs.map`)).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); }); - it('brs: rewrites when // is omitted with legacy @ (\'@ sourceMappingURL=...)', async () => { + it(`brs: rewrites when // is omitted with legacy @ ('@ sourceMappingURL=...)`, async () => { expect(await stageFileWithComment('.brs', `'@ sourceMappingURL=../maps/main.brs.map`)).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); }); - it('brs: rewrites with whitespace between \' and //# (\' //# sourceMappingURL=...)', async () => { + it(`brs: rewrites with whitespace between ' and //# (' //# sourceMappingURL=...)`, async () => { expect(await stageFileWithComment('.brs', `' //# sourceMappingURL=../maps/main.brs.map`)).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); }); - it('brs: rewrites with whitespace and no // (\' # sourceMappingURL=...)', async () => { + it(`brs: rewrites with whitespace and no // (' # sourceMappingURL=...)`, async () => { expect(await stageFileWithComment('.brs', `' # sourceMappingURL=../maps/main.brs.map`)).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); }); it('brs: no space between # and sourceMappingURL', async () => { @@ -957,7 +1007,7 @@ describe('Project', () => { expect(await stageFileWithComment('.xml', ``)).to.equal(`content\n`); }); - // other (markdown) variants — prefix is normalized (whitespace collapsed, @ → #, // ensured) + // other (markdown) variants it('other: rewrites legacy @ form (//@ sourceMappingURL=...)', async () => { expect(await stageFileWithComment('.md', `//@ sourceMappingURL=../maps/main.md.map`)).to.equal(`content\n//# sourceMappingURL=main.md.map`); }); diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index 9767a1cf..8d679fbe 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -593,17 +593,16 @@ export class Project { const leadingWhitespaceAndCommentChars = commentMatch[1]; const newComment = `${leadingWhitespaceAndCommentChars.trimEnd()}//# sourceMappingURL=${newRelativePath}`; contents = contents.replace(commentMatch[0], newComment); - return; - } - - //At this point we HAVE a sourcemap and no comment. The sourcemap is NOT located in staging, - //so we need to write a sourcemap comment into the source file to reference the original map location - if (ext === '.brs') { - contents += `${newline}'//# sourceMappingURL=${newRelativePath}`; - } else if (ext === '.xml') { - contents += `${newline}`; } else { - contents += `${newline}//# sourceMappingURL=${newRelativePath}`; + //At this point we HAVE a sourcemap and no comment. The sourcemap is NOT located in staging, + //so we need to write a sourcemap comment into the source file to reference the original map location + if (ext === '.brs') { + contents += `${newline}'//# sourceMappingURL=${newRelativePath}`; + } else if (ext === '.xml') { + contents += `${newline}`; + } else { + contents += `${newline}//# sourceMappingURL=${newRelativePath}`; + } } await fsExtra.writeFile(stagingFilePath, contents, 'utf8'); } catch (e) { From 7bd1402034bc85b9e4677d83542386b62dcf54e4 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Thu, 9 Apr 2026 10:58:14 -0400 Subject: [PATCH 13/14] Copy all external maps into staging. --- src/debugSession/BrightScriptDebugSession.ts | 13 ++++ src/managers/ProjectManager.ts | 74 ++++++++++++++++---- src/managers/SourceMapManager.ts | 35 ++++++++- 3 files changed, 106 insertions(+), 16 deletions(-) diff --git a/src/debugSession/BrightScriptDebugSession.ts b/src/debugSession/BrightScriptDebugSession.ts index 33b4b8c9..68a907fd 100644 --- a/src/debugSession/BrightScriptDebugSession.ts +++ b/src/debugSession/BrightScriptDebugSession.ts @@ -291,6 +291,11 @@ export class BrightScriptDebugSession extends LoggingDebugSession { */ private firstRunDeferred = defer(); + /** + * Resolved whenever we're finished copying all the files to staging for all projects + */ + private stagingDefered = defer(); + private evaluateRefIdLookup: Record = {}; private evaluateRefIdCounter = 1; @@ -608,6 +613,10 @@ export class BrightScriptDebugSession extends LoggingDebugSession { this.prepareMainProject(), this.prepareAndHostComponentLibraries(this.launchConfiguration.componentLibraries, this.launchConfiguration.componentLibrariesPort) ]); + + //all of the projects have been successfully staged. + this.stagingDefered.resolve(); + packageEnd(); if (this.enableDebugProtocol) { @@ -1423,6 +1432,9 @@ export class BrightScriptDebugSession extends LoggingDebugSession { }; this.sendResponse(response); + //ensure we've staged all the files + await this.stagingDefered.promise; + await this.rokuAdapter?.syncBreakpoints(); } @@ -2483,6 +2495,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { await this.rokuAdapter.destroy(); await this.ensureAppIsInactive(); this.rokuAdapterDeferred = defer(); + this.stagingDefered = defer(); } await this.launchRequest(response, args.arguments as LaunchConfiguration); } diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index 8d679fbe..319b63ea 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -1,4 +1,3 @@ -import * as assert from 'assert'; import * as fsExtra from 'fs-extra'; import * as path from 'path'; import { rokuDeploy, RokuDeploy, util as rokuDeployUtil } from 'roku-deploy'; @@ -14,7 +13,6 @@ import { BscProjectThreaded } from '../bsc/BscProjectThreaded'; import type { ScopeFunction } from '../bsc/BscProject'; import type { Position } from 'brighterscript'; import type { SourceMapPayload } from 'module'; -import { SourceMap } from 'module'; // eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports const replaceInFile = require('replace-in-file'); @@ -471,7 +469,10 @@ export class Project { const ext = path.extname(stagingFilePath).toLowerCase(); if (ext === '.map') { - await this.fixSourceMapSources(stagingFilePath, originalSrcPath); + await this.fixSourceMapSources({ + stagingMapPath: stagingFilePath, + originalMapPath: originalSrcPath + }); } else { await this.fixSourceMapComment(stagingFilePath, originalSrcPath, srcToDestMap); } @@ -482,7 +483,9 @@ export class Project { * Rewrite the `sources` paths in a staged .map file so they are relative to the map's * new staging location rather than the original source directory. */ - private async fixSourceMapSources(stagingMapPath: string, originalMapPath: string) { + private async fixSourceMapSources(params: { stagingMapPath: string; originalMapPath: string }) { + const { stagingMapPath, originalMapPath } = params; + try { const sourceMap = await fsExtra.readJsonSync(stagingMapPath) as SourceMapPayload; if (!Array.isArray(sourceMap.sources) || sourceMap.sources.length === 0) { @@ -531,6 +534,24 @@ export class Project { '.roku', '.rdb', '.squashfs' ]); + /** + * Extracts the sourceMappingURL comment from the given file contents. + * + * `match[3]` is the path (which may be relative or absolute) + * @param contents + * @returns + */ + public static getSourceMapComment(contents: string): RegExpMatchArray | undefined { + + //https://regex101.com/r/FMRJNy/1 + const matches = [ + ...contents.matchAll(/^([ \t]*(?:'|)?/gm) + ]; + // in case there are multiple comments, use the last one since that's what tools typically do + const commentMatch = matches?.[matches?.length - 1]; + return commentMatch; + } + /** * Rewrite the sourceMappingURL comment in a staged .brs or .xml file so the path points * to the map file's new staging location. @@ -553,12 +574,8 @@ export class Project { let contents = await fsExtra.readFile(stagingFilePath, 'utf8'); const newline = /\r?\n/.exec(contents)?.[0] ?? '\n'; - //https://regex101.com/r/FMRJNy/1 - const matches = [ - ...contents.matchAll(/^([ \t]*(?:'|)?/gm) - ]; - // in case there are multiple comments, use the last one since that's what tools typically do - const commentMatch = matches?.[matches?.length - 1]; + const commentMatch = Project.getSourceMapComment(contents); + const ext = path.extname(stagingFilePath).toLowerCase(); let absoluteMapPath: string; @@ -569,17 +586,32 @@ export class Project { ? commentPath : path.resolve(path.dirname(originalSrcPath), commentPath) ); + + //copy the sourcemap right next to our file in staging + absoluteMapPath = await this.colocateSourceMap({ + absoluteMapPath: absoluteMapPath, + stagingFilePath: stagingFilePath + }); + } else { // No comment — check if a colocated map exists next to the original source file - const colocatedMapPath = fileUtils.standardizePath(originalSrcPath + '.map'); - if (!await fsExtra.pathExists(colocatedMapPath)) { + absoluteMapPath = fileUtils.standardizePath(originalSrcPath + '.map'); + + //there is no colocated map next to the original source file + if (!await fsExtra.pathExists(absoluteMapPath)) { return; } + + //copy the sourcemap right next to our file in staging + absoluteMapPath = await this.colocateSourceMap({ + absoluteMapPath: absoluteMapPath, + stagingFilePath: stagingFilePath + }); + // If the colocated map was staged right next to this file, the debugger will find it automatically — no comment needed - if (srcToDestMap.get(colocatedMapPath) === stagingFilePath + '.map') { + if (srcToDestMap.get(absoluteMapPath) === stagingFilePath + '.map') { return; } - absoluteMapPath = colocatedMapPath; } // If the map was also staged, point at its new location; otherwise point back at the original @@ -610,6 +642,20 @@ export class Project { } } + private async colocateSourceMap(options: { stagingFilePath: string; absoluteMapPath: string }) { + //copy the sourcemap right next to our file + const stagingMapPath = `${options.stagingFilePath}.map`; + await fsExtra.copyFile(options.absoluteMapPath, stagingMapPath); + //delete the original sourcemap so node-debug doesn't use it + await fsExtra.unlink(options.absoluteMapPath); + await this.fixSourceMapSources({ + stagingMapPath: stagingMapPath, + originalMapPath: options.absoluteMapPath + }); + return stagingMapPath; + } + + /** * Apply the bsConst transformations to the manifest file for this project */ diff --git a/src/managers/SourceMapManager.ts b/src/managers/SourceMapManager.ts index 5a80377a..004a394b 100644 --- a/src/managers/SourceMapManager.ts +++ b/src/managers/SourceMapManager.ts @@ -6,6 +6,7 @@ import * as path from 'path'; import type { SourceLocation } from './LocationManager'; import { logger } from '../logging'; import type { MaybePromise } from '../interfaces'; +import { Project } from './ProjectManager'; /** * Unifies access to source files across the whole project @@ -83,6 +84,37 @@ export class SourceMapManager { } } + + /** + * Get the path to the sourcemap for a given file, either from a sourceMappingURL comment or by assuming a co-located .map file. + * + * Returns undefined if no source map is found. + * @param filePath + * @returns + */ + private async getSourceMapPath(filePath: string) { + filePath = s`${filePath}`; + let sourceMapPath = this.sourceMapPathCache.get(filePath); + if (!sourceMapPath) { + //read the file on disk and find the sourceMapURL comment (if available) + let contents = await fsExtra.readFile(filePath, 'utf8'); + const match = Project.getSourceMapComment(contents); + //if we have a comment, use it + if (match) { + sourceMapPath = path.resolve(path.dirname(filePath), match[3] ?? ''); + + //we don't have a comment. Assume a co-located source map with the same name as the file + .map + } else { + sourceMapPath = `${filePath}.map`; + } + + this.sourceMapPathCache.set(filePath, sourceMapPath); + } + return sourceMapPath; + } + private sourceMapPathCache = new Map(); + + /** * Get the source location of a position using a source map. If no source map is found, undefined is returned * @param filePath - the absolute path to the file @@ -90,8 +122,7 @@ export class SourceMapManager { * @param currentColumnIndex - the 0-based column number of the current location. */ public async getOriginalLocation(filePath: string, currentLineNumber: number, currentColumnIndex = 0): Promise { - //look for a source map for this file - let sourceMapPath = `${filePath}.map`; + const sourceMapPath = await this.getSourceMapPath(filePath); //if we have a source map, use it let parsedSourceMap = await this.getSourceMap(sourceMapPath); From 59a67e37fb9c3b6a1b93394543a73a3ddee0b69d Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Thu, 9 Apr 2026 12:33:34 -0400 Subject: [PATCH 14/14] Improve comment rewriting logic --- .../BrightScriptDebugSession.spec.ts | 3 + src/debugSession/BrightScriptDebugSession.ts | 4 +- src/managers/ProjectManager.spec.ts | 236 +++++++++++++++--- src/managers/ProjectManager.ts | 73 +++--- src/managers/SourceMapManager.ts | 26 +- 5 files changed, 254 insertions(+), 88 deletions(-) diff --git a/src/debugSession/BrightScriptDebugSession.spec.ts b/src/debugSession/BrightScriptDebugSession.spec.ts index 7ecd0ab7..a055bb76 100644 --- a/src/debugSession/BrightScriptDebugSession.spec.ts +++ b/src/debugSession/BrightScriptDebugSession.spec.ts @@ -62,6 +62,9 @@ describe('BrightScriptDebugSession', () => { } catch (e) { console.log(e); } + //always resolve the stagingDefered promise right away since most tests don't care about staging and this prevents a lot of unnecessary waiting + session['stagingDefered'].resolve(); + errorSpy = sinon.spy(session.logger, 'error'); //override the error response function and throw an exception so we can fail any tests (session as any).sendErrorResponse = (...args: string[]) => { diff --git a/src/debugSession/BrightScriptDebugSession.ts b/src/debugSession/BrightScriptDebugSession.ts index 68a907fd..d43f1c71 100644 --- a/src/debugSession/BrightScriptDebugSession.ts +++ b/src/debugSession/BrightScriptDebugSession.ts @@ -84,6 +84,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { //give util a reference to this session to assist in logging across the entire module util._debugSession = this; + this.fileManager = new FileManager(); this.sourceMapManager = new SourceMapManager(); this.locationManager = new LocationManager(this.sourceMapManager); @@ -615,7 +616,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { ]); //all of the projects have been successfully staged. - this.stagingDefered.resolve(); + this.stagingDefered.tryResolve(); packageEnd(); @@ -2495,6 +2496,7 @@ export class BrightScriptDebugSession extends LoggingDebugSession { await this.rokuAdapter.destroy(); await this.ensureAppIsInactive(); this.rokuAdapterDeferred = defer(); + this.stagingDefered.tryResolve(); this.stagingDefered = defer(); } await this.launchRequest(response, args.arguments as LaunchConfiguration); diff --git a/src/managers/ProjectManager.spec.ts b/src/managers/ProjectManager.spec.ts index 2dc364b8..7e654259 100644 --- a/src/managers/ProjectManager.spec.ts +++ b/src/managers/ProjectManager.spec.ts @@ -403,6 +403,92 @@ describe('Project', () => { }); }); + describe('getSourceMapComment', () => { + const call = (contents: string) => Project.getSourceMapComment(contents); + + it('returns undefined when no sourceMappingURL comment is present', () => { + expect(call(`sub main()\nend sub`)).to.be.undefined; + expect(call(``)).to.be.undefined; + }); + + it('returns the correct named fields for a standard brs comment', () => { + const result = call(`sub main()\nend sub\n'//# sourceMappingURL=main.brs.map`); + expect(result).to.exist; + expect(result.fullMatch).to.equal(`'//# sourceMappingURL=main.brs.map`); + expect(result.leadingInfo).to.equal(`'`); + expect(result.wholeComment).to.equal(`//# sourceMappingURL=main.brs.map`); + expect(result.mapPath).to.equal(`main.brs.map`); + }); + + it('returns the correct named fields for a standard xml comment', () => { + const result = call(`\n\n`); + expect(result).to.exist; + // fullMatch does not include the trailing ' -->' (it's consumed by the non-capturing (?:|-->) group) + expect(result.fullMatch).to.equal(``); + expect(result?.leadingInfo).to.equal(``); + expect(result?.leadingInfo).to.equal(`$/.exec(result); - expect(commentMatch, 'sourceMappingURL comment should have been injected').to.exist; - expect(fileUtils.standardizePath(path.resolve(path.dirname(stagingXmlPath), commentMatch[1]))).to.equal(originalMapPath); + expect(result).to.equal(originalContent); + expect(fsExtra.pathExistsSync(stagingMapPath), 'map should have been copied next to the staging file').to.be.true; }); - it('injects a //# comment for other file types when there is none but a colocated .map exists', async () => { - const stagingPath = s`${stagingDir}/source/main.md`; - const originalMapPath = s`${tempPath}/rootDir/source/main.md.map`; + it('copies the colocated map next to the staging file for other file types', async () => { + const originalContent = `sub main()\nend sub`; + const stagingMapPath = s`${stagingDir}/source/main.md.map`; const result = await stageFileWithColocatedMap('.md'); - const commentMatch = /\/\/# sourceMappingURL=(.+)$/.exec(result); - expect(commentMatch, 'sourceMappingURL comment should have been injected').to.exist; - expect(fileUtils.standardizePath(path.resolve(path.dirname(stagingPath), commentMatch[1]))).to.equal(originalMapPath); + expect(result).to.equal(originalContent); + expect(fsExtra.pathExistsSync(stagingMapPath), 'map should have been copied next to the staging file').to.be.true; }); - it('does not inject a comment when the colocated .map was staged right next to the source file', async () => { + it('does not modify the file when the colocated .map was already staged right next to the source file', async () => { const original = `sub main()\nend sub`; const result = await stageFileWithColocatedMap('.brs', { stageMap: true }); expect(result).to.equal(original); }); - it('injects a comment when the colocated .map was staged but at a different relative location', async () => { - // Map is staged to a different subdirectory, not right next to the source — so the - // debugger cannot find it automatically and we must inject a comment. + it('does not modify the file when the colocated .map was staged at a different location — colocates the map next to the staging file', async () => { + const originalContent = `sub main()\nend sub`; const stagingBrsPath = s`${stagingDir}/source/main.brs`; + const stagingMapPath = s`${stagingDir}/source/main.brs.map`; const mapStagedElsewhere = s`${stagingDir}/maps/main.brs.map`; const result = await stageFileWithColocatedMap('.brs', { stageMap: true, stagingMapDest: mapStagedElsewhere }); - const commentMatch = /'\/\/# sourceMappingURL=(.+)$/.exec(result); - expect(commentMatch, 'sourceMappingURL comment should have been injected').to.exist; - // The injected path should resolve to the map's staged location - expect(fileUtils.standardizePath(path.resolve(path.dirname(stagingBrsPath), commentMatch[1]))).to.equal(mapStagedElsewhere); - }); - - it('uses CRLF when injecting a comment into a CRLF file', async () => { - const result = await stageFileWithColocatedMap('.brs', { crlf: true }); - expect(result).to.match(/\r\n'\/\/# sourceMappingURL=/); + expect(result).to.equal(originalContent); + expect(fsExtra.pathExistsSync(stagingMapPath), 'map should have been copied next to the staging file').to.be.true; }); // ── no comment, no colocated map ────────────────────────────────────────── @@ -1021,6 +1109,76 @@ describe('Project', () => { expect(await stageFileWithComment('.md', `//#sourceMappingURL=../maps/main.md.map`)).to.equal(`content\n//# sourceMappingURL=main.md.map`); }); }); + + // ── map file lifecycle ──────────────────────────────────────────────────── + it('deletes the original map from its source location when a comment references it', async () => { + const originalMapDir = s`${tempPath}/src/components/maps`; + const originalMapPath = s`${originalMapDir}/main.brs.map`; + + await stageFileWithComment('.brs', `'//# sourceMappingURL=../maps/main.brs.map`, { + originalMapDir: originalMapDir + }); + + expect(fsExtra.pathExistsSync(originalMapPath), 'original map should have been deleted by colocateSourceMap').to.be.false; + }); + + it('deletes the original map from its source location when the map is colocated next to the original source', async () => { + const srcDir = s`${tempPath}/rootDir/source`; + const originalMapPath = s`${srcDir}/main.brs.map`; + + await stageFileWithColocatedMap('.brs'); + + expect(fsExtra.pathExistsSync(originalMapPath), 'original colocated map should have been deleted by colocateSourceMap').to.be.false; + }); + + it('copies the map file to staging and it is valid JSON', async () => { + const mapContent = { version: 3, sources: ['main.brs'], mappings: 'AAAA' }; + const srcDir = s`${tempPath}/rootDir/source`; + const originalPath = s`${srcDir}/main.brs`; + const originalMapPath = s`${srcDir}/main.brs.map`; + const stagingPath = s`${stagingDir}/source/main.brs`; + const stagingMapPath = s`${stagingDir}/source/main.brs.map`; + + fsExtra.ensureDirSync(srcDir); + fsExtra.ensureDirSync(path.dirname(stagingPath)); + fsExtra.writeFileSync(originalPath, `sub main()\nend sub`); + fsExtra.writeJsonSync(originalMapPath, mapContent); + fsExtra.copySync(originalPath, stagingPath); + project.fileMappings = [{ src: originalPath, dest: stagingPath }]; + + await project['preprocessStagingFiles'](); + + expect(fsExtra.pathExistsSync(stagingMapPath), 'map should have been copied to staging').to.be.true; + const copiedMap = fsExtra.readJsonSync(stagingMapPath); + // version is preserved; sources are rewritten by fixSourceMapSources (which is expected) + expect(copiedMap.version).to.equal(mapContent.version); + expect(copiedMap.mappings).to.equal(mapContent.mappings); + }); + + it('rewrites the comment and copies the map even when the map was not listed in fileMappings', async () => { + // The map exists on disk but was not staged through fileMappings — colocateSourceMap + // should still copy it next to the staging file and the comment should point at it. + const originalDir = s`${tempPath}/src/source`; + const originalMapDir = s`${tempPath}/src/source`; + const originalPath = s`${originalDir}/main.brs`; + const originalMapPath = s`${originalMapDir}/main.brs.map`; + const stagingPath = s`${stagingDir}/source/main.brs`; + const stagingMapPath = s`${stagingDir}/source/main.brs.map`; + + fsExtra.ensureDirSync(originalDir); + fsExtra.ensureDirSync(path.dirname(stagingPath)); + fsExtra.writeFileSync(originalPath, `content\n'//# sourceMappingURL=main.brs.map`); + fsExtra.writeJsonSync(originalMapPath, { version: 3, sources: [], mappings: '' }); + fsExtra.copySync(originalPath, stagingPath); + // Only stage the source file, not the map + project.fileMappings = [{ src: originalPath, dest: stagingPath }]; + + await project['preprocessStagingFiles'](); + + const result = fsExtra.readFileSync(stagingPath, 'utf8'); + expect(result).to.equal(`content\n'//# sourceMappingURL=main.brs.map`); + expect(fsExtra.pathExistsSync(stagingMapPath), 'map should have been copied next to the staging file').to.be.true; + }); }); }); diff --git a/src/managers/ProjectManager.ts b/src/managers/ProjectManager.ts index 319b63ea..79e02198 100644 --- a/src/managers/ProjectManager.ts +++ b/src/managers/ProjectManager.ts @@ -541,15 +541,34 @@ export class Project { * @param contents * @returns */ - public static getSourceMapComment(contents: string): RegExpMatchArray | undefined { + public static getSourceMapComment(contents: string) { - //https://regex101.com/r/FMRJNy/1 - const matches = [ + //https://regex101.com/r/FMRJNy/2 + const commentMatch = [ ...contents.matchAll(/^([ \t]*(?:'|)?/gm) - ]; - // in case there are multiple comments, use the last one since that's what tools typically do - const commentMatch = matches?.[matches?.length - 1]; - return commentMatch; + ].pop(); + if (commentMatch) { + return { + /** + * The entire matched comment, including any leading whitespace and comment characters (e.g. `'` or ``; - } else { - contents += `${newline}//# sourceMappingURL=${newRelativePath}`; - } - } + const newComment = `${commentMatch.leadingInfo.trimEnd()}//# sourceMappingURL=${newRelativePath}`; + contents = contents.replace(commentMatch.fullMatch, newComment); await fsExtra.writeFile(stagingFilePath, contents, 'utf8'); } catch (e) { this.logger.error(`Error updating sourceMappingURL comment in '${stagingFilePath}'`, e); diff --git a/src/managers/SourceMapManager.ts b/src/managers/SourceMapManager.ts index 004a394b..ea51b0e6 100644 --- a/src/managers/SourceMapManager.ts +++ b/src/managers/SourceMapManager.ts @@ -84,37 +84,40 @@ export class SourceMapManager { } } - /** * Get the path to the sourcemap for a given file, either from a sourceMappingURL comment or by assuming a co-located .map file. * * Returns undefined if no source map is found. - * @param filePath + * @param stagingFilePath * @returns */ - private async getSourceMapPath(filePath: string) { - filePath = s`${filePath}`; - let sourceMapPath = this.sourceMapPathCache.get(filePath); + private async getSourceMapPath(stagingFilePath: string) { + stagingFilePath = s`${stagingFilePath}`; + let sourceMapPath = this.sourceMapPathCache.get(stagingFilePath); if (!sourceMapPath) { //read the file on disk and find the sourceMapURL comment (if available) - let contents = await fsExtra.readFile(filePath, 'utf8'); - const match = Project.getSourceMapComment(contents); + let contents: string | undefined; + try { + contents = await fsExtra.readFile(stagingFilePath, 'utf8'); + } catch { + // file doesn't exist — fall through to the colocated map assumption + } + const match = contents ? Project.getSourceMapComment(contents) : undefined; //if we have a comment, use it if (match) { - sourceMapPath = path.resolve(path.dirname(filePath), match[3] ?? ''); + sourceMapPath = path.resolve(path.dirname(stagingFilePath), match.mapPath ?? ''); //we don't have a comment. Assume a co-located source map with the same name as the file + .map } else { - sourceMapPath = `${filePath}.map`; + sourceMapPath = `${stagingFilePath}.map`; } - this.sourceMapPathCache.set(filePath, sourceMapPath); + this.sourceMapPathCache.set(stagingFilePath, sourceMapPath); } return sourceMapPath; } private sourceMapPathCache = new Map(); - /** * Get the source location of a position using a source map. If no source map is found, undefined is returned * @param filePath - the absolute path to the file @@ -200,6 +203,7 @@ export class SourceMapManager { locations.push({ lineNumber: position.line, columnIndex: position.column, + //TODO this may be wrong if the sourcemap is not colocated with the generated file filePath: sourceMapPath.replace(/\.map$/g, '') }); }