Skip to content

Commit 1dff40f

Browse files
authored
perf(coverage-istanbul): optimize sourcemap URL scanning (#1296)
1 parent 0296eef commit 1dff40f

6 files changed

Lines changed: 172 additions & 30 deletions

File tree

knip.jsonc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,15 @@
8888
"@rstest/browser",
8989
],
9090
},
91+
"packages/coverage-istanbul": {
92+
"entry": [
93+
// Test files
94+
"tests/**/*.ts",
95+
],
96+
},
9197
// No explicit entry needed for the following packages — knip auto-infers from package.json exports:
92-
// packages/browser-react, packages/coverage-istanbul,
93-
// packages/adapter-rslib, packages/adapter-rsbuild, packages/adapter-rspack
98+
// packages/browser-react, packages/adapter-rslib,
99+
// packages/adapter-rsbuild, packages/adapter-rspack
94100

95101
"packages/vscode": {
96102
"entry": [
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { defineConfig } from '@rstest/core';
2+
import { withRslibConfig } from '../adapter-rslib/src';
3+
4+
export default defineConfig({
5+
extends: withRslibConfig({
6+
cwd: __dirname,
7+
}),
8+
name: 'coverage-istanbul',
9+
setupFiles: ['../../scripts/rstest.setup.ts'],
10+
include: ['<rootDir>/tests/**/*.test.ts'],
11+
globals: true,
12+
source: {
13+
tsconfigPath: './tests/tsconfig.json',
14+
},
15+
tools: {
16+
rspack: {
17+
watchOptions: {
18+
ignored: /test-temp-.*/,
19+
},
20+
},
21+
},
22+
});

packages/coverage-istanbul/src/plugin.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ export const pluginCoverage: (
8585
});
8686

8787
api.onExit(() => {
88-
Object.assign(transformCoverageFns, {});
88+
for (const environmentName of Object.keys(transformCoverageFns)) {
89+
delete transformCoverageFns[environmentName];
90+
}
8991
});
9092
},
9193
});

packages/coverage-istanbul/src/utils.ts

