diff --git a/src/managers/BreakpointManager.ts b/src/managers/BreakpointManager.ts index 9c00ffe8..77bbf58f 100644 --- a/src/managers/BreakpointManager.ts +++ b/src/managers/BreakpointManager.ts @@ -780,8 +780,9 @@ export class BreakpointManager { //map via applySourceMap. This collapses staging→rootDir→src into a single staging→src map, //so the debugger can always trace breakpoints back to the true source file in one hop. if (breakpoints[0].type === 'sourceMap') { - //follow any sourceMappingURL comment to find the existing map (read-only) - const existingMapPath = await this.sourceMapManager.getSourceMapPath(stagingFilePath); + //follow any sourceMappingURL comment to find the existing map (read-only). Reuse the + //fileContents we already loaded above instead of making getSourceMapPath re-read the file. + const existingMapPath = await this.sourceMapManager.getSourceMapPath(stagingFilePath, fileContents); const existingMap = await this.sourceMapManager.getSourceMap(existingMapPath); if (existingMap) { await SourceMapConsumer.with(existingMap, null, (consumer) => { diff --git a/src/managers/SourceMapManager.spec.ts b/src/managers/SourceMapManager.spec.ts index 0e98ca0c..a8aa6dd1 100644 --- a/src/managers/SourceMapManager.spec.ts +++ b/src/managers/SourceMapManager.spec.ts @@ -1,10 +1,12 @@ import { expect } from 'chai'; import * as fsExtra from 'fs-extra'; import * as path from 'path'; +import * as sinonActual from 'sinon'; import { standardizePath as s } from '../FileUtils'; import { SourceMapManager } from './SourceMapManager'; import { forceDeleteDir } from '../testHelpers.spec'; let tmpPath = s`${process.cwd()}/.tmp`; +const sinon = sinonActual.createSandbox(); describe('SourceMapManager', () => { let manager: SourceMapManager; @@ -14,6 +16,7 @@ describe('SourceMapManager', () => { manager = new SourceMapManager(); }); afterEach(async () => { + sinon.restore(); await forceDeleteDir(tmpPath); }); @@ -181,6 +184,49 @@ describe('SourceMapManager', () => { }); }); + describe('getSourceMapPath', () => { + it('reads the file from disk to find the sourceMappingURL comment when no contents are provided', async () => { + const brsPath = s`${tmpPath}/staging/source/main.brs`; + fsExtra.ensureDirSync(path.dirname(brsPath)); + fsExtra.writeFileSync(brsPath, `sub main()\nend sub\n'//# sourceMappingURL=custom.brs.map`); + + const result = await manager.getSourceMapPath(brsPath); + expect(s`${result}`).to.equal(s`${tmpPath}/staging/source/custom.brs.map`); + }); + + it('uses the provided fileContents instead of reading from disk', async () => { + const brsPath = s`${tmpPath}/staging/source/main.brs`; + const readFile = sinon.spy(fsExtra, 'readFile'); + + const result = await manager.getSourceMapPath(brsPath, `sub main()\nend sub\n'//# sourceMappingURL=custom.brs.map`); + + expect(s`${result}`).to.equal(s`${tmpPath}/staging/source/custom.brs.map`); + //it did NOT read the file from disk + expect(readFile.called).to.be.false; + }); + + it('falls back to the colocated .map path when contents have no comment', async () => { + const brsPath = s`${tmpPath}/staging/source/main.brs`; + const result = await manager.getSourceMapPath(brsPath, `sub main()\nend sub`); + expect(s`${result}`).to.equal(s`${brsPath}.map`); + }); + + it('caches the resolved path and does not read again on subsequent calls', async () => { + const brsPath = s`${tmpPath}/staging/source/main.brs`; + fsExtra.ensureDirSync(path.dirname(brsPath)); + fsExtra.writeFileSync(brsPath, `sub main()\nend sub\n'//# sourceMappingURL=custom.brs.map`); + + //first call populates the cache (reading from disk) + await manager.getSourceMapPath(brsPath); + + const readFile = sinon.spy(fsExtra, 'readFile'); + const result = await manager.getSourceMapPath(brsPath); + expect(s`${result}`).to.equal(s`${tmpPath}/staging/source/custom.brs.map`); + //second call served from cache — no disk read + expect(readFile.called).to.be.false; + }); + }); + describe('getOriginalLocation', () => { it('resolves location using a map with a relative sourceRoot', async () => { // staging/components/foo.brs maps back to src/components/foo.brs diff --git a/src/managers/SourceMapManager.ts b/src/managers/SourceMapManager.ts index 7000ce53..0c525b05 100644 --- a/src/managers/SourceMapManager.ts +++ b/src/managers/SourceMapManager.ts @@ -94,18 +94,23 @@ export class SourceMapManager { * * Returns undefined if no source map is found. * @param stagingFilePath + * @param fileContents the already-loaded contents of `stagingFilePath`. Pass this when the caller + * already has the file in memory to avoid a redundant disk read; omit it to have this method read + * the file itself. Ignored once the result for this path is cached. * @returns */ - public async getSourceMapPath(stagingFilePath: string) { + public async getSourceMapPath(stagingFilePath: string, fileContents?: 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: string | undefined; - try { - contents = await fsExtra.readFile(stagingFilePath, 'utf8'); - } catch { - // file doesn't exist — fall through to the colocated map assumption + //use the caller-provided contents when available; otherwise read the file on disk + let contents = fileContents; + if (contents === 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