fix: ensure the callback 'name' arg is the module name when matching the module main file, even when 'internals: true' option is used by trentm · Pull Request #241 · nodejs/import-in-the-middle

trentm · 2026-01-27T00:12:06Z

This implements "option 1" of #238

Using the got module as an example, the issue was that the following
two hooks would have a different name value in the callback when the
got main file (".../node_modules/got/dist/source/index.js") was
loaded:

Hook(['got'], (exports, name) => { ... })
Hook(['got'], {internals: true}, (exports, name) => { ... })

In the first case name === 'got',
in the second case name === 'got/dist/source/index.js.
This differed from RITM behaviour where name === 'got' in both cases.

This also fixes an issue with IITM not properly hooking an absolute
path: when the absolute path being imported happened to be in a
"node_modules" subdir.

Closes: #238

Checklist

This builds on fix: fix a couple issues with duplicate entries and specifier (submodule) matching #237 and needs that PR to merge first.

Details on the absolute-path import fixes

Consider this can-iitm-match-abs-paths.mjs which attempts to hook four absolute paths, with slightly different cases:

import { register } from 'node:module'
import { Hook } from './index.js'

register('./hook.mjs', import.meta.url)

const CWD = process.cwd()
Hook([
  `${CWD}/test/fixtures/index.js`,
  `${CWD}/test/fixtures/something.js`,
  `${CWD}/test/fixtures/node_modules/some-external-module/index.mjs`,
  `${CWD}/test/fixtures/node_modules/some-external-module/sub.mjs`
], (mod, name, baseDir) => {
  console.log(`IITM Hook: name=${name}, baseDir=${baseDir}`)
})

// 1. Import an absolute path.
await import(`${CWD}/test/fixtures/index.js`)

// 2. Import a relative path.
await import('./test/fixtures/something.js')

// 3. Absolute path, note the file happens to be in a `node_modules` dir.
await import(`${CWD}/test/fixtures/node_modules/some-external-module/index.mjs`)

// 4. Relative path, note the file happens to be in a `node_modules` dir.
await import('./test/fixtures/node_modules/some-external-module/sub.mjs')

With current IITM:

Case 3 matches with a misleading (and inconsistent with other abspath matches) baseDir in the callback.
Case 4 doesn't match at all.

% node can-iitm-match-abs-paths.mjs
IITM Hook: name=/Users/trentm/tm/import-in-the-middle6/test/fixtures/index.js, baseDir=undefined
IITM Hook: name=/Users/trentm/tm/import-in-the-middle6/test/fixtures/something.js, baseDir=undefined
IITM Hook: name=/Users/trentm/tm/import-in-the-middle6/test/fixtures/node_modules/some-external-module/index.mjs, baseDir=/Users/trentm/tm/import-in-the-middle6/test/fixtures/node_modules/some-external-module

The reason for both issues is that the absolute files happen to be under a .../node_modules/... tree. Case 3 accidentally matches using the matchArg === specifier branch, rather than the intended match branch for absolute paths.

With the changes in this PR:

% node can-iitm-match-abs-paths.mjs
IITM Hook: name=/Users/trentm/tm/import-in-the-middle6/test/fixtures/index.js, baseDir=undefined
IITM Hook: name=/Users/trentm/tm/import-in-the-middle6/test/fixtures/something.js, baseDir=undefined
IITM Hook: name=/Users/trentm/tm/import-in-the-middle6/test/fixtures/node_modules/some-external-module/index.mjs, baseDir=undefined
IITM Hook: name=/Users/trentm/tm/import-in-the-middle6/test/fixtures/node_modules/some-external-module/sub.mjs, baseDir=undefined

Comparing to RITM

Using the equivalent test script for RITM:

can-ritm-match-abs-paths.cjs

const { Hook } = require('require-in-the-middle')

const CWD = process.cwd()
Hook([
  `${CWD}/test/fixtures/index.js`,
  `${CWD}/test/fixtures/something.js`,
  `${CWD}/test/fixtures/node_modules/some-external-cjs-module/index.js`,
  `${CWD}/test/fixtures/node_modules/some-external-cjs-module/sub.js`
], (exports, name, baseDir) => {
  console.log(`IITM Hook: name=${name}, baseDir=${baseDir}`)
  return exports
})