Lines changed: 42 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -82,20 +82,27 @@ const sourceMappingURLRegex = new RegExp(
8282
* @param {string} code source code content
8383
* @returns {string | undefined} source mapping information
8484
*/
85-
function getSourceMappingURL(code: string): string | undefined {
86-
const lines = code.split(/^/m);
87-
let match: RegExpMatchArray | null | undefined = null;
85+
export function getSourceMappingURL(code: string): string | undefined {
86+
let searchIndex = code.lastIndexOf('sourceMappingURL');
87+
88+
while (searchIndex !== -1) {
89+
const lineStart = code.lastIndexOf('\n', searchIndex);
90+
const lineEnd = code.indexOf('\n', searchIndex);
91+
const line = code.slice(
92+
lineStart + 1,
93+
lineEnd === -1 ? code.length : lineEnd,
94+
);
95+
const match = line.match(sourceMappingURLRegex);
8896

89-
for (let i = lines.length - 1; i >= 0; i--) {
90-
match = lines[i]?.match(sourceMappingURLRegex);
9197
if (match) {
92-
break;
98+
const sourceMappingURL = match[1] || match[2] || '';
99+
return sourceMappingURL ? decodeURI(sourceMappingURL) : undefined;
93100
}
94-
}
95101

96-
const sourceMappingURL = match ? match[1] || match[2] || '' : '';
102+
searchIndex = code.lastIndexOf('sourceMappingURL', lineStart - 1);
103+
}
97104

98-
return sourceMappingURL ? decodeURI(sourceMappingURL) : sourceMappingURL;
105+
return undefined;
99106
}
100107

101108
export function registerSourceMapURL(
@@ -135,28 +142,36 @@ export async function transformCoverage(
135142
coverageMap: CoverageMap,
136143
sourcemapUrlCache: Map<string, string | undefined>,
137144
): Promise<CoverageMap> {
138-
await mapWithConcurrency(
139-
coverageMap
140-
.files()
141-
// process js/cjs/mjs file only
142-
.filter((filename) => filename.endsWith('js')),
143-
SOURCE_MAP_SCAN_CONCURRENCY,
144-
async (filename) => {
145-
let url = sourcemapUrlCache.get(filename);
146-
if (!url) {
147-
const { readFile } = await import('node:fs/promises');
148-
url = await readFile(filename, 'utf8').then(
149-
(content) => getSourceMappingURL(content),
150-
() => undefined,
151-
);
152-
}
153-
sourcemapUrlCache.set(filename, url);
154-
},
145+
const jsFiles = coverageMap
146+
.files()
147+
// process js/cjs/mjs file only
148+
.filter((filename) => filename.endsWith('js'));
149+
150+
const uncachedFiles = jsFiles.filter(
151+
(filename) => !sourcemapUrlCache.has(filename),
155152
);
156153

154+
if (uncachedFiles.length) {
155+
const { readFile } = await import('node:fs/promises');
156+
await mapWithConcurrency(
157+
uncachedFiles,
158+
SOURCE_MAP_SCAN_CONCURRENCY,
159+
async (filename) => {
160+
try {
161+
const content = await readFile(filename, 'utf8');
162+
sourcemapUrlCache.set(filename, getSourceMappingURL(content));
163+
} catch {
164+
// Do not cache failed reads. The file may be temporarily unavailable
165+
// during watch-mode rebuilds, so retry it on the next report.
166+
}
167+
},
168+
);
169+
}
170+
157171
// Call createSourceMapStore as needed
158172
let store: MapStore | undefined;
159-
for (const [filename, url] of sourcemapUrlCache) {
173+
for (const filename of jsFiles) {
174+
const url = sourcemapUrlCache.get(filename);
160175
if (url) {
161176
if (!store) {
162177
const { createSourceMapStore } =
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"extends": "../tsconfig.json",
3+
"compilerOptions": {
4+
"rootDir": ".",
5+
"types": ["node", "@rstest/core/globals"]
6+
},
7+
"include": ["**/*.test.ts"]
8+
}
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
2+
import { tmpdir } from 'node:os';
3+
import path from 'node:path';
4+
import istanbulCoverage from 'istanbul-lib-coverage';
5+
import { getSourceMappingURL, transformCoverage } from '../src/utils';
6+
7+
describe('coverage istanbul utils', () => {
8+
it('extracts the last sourceMappingURL without splitting the whole file', () => {
9+
const code = [
10+
'const sourceMappingURL = "not a comment";',
11+
'//# sourceMappingURL=first.js.map',
12+
'console.log("hello");',
13+
'//# sourceMappingURL=second.js.map',
14+
].join('\n');
15+
16+
expect(getSourceMappingURL(code)).toBe('second.js.map');
17+
});
18+
19+
it('returns undefined when sourceMappingURL only appears outside comments', () => {
20+
expect(getSourceMappingURL('const sourceMappingURL = "inline";')).toBe(
21+
undefined,
22+
);
23+
});
24+
25+
it('does not reread files whose sourcemap cache entry is already undefined', async () => {
26+
const root = mkdtempSync(path.join(tmpdir(), 'rstest-coverage-cache-'));
27+
const file = path.join(root, 'index.js');
28+
writeFileSync(file, 'export const value = 1;\n');
29+
30+
const coverageMap = istanbulCoverage.createCoverageMap({
31+
[file]: {
32+
path: file,
33+
statementMap: {},
34+
fnMap: {},
35+
branchMap: {},
36+
s: {},
37+
f: {},
38+
b: {},
39+
},
40+
});
41+
const sourcemapUrlCache = new Map<string, string | undefined>([
42+
[file, undefined],
43+
]);
44+
45+
try {
46+
rmSync(file);
47+
await expect(
48+
transformCoverage(coverageMap, sourcemapUrlCache),
49+
).resolves.toBe(coverageMap);
50+
} finally {
51+
rmSync(root, { recursive: true, force: true });
52+
}
53+
});
54+
55+
it('retries sourcemap reads after filesystem errors', async () => {
56+
const root = mkdtempSync(path.join(tmpdir(), 'rstest-coverage-retry-'));
57+
const file = path.join(root, 'index.js');
58+
59+
const coverageMap = istanbulCoverage.createCoverageMap({
60+
[file]: {
61+
path: file,
62+
statementMap: {},
63+
fnMap: {},
64+
branchMap: {},
65+
s: {},
66+
f: {},
67+
b: {},
68+
},
69+
});
70+
const sourcemapUrlCache = new Map<string, string | undefined>();
71+
72+
try {
73+
await expect(
74+
transformCoverage(coverageMap, sourcemapUrlCache),
75+
).resolves.toBe(coverageMap);
76+
expect(sourcemapUrlCache.has(file)).toBe(false);
77+
78+
writeFileSync(file, 'export const value = 1;\n');
79+
80+
await expect(
81+
transformCoverage(coverageMap, sourcemapUrlCache),
82+
).resolves.toBe(coverageMap);
83+
expect(sourcemapUrlCache.has(file)).toBe(true);
84+
expect(sourcemapUrlCache.get(file)).toBe(undefined);
85+
} finally {
86+
rmSync(root, { recursive: true, force: true });
87+
}
88+
});
89+
});

0 commit comments

Comments
 (0)