nodejs / nodejs/import-in-the-middle
Retry for delayed exports cannot cover synchronous consumers (`class X extends Y`) — follow-up to #227
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 170
- Forks
- 58
- Avg merge
- 3d 19h
- Merged PRs (30d)
- 7
Description
[!NOTE]
This issue was researched and written by Claude (an AI agent) on behalf of @segevfiner, who reviewed it before filing. All source references and the reproduction output below come from real runs againstimport-in-the-middle@3.3.3on Node v22.23.2 — nothing here is inferred.
Summary
The retry-for-undefined-exports mechanism added in 2.0.3 (#221, "handle undefined exports properly") fixes delayed-initialization cases only when the importer reads the export asynchronously. It cannot help when the importer reads it synchronously during its own module evaluation — the canonical case being class Sub extends Imported {}.
This looks like the "likely not working 100%" caveat @BridgeAR called out in https://github.com/nodejs/import-in-the-middle/issues/227#issuecomment-3755150401, so I'm filing it as a follow-up to #227 / #32 rather than reopening either.
It matters in practice because it is exactly the shape bundlers emit: esbuild's __esm lazy-init wrapper (which rolldown also emits) declares var X; and assigns it inside an init_*() function, relying on ESM live bindings across the chunk boundary.
Reproduction
Standalone, three files, no OpenTelemetry and no bundler involved.
npm init -y && npm pkg set type=module && npm i import-in-the-middle@3.3.3
dep.mjs
// Assigned after evaluation, exactly like a bundler's lazy-init wrapper emits.
export var Late;
export function init() {
Late = class Late {};
}
main.mjs
import { Late, init } from './dep.mjs';
init(); // assigns `Late` inside dep.mjs
class Sub extends Late {} // reads it back synchronously, via the live binding
console.log('ok:', Sub.name);
hook.mjs
import { register } from 'node:module';
register('import-in-the-middle/hook.mjs', import.meta.url);
Actual
$ node main.mjs
ok: Sub
$ node --import ./hook.mjs main.mjs
TypeError: Class extends value undefined is not a constructor or null
Expected
Both invocations print ok: Sub. Registering the hook should not change program semantics.
Why the retry cannot reach this case
buildSetter in create-hook.mjs generates, per export:
let $Late
__binder.bind("Late", namespace, v => { $Late = v }, () => $Late, false)
export { $Late as "Late" }
So consumers' static imports bind to $Late, a local in the wrapper — not to namespace.Late. ModuleBinder.bind() seeds $Late once from namespace.Late, and because that read yields undefined, queues an updater on #pending. The wrapper then calls __binder.flush(), which retries on a microtask and then at RETRY_DELAYS = [0, 10, 50].
The importing module's body — including class Sub extends Late — is evaluated as part of the same synchronous graph evaluation, which completes before any microtask runs. So the first retry always fires after the TypeError has already been thrown. This isn't a tuning problem; no delay value can win that race, because the read happens before control returns to the event loop.
The underlying issue is that the wrapper severs the live binding: an exported let cannot be a getter, so $Late cannot track namespace.Late. bind() does hold a live read path (readSource closes over namespace) and #overridden already records whether an instrumentation has patched a name, but neither can be used on the export path.
Real-world impact
Found while adding OpenTelemetry ESM instrumentation to a bundled Fastify service. Registering @opentelemetry/instrumentation/hook.mjs — with no SDK started, the hook alone — crashed the app inside a rolldown chunk:
// chunk-…mjs
var CTag;
var init_ctags = __esm({ "src/analysis/ctags.ts"() {
CTag = class _CTag extends Entity { /* … */ }
}});
export { CTag as r, init_ctags as ct }
// other-chunk.mjs
import { r as CTag, ct as init_ctags } from "./chunk-….mjs";
init_ctags();
var ClassEntity = class _ClassEntity extends CTag { /* … */ } // TypeError
Since __esm is standard esbuild/rolldown output, any sufficiently large bundled ESM app can hit this, and the error names the application's own class rather than anything pointing at a loader hook — in a 260k-line generated chunk that is a genuinely hard trail to follow.
Possible directions
- Diagnostic (cheap, no redesign). When
bind()defers an export because it readundefined, that is the moment liveness was lost. Emitting a warning naming the module and export — behind a debug flag — would turn a multi-hour investigation into one log line. This seems worth doing regardless of whether the rest is ever fixed. - Documentation. The README could state that exports assigned after module evaluation are not tracked synchronously, and name bundler lazy-init output as a known-affected pattern.
- A sound fix, if it's ever in scope. Rewrite the original module to expose setters for its own bindings, have the wrapper
export … fromit so liveness survives natively, and route patches through the original's setters instead of a wrapper-local. That's a redesign rather than a flag, and I appreciate it may not be worth it — but it would remove the whole class rather than narrowing the race window.
Workaround, for anyone who lands here
The experimental include list keeps the hook working while leaving un-instrumented modules untouched, so their live bindings survive:
import { register } from 'node:module';
import { createAddHookMessageChannel } from 'import-in-the-middle';
const { registerOptions, waitForAllMessagesAcknowledged } = createAddHookMessageChannel();
register('@opentelemetry/instrumentation/hook.mjs', import.meta.url, registerOptions);
// … start the SDK, so instrumentations register their modules …
await waitForAllMessagesAcknowledged();
This only avoids the bug by not wrapping the affected modules at all; a module that is instrumented and has delayed exports would still break.
Environment
import-in-the-middle |
3.3.3 |
| Node.js | v22.23.2 |
| Platform | macOS (darwin arm64) |
| Bundler (real-world case) | rolldown via tsdown 0.22.14 |
Related
- #227 — circular re-export "is not a constructor" (closed; the retry is the fix)
- #32 — loader fails with circular dependencies within an application (closed)
- #221 — "handle undefined exports properly", where the retry landed (2.0.3)
- open-telemetry/opentelemetry-js#6984 — downstream proposal to default to the include list, so OpenTelemetry stops triggering this for bundled apps
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Reproduce the failure with dep.mjs, main.mjs, and hook.mjs, then read buildSetter in create-hook.mjs and ModuleBinder.bind(). Compare the diagnostic, documentation, and redesign directions described in the issue before choosing scope. Done should be defined by that choice; for a behavioral fix, the hooked and unhooked commands must both print “ok: Sub”.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100