// Import an absolute path.
require(`${CWD}/test/fixtures/index.js`)

// Import a relative path.
require('./test/fixtures/something.js')

// Absolute path, note the file happens to be in a `node_modules` dir.
require(`${CWD}/test/fixtures/node_modules/some-external-cjs-module/index.js`)

// Relative path, note the file happens to be in a `node_modules` dir.
require('./test/fixtures/node_modules/some-external-cjs-module/sub.js')

the result is:

% node can-ritm-match-abs-paths.cjs
IITM Hook: name=index, baseDir=/Users/trentm/tm/import-in-the-middle6/test/fixtures
IITM Hook: name=something, baseDir=/Users/trentm/tm/import-in-the-middle6/test/fixtures
IITM Hook: name=index, baseDir=/Users/trentm/tm/import-in-the-middle6/test/fixtures/node_modules/some-external-cjs-module
IITM Hook: name=sub, baseDir=/Users/trentm/tm/import-in-the-middle6/test/fixtures/node_modules/some-external-cjs-module

All four match.
(Aside: the name and baseDir values are different between IITM and RITM, but that is a topic for a separate PR.)

…ies and specifier (submodule) matching

…mproved import(cjs) export sniffing

…ine, but CI currently tests v21 which did NOT get sufficient backports to work for these test files

…to module main file, even when 'internals: true' option is used This implements "option 1" of nodejs#238 Using the `got` module as an example, the issue was that the following two hooks would have a different `name` value when calling back when the `got` main file (".../node_modules/got/dist/source/index.js") was loaded: Hook(['got'], (exports, name) => { ... }) Hook(['got'], {internals: true}, (exports, name) => { ... }) In the first case `name === 'got'`, in the second case `name === 'got/dist/source/index.js`. This differed from RITM behaviour where `name === 'got'` in both cases. This *also* fixes an issue with IITM not properly hooking an absolute path: when the absolute path being imported happened to be in a "node_modules" subdir. Closes: nodejs#238

trentm

Add some notes for reviewers.

trentm · 2026-01-27T17:37:38Z

index.js

-    const isBuiltin = name.startsWith('node:')
-    let baseDir
+    const loadUrl = name
+    const isNodeUrl = loadUrl.startsWith('node:')


