diff --git a/README.md b/README.md index af26068..2aeca61 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,14 @@ If you just use the normal `spawn`/`exec` methods from the Node.js `child_process` module, then the relevant environment variables will still be tracked, unless explicitly set to `''` or some other value. +### Important + +In order to properly track `lineLengths` (required for coverage +reporting on source mapped files), `@tapjs/processinfo` must be +the **last** loader specified on the command line, so that it can +get access to the transpiled source that Node.js actually +executes. + ### Interacting with Process Info To load the process info data, use the exported `ProcessInfo` class. diff --git a/lib/esm.mts b/lib/esm.mts index ba65941..eb19b9e 100644 --- a/lib/esm.mts +++ b/lib/esm.mts @@ -4,6 +4,7 @@ import { fileURLToPath } from 'url' import type { Serializable } from 'worker_threads' import { getExclude } from './get-exclude.js' import { getProcessInfo } from './get-process-info.js' +import { saveLineLengths } from './line-lengths.js' import { resolve } from './require-resolve.js' // copy main module so that we can --loader=@tapjs/processinfo and use @@ -34,12 +35,14 @@ if (typeof port !== 'undefined') { const { fileURLToPath } = getBuiltin('url') const require = createRequire(${JSON.stringify(base)}) const { getProcessInfo } = require('./get-process-info.js') + const { saveLineLengths } = require('./line-lengths.js') // must be called eagerly here. // this does all the registration as well. const processInfo = getProcessInfo() port.onmessage = (e) => { - const filename = e.data + const { filename, content } = e.data processInfo.files.push(filename) + saveLineLengths(filename, content) } port.unref() } @@ -47,32 +50,40 @@ if (typeof port !== 'undefined') { } const exclude = getExclude('_TAPJS_PROCESSINFO_EXCLUDE_', false) +const record = (url: string, content?: string) => { + const filename = url.startsWith('file://') ? fileURLToPath(url) : url + if (exclude.test(filename)) return + if (PORT) { + PORT.postMessage({ filename, content }) + } else { + // call lazily so we don't double-register + getProcessInfo().files.push(filename) + if (content) saveLineLengths(filename, content) + } +} + export const load = async ( url: string, context: any, nextLoad: Function ) => { - if (/^file:/.test(url)) { + if (url.startsWith('file://')) { const filename = fileURLToPath(url) - if (!exclude.test(filename)) { - if (PORT) { - PORT.postMessage(filename) - } else { - // call lazily so we don't double-register - getProcessInfo().files.push(filename) - } - } const { ext } = parse(filename) // Package bins will sometimes have an extensionless bin script // instead of just naming their extensioned file and letting npm // symlink it for them. Don't blow up when this happens, just tell // node that it's commonjs. if (!ext) { + record(url) return { format: 'commonjs', shortCircuit: true, } } } - return nextLoad(url, context) + + const ret = await nextLoad(url, context) + record(url, String(ret.source)) + return ret } diff --git a/lib/line-lengths.ts b/lib/line-lengths.ts new file mode 100644 index 0000000..866e46f --- /dev/null +++ b/lib/line-lengths.ts @@ -0,0 +1,29 @@ +// TODO: Refactor once https://github.com/nodejs/node/issues/48460 fixed + +import { fileURLToPath } from 'url' + +const kLLC = Symbol.for('@tapjs/processinfo lineLength cache') +const g = global as { + [kLLC]?: Map +} +const cache = g[kLLC] || new Map() +g[kLLC] = cache + +const sourceMapComment = '//# sourceMappingURL=' +export const saveLineLengths = (filename: string, content: string) => { + if (filename.startsWith('file://')) filename = fileURLToPath(filename) + if (cache.has(filename)) return + // no need if it's not sourcemapped + const ll = !content.includes(sourceMapComment) + ? [] + : content + .replace(/[\n\u2028\u2029]$/, '') + .split(/\n|\u2028|\u2029/) + .map(l => l.length) + cache.set(filename, ll) +} + +export const getLineLengths = (filename: string) => { + if (filename.startsWith('file://')) filename = fileURLToPath(filename) + return cache.get(filename) +} diff --git a/lib/register-cjs.ts b/lib/register-cjs.ts index b3f343a..4c255f3 100644 --- a/lib/register-cjs.ts +++ b/lib/register-cjs.ts @@ -2,6 +2,7 @@ import { addHook } from 'pirates' import { getExclude } from './get-exclude.js' import { getProcessInfo } from './get-process-info.js' +import { saveLineLengths } from './line-lengths.js' const kRegisterCJS = Symbol.for('@tapjs/processinfo.registerCJS') const g = global as typeof globalThis & { @@ -16,8 +17,12 @@ export const register = () => { addHook( (code, filename) => { getProcessInfo().files.push(filename) + saveLineLengths(filename, code) return code }, - { exts: ['.js', '.cjs'], matcher: filename => !exclude.test(filename) } + { + exts: ['.js', '.cjs', '.ts', '.cts', '.jsx', '.tsx'], + matcher: filename => !exclude.test(filename), + } ) } diff --git a/lib/register-coverage.ts b/lib/register-coverage.ts index 4793408..04dd18c 100644 --- a/lib/register-coverage.ts +++ b/lib/register-coverage.ts @@ -3,10 +3,11 @@ const enabled = process.env._TAPJS_PROCESSINFO_COVERAGE_ !== '0' import { mkdirSync, readFileSync, writeFileSync } from 'fs' import { Session } from 'inspector' -import { findSourceMap, SourceMapPayload } from 'module' +import { findSourceMap, SourceMap, SourceMapPayload } from 'module' import { fileURLToPath } from 'url' import { getExclude } from './get-exclude.js' import { ProcessInfoNodeData } from './get-process-info.js' +import {getLineLengths} from './line-lengths.js' export let SESSION: Session | undefined = undefined @@ -20,7 +21,10 @@ const exclude = getExclude('_TAPJS_PROCESSINFO_COV_EXCLUDE_') // that pass the exclusion RegExp filter. If included in this list, // then coverage will be recorded, even if it matches exclude. const cfEnv = process.env._TAPJS_PROCESSINFO_COV_FILES_ || '' -const coveredFiles: string[] = cfEnv.trim().split('\n').filter(f => !!f) +const coveredFiles: string[] = cfEnv + .trim() + .split('\n') + .filter(f => !!f) const fileCovered = ( f: string, s?: SourceMapPayload, @@ -69,16 +73,6 @@ export const register = () => { } /* c8 ignore stop */ -// NB: this is the *generated* line lengths, not the source -const lineLengths = (f: string): number[] => { - try { - const content = readFileSync(f, 'utf8') - return content.split(/\n|\u2028|\u2029/).map(l => l.length) - } catch { - return [] - } -} - export const coverageOnProcessEnd = ( cwd: string, processInfo: ProcessInfoNodeData @@ -128,7 +122,7 @@ export const coverageOnProcessEnd = ( if (s) { const { payload } = s sourceMapCache[obj.url] = Object.assign(Object.create(null), { - lineLengths: lineLengths(f), + lineLengths: getLineLengths(f), data: payload, }) } diff --git a/package-lock.json b/package-lock.json index 1869d5d..94ae2b8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -26,7 +26,7 @@ "tap": "^16.3.0", "ts-node": "^10.9.1", "typedoc": "^0.24.8", - "typescript": "^5.0.4" + "typescript": "^5.1.3" }, "engines": { "node": ">=16.17" @@ -4610,16 +4610,16 @@ } }, "node_modules/typescript": { - "version": "5.0.4", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.0.4.tgz", - "integrity": "sha512-cW9T5W9xY37cc+jfEnaUvX91foxtHkza3Nw3wkoF4sSlKn0MONdkdEndig/qPBWXNkmplh3NzayQzCiHM4/hqw==", + "version": "5.1.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.1.3.tgz", + "integrity": "sha512-XH627E9vkeqhlZFQuL+UsyAXEnibT0kWR2FWONlr4sTjvxyJYnyefgrkyECLzM5NenmKzRAy2rR/OlYLA1HkZw==", "dev": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" }, "engines": { - "node": ">=12.20" + "node": ">=14.17" } }, "node_modules/unicode-length": { diff --git a/package.json b/package.json index 3431e1d..0909e00 100644 --- a/package.json +++ b/package.json @@ -85,7 +85,7 @@ "tap": "^16.3.0", "ts-node": "^10.9.1", "typedoc": "^0.24.8", - "typescript": "^5.0.4" + "typescript": "^5.1.3" }, "repository": "https://github.com/tapjs/processinfo" } diff --git a/test/line-lengths.ts b/test/line-lengths.ts new file mode 100644 index 0000000..11d0f35 --- /dev/null +++ b/test/line-lengths.ts @@ -0,0 +1,27 @@ +// just here to explicitly hit the bit that C8 doesn't capture otherwise +import t from 'tap' +import { + getLineLengths, + saveLineLengths, +} from '../dist/cjs/line-lengths.js' + +const contentNoSM = `#!/usr/bin/env node + +console.log('hello, world') + +process.exit(99) +` + +const contentWithSM = `${contentNoSM} +//# sourceMappingURL=https://www.example.com/ +` + +t.equal(getLineLengths('/content/no-sm'), undefined) +t.equal(getLineLengths('/content/with-sm'), undefined) +saveLineLengths('file:///content/no-sm', contentNoSM) +saveLineLengths('/content/with-sm', contentWithSM) +t.strictSame(getLineLengths('/content/no-sm'), []) +t.strictSame( + getLineLengths('file:///content/with-sm'), + [19, 0, 27, 0, 16, 0, 45] +) diff --git a/test/register-coverage.ts b/test/register-coverage.ts index 1fcb2cc..3784244 100644 --- a/test/register-coverage.ts +++ b/test/register-coverage.ts @@ -103,8 +103,16 @@ t.test('coverage enabled', async t => { }) t.test('coverage of diff module enabled', async t => { + const lineLengthMod = require.resolve('../dist/cjs/line-lengths.js') const dir = t.testdir({ 'r.js': ` + const { saveLineLengths } = require(${JSON.stringify(lineLengthMod)}) + const { readFileSync } = require('fs') + const mods = [__filename, './x.js', 'diff'].map(m => require.resolve(m)) + for (const m of mods) { + const content = readFileSync(m, 'utf8') + saveLineLengths(m, content) + } const {coverageOnProcessEnd, register} = require(${JSON.stringify( mod )}) @@ -147,7 +155,10 @@ t.test('coverage of diff module enabled', async t => { t.notSame(cov['source-map-cache'], {}) const f = String(pathToFileURL(require.resolve('diff'))) const content = readFileSync(require.resolve('diff'), 'utf8') - const lineLengths = content.split(/\n/).map(l => l.length) + const lineLengths = content + .replace(/\n$/, '') + .split(/\n/) + .map(l => l.length) t.strictSame(lineLengths, cov['source-map-cache'][f].lineLengths) })