-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Optionally only wrap modules hooked in --import
#146
Changes from all commits
7335da6
bc8eb03
b001494
612abcf
a92146a
bf1cde8
ab39976
766bc2b
e9c7c41
b784138
d82c8ae
708ed61
d41b017
35fdfe0
323bfc2
6c4384b
b70a48f
3a510c4
7299d2d
2d3fc58
0654e75
e0ebcb9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,3 +84,39 @@ export declare function addHook(hookFn: HookFunction): void | |
* @param {HookFunction} hookFn The function to be removed. | ||
*/ | ||
export declare function removeHook(hookFn: HookFunction): void | ||
|
||
type CreateAddHookMessageChannelReturn<Data> = { | ||
addHookMessagePort: MessagePort, | ||
waitForAllMessagesAcknowledged: Promise<void> | ||
registerOptions: { data?: Data; transferList?: any[]; } | ||
} | ||
|
||
/** | ||
* EXPERIMENTAL | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Too bad TypeScript jsdoc tag support doesn't support |
||
* This feature is experimental and may change in minor versions. | ||
* **NOTE** This feature is incompatible with the {internals: true} Hook option. | ||
* | ||
* Creates a message channel with a port that can be used to add hooks to the | ||
* list of exclusively included modules. | ||
* | ||
* This can be used to only wrap modules that are Hook'ed, however modules need | ||
* to be hooked before they are imported. | ||
* | ||
* ```ts | ||
* import { register } from 'module' | ||
* import { Hook, createAddHookMessageChannel } from 'import-in-the-middle' | ||
* | ||
* const { registerOptions, waitForAllMessagesAcknowledged } = createAddHookMessageChannel() | ||
* | ||
* register('import-in-the-middle/hook.mjs', import.meta.url, registerOptions) | ||
* | ||
* Hook(['fs'], (exported, name, baseDir) => { | ||
* // Instrument the fs module | ||
* }) | ||
* | ||
* // Ensure that the loader has acknowledged all the modules | ||
* // before we allow execution to continue | ||
* await waitForAllMessagesAcknowledged() | ||
* ``` | ||
*/ | ||
export declare function createAddHookMessageChannel<Data = any>(): CreateAddHookMessageChannelReturn<Data>; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
const path = require('path') | ||
const parse = require('module-details-from-path') | ||
const { fileURLToPath } = require('url') | ||
const { MessageChannel } = require('worker_threads') | ||
|
||
const { | ||
importHooks, | ||
|
@@ -31,6 +32,75 @@ function callHookFn (hookFn, namespace, name, baseDir) { | |
} | ||
} | ||
|
||
let sendModulesToLoader | ||
|
||
/** | ||
* EXPERIMENTAL | ||
* This feature is experimental and may change in minor versions. | ||
* **NOTE** This feature is incompatible with the {internals: true} Hook option. | ||
* | ||
* Creates a message channel with a port that can be used to add hooks to the | ||
* list of exclusively included modules. | ||
* | ||
* This can be used to only wrap modules that are Hook'ed, however modules need | ||
* to be hooked before they are imported. | ||
* | ||
* ```ts | ||
* import { register } from 'module' | ||
* import { Hook, createAddHookMessageChannel } from 'import-in-the-middle' | ||
* | ||
* const { registerOptions, waitForAllMessagesAcknowledged } = createAddHookMessageChannel() | ||
* | ||
* register('import-in-the-middle/hook.mjs', import.meta.url, registerOptions) | ||
* | ||
* Hook(['fs'], (exported, name, baseDir) => { | ||
* // Instrument the fs module | ||
* }) | ||
* | ||
* // Ensure that the loader has acknowledged all the modules | ||
* // before we allow execution to continue | ||
* await waitForAllMessagesAcknowledged() | ||
* ``` | ||
*/ | ||
function createAddHookMessageChannel () { | ||
timfish marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const { port1, port2 } = new MessageChannel() | ||
let pendingAckCount = 0 | ||
let resolveFn | ||
|
||
sendModulesToLoader = (modules) => { | ||
pendingAckCount++ | ||
port1.postMessage(modules) | ||
} | ||
|
||
port1.on('message', () => { | ||
pendingAckCount-- | ||
|
||
if (resolveFn && pendingAckCount <= 0) { | ||
resolveFn() | ||
} | ||
}).unref() | ||
|
||
function waitForAllMessagesAcknowledged () { | ||
// This timer is to prevent the process from exiting with code 13: | ||
// 13: Unsettled Top-Level Await. | ||
const timer = setInterval(() => { }, 1000) | ||
const promise = new Promise((resolve) => { | ||
resolveFn = resolve | ||
}).then(() => { clearInterval(timer) }) | ||
Comment on lines
+86
to
+89
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This timer stops the process exiting with error code 13 which is triggered by blocking exit via top-level await if there are no more active handles. This feels... sub-optimal but I don't know how else we should be working around this! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I spent little bit trying to avoid this but couldn't come up with a better way. I'm gonna take another look tomorrow morning, but if I can't figure anything else out I'll approve. |
||
|
||
if (pendingAckCount === 0) { | ||
resolveFn() | ||
} | ||
|
||
return promise | ||
} | ||
|
||
const addHookMessagePort = port2 | ||
const registerOptions = { data: { addHookMessagePort, include: [] }, transferList: [addHookMessagePort] } | ||
|
||
return { registerOptions, addHookMessagePort, waitForAllMessagesAcknowledged } | ||
} | ||
|
||
function Hook (modules, options, hookFn) { | ||
if ((this instanceof Hook) === false) return new Hook(modules, options, hookFn) | ||
if (typeof modules === 'function') { | ||
|
@@ -43,6 +113,10 @@ function Hook (modules, options, hookFn) { | |
} | ||
const internals = options ? options.internals === true : false | ||
|
||
if (sendModulesToLoader && Array.isArray(modules)) { | ||
sendModulesToLoader(modules) | ||
} | ||
|
||
this._iitmHook = (name, namespace) => { | ||
const filename = name | ||
const isBuiltin = name.startsWith('node:') | ||
|
@@ -92,3 +166,4 @@ module.exports = Hook | |
module.exports.Hook = Hook | ||
module.exports.addHook = addHook | ||
module.exports.removeHook = removeHook | ||
module.exports.createAddHookMessageChannel = createAddHookMessageChannel |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
import { strictEqual } from 'assert' | ||
import { sep } from 'path' | ||
import * as os from 'node:os' | ||
import { Hook } from '../../index.js' | ||
|
||
const hooked = [] | ||
|
||
Hook((_, name) => { | ||
hooked.push(name) | ||
}) | ||
|
||
strictEqual(hooked.length, 2) | ||
strictEqual(hooked[0], 'path') | ||
strictEqual(hooked[1], 'os') | ||
strictEqual(sep, '@') | ||
strictEqual(os.arch(), 'new_crazy_arch') |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import { register } from 'module' | ||
import { Hook, createAddHookMessageChannel } from '../../index.js' | ||
// We've imported path here to ensure that the hook is still applied later even | ||
// if the library is used here. | ||
import * as path from 'path' | ||
|
||
const { registerOptions, waitForAllMessagesAcknowledged } = createAddHookMessageChannel() | ||
|
||
register('../../hook.mjs', import.meta.url, registerOptions) | ||
|
||
Hook(['path'], (exports) => { | ||
exports.sep = '@' | ||
}) | ||
|
||
Hook(['os'], (exports) => { | ||
exports.arch = function () { | ||
return 'new_crazy_arch' | ||
} | ||
}) | ||
|
||
console.assert(path.sep !== '@') | ||
|
||
await waitForAllMessagesAcknowledged() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import { spawnSync } from 'child_process' | ||
|
||
const out = spawnSync(process.execPath, | ||
['--import', './test/fixtures/import.mjs', './test/fixtures/import-after.mjs'], | ||
{ stdio: 'inherit', env: {} } | ||
) | ||
|
||
if (out.error) { | ||
console.error(out.error) | ||
} | ||
if (out.status !== 0) { | ||
console.error(`Expected exit code 0, got ${out.status}`) | ||
} | ||
process.exit(out.status) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the addition I need to make!