Reviewer note: The name change to isNodeUrl is to avoid the name isBuiltin for the actual isBuiltin function from the node:module module in a separate PR (https://github.com/nodejs/import-in-the-middle/pull/240/files).

trentm · 2026-01-27T17:39:42Z

index.js

+      Error.stackTraceLimit = stackTraceLimit
+
+      if (filePath) {
+        const details = moduleDetailsFromPath(filePath)


Reviewer note: This section changes to only do the moduleDetailsFromPath() parsing if we have a file:// URL that was successfully converted to a path.

trentm · 2026-01-27T17:43:27Z

index.js

            callHookFn(hookFn, namespace, name, baseDir)
+          } else if (internals) {
+            const internalPath = name + path.sep + path.relative(baseDir, filePath)
+            callHookFn(hookFn, namespace, internalPath, baseDir)


Reviewer note: This part of the change (that moves the } else if (internals) {-block to be after the else if (baseDir.endsWith(specifier..-block) is for the first/primary fix in this PR: the fix the name in the callback to match what RITM does.

The rest of the changes in "index.js" are for the absolute-paths fix.

jsumners-nr · 2026-01-27T19:15:23Z

index.js


 const path = require('path')
-const parse = require('module-details-from-path')
+const moduleDetailsFromPath = require('module-details-from-path')


trentm · 2026-01-27T21:35:58Z

PTAL, merging from main with a merge conflict dismissed reviews. Thanks.

BridgeAR · 2026-01-31T12:26:04Z

I did not get the chance to fully review this before it landed and I believe this change is actually not what we want, no matter that it deviates from ritm.

The reason is: if we declare internals: true, I really want to be on the safe side to always match the internal loaded file, no matter that the user loads it with the module name. That way it is safe that the correct code in fact always wrapped as expected. Otherwise, we would could up ending with a hook for the module name but not in case it is loaded directly by the user as internal file and I would have to declare a second hook that may only run in case the other one did not run and the other way around, if I am not mistaken (I did not yet verify this with a test case).

@trentm instead of aligning with ritm, I think we should consider changing ritm to also load the internal file when being loaded with the module name.

trentm · 2026-01-31T23:27:52Z

@BridgeAR I moved this to a new issue for discussion. #244

trentm added 11 commits January 23, 2026 11:32

example that demonstrates the problem with 'experimentalPatchInternals'

3ed5244

fix: clarify the matching and fix a couple issues with duplicate entr…

747d2ca

…ies and specifier (submodule) matching

remove play/dev files added earlier for a demo

5a299f0

test fixes for running with older Node.js versions

f61aee9

move the new tests to only test with Node.js v20 and later, for the i…

a083bc7

…mproved import(cjs) export sniffing

need at least v20.19 for these tests apparently

8506ccd

fix lint

0e5f6be

only run these tests with Node.js v22 and later, ^20.19 is actually f…

1cec328

…ine, but CI currently tests v21 which did NOT get sufficient backports to work for these test files

since limiting to Node.js v22, can use top-level await in these tests

cadefca

Merge branch 'main' into trentm-clearer-match-block

2f55c6e

trentm self-assigned this Jan 27, 2026

trentm added 2 commits January 26, 2026 16:17

lint fix

6198e19

Merge branch 'main' into trentm-issue238-ritm-compat

35b437b

trentm commented Jan 27, 2026

View reviewed changes

trentm requested review from BridgeAR and timfish January 27, 2026 17:44

trentm mentioned this pull request Jan 27, 2026

chore: release v2.0.6 #242

Merged

jsumners-nr previously approved these changes Jan 27, 2026

View reviewed changes

timfish previously approved these changes Jan 27, 2026

View reviewed changes

Merge branch 'main' into trentm-issue238-ritm-compat

b703995

trentm dismissed stale reviews from timfish and jsumners-nr via b703995 January 27, 2026 21:34

trentm requested review from jsumners-nr and timfish January 27, 2026 22:23

timfish approved these changes Jan 27, 2026

View reviewed changes

timfish merged commit ad9d02c into nodejs:main Jan 27, 2026
51 checks passed

trentm deleted the trentm-issue238-ritm-compat branch January 27, 2026 22:27

trentm mentioned this pull request Jan 28, 2026

feat(instrumentation): use the internals: true option with import-in-the-middle hook, allowing instrumentations to hook internal files in ES modules open-telemetry/opentelemetry-js#6344

Merged

trentm mentioned this pull request Jan 31, 2026

discuss R/IITM design for callback args with internals: true #244

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: ensure the callback 'name' arg is the module name when matching the module main file, even when 'internals: true' option is used#241

fix: ensure the callback 'name' arg is the module name when matching the module main file, even when 'internals: true' option is used#241
timfish merged 14 commits intonodejs:mainfrom
trentm:trentm-issue238-ritm-compat

trentm commented Jan 27, 2026 •

edited

Loading

Uh oh!

trentm left a comment

Uh oh!

trentm Jan 27, 2026

Uh oh!

trentm Jan 27, 2026 •

edited

Loading

Uh oh!

trentm Jan 27, 2026

Uh oh!

jsumners-nr Jan 27, 2026

Uh oh!

trentm commented Jan 27, 2026

Uh oh!

Uh oh!

BridgeAR commented Jan 31, 2026

Uh oh!

trentm commented Jan 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

trentm commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Details on the absolute-path import fixes

Comparing to RITM

Uh oh!

trentm left a comment

Choose a reason for hiding this comment

Uh oh!

trentm Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

trentm Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trentm Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

jsumners-nr Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

trentm commented Jan 27, 2026

Uh oh!

Uh oh!

BridgeAR commented Jan 31, 2026

Uh oh!

trentm commented Jan 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

trentm commented Jan 27, 2026 •

edited

Loading

trentm Jan 27, 2026 •

edited

Loading