nodejs / nodejs/import-in-the-middle

Retry for delayed exports cannot cover synchronous consumers (`class X extends Y`) — follow-up to #227

Open
#280 0 comments 0 reactions 0 assignees View on GitHub

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 against import-in-the-middle@3.3.3 on 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

  1. Diagnostic (cheap, no redesign). When bind() defers an export because it read undefined, 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.
  2. 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.
  3. A sound fix, if it's ever in scope. Rewrite the original module to expose setters for its own bindings, have the wrapper export … from it 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.