Skip to content

Commit

Permalink
Track lineLengths during loader process
Browse files Browse the repository at this point in the history
Workaround for: nodejs/node#48460
  • Loading branch information
isaacs committed Jun 15, 2023
1 parent 02656e4 commit 50304ce
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 32 deletions.
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
33 changes: 22 additions & 11 deletions lib/esm.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -34,45 +35,55 @@ 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()
}
`
}

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
}
29 changes: 29 additions & 0 deletions lib/line-lengths.ts
Original file line number Diff line number Diff line change
@@ -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<string, number[]>
}
const cache = g[kLLC] || new Map<string, number[]>()
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)
}
7 changes: 6 additions & 1 deletion lib/register-cjs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 & {
Expand All @@ -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),
}
)
}
20 changes: 7 additions & 13 deletions lib/register-coverage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
})
}
Expand Down
10 changes: 5 additions & 5 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
27 changes: 27 additions & 0 deletions test/line-lengths.ts
Original file line number Diff line number Diff line change
@@ -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]
)
13 changes: 12 additions & 1 deletion test/register-coverage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
)})
Expand Down Expand Up @@ -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)
})

Expand Down

0 comments on commit 50304ce

Please sign in to comment.