fix: resolve 'ambiguous' chained export * via explicit fallback

create-hook.mjs

@@ -198,21 +198,31 @@ function emitWarning (err) {
  * of how the loader is driven, so both the synchronous and asynchronous paths
  * share it.
  *
- * The value is read from `namespaceVar`, the wrapper's namespace binding for the
- * module that *defines* the export. For a module's own exports that is the
- * wrapped module itself; for a name re-exported through `export *` it is the
- * leaf that declares it. Reading from the defining module rather than the
- * aggregating one keeps the value resolvable when the same binding reaches the
- * aggregator through more than one re-export chain — Node sees those chains as
- * distinct wrapper modules and leaves the name ambiguous (hence `undefined`) on
- * the aggregate namespace, while the defining module always holds it (#171).
+ * The value is read from `namespaceVar`, the wrapped module's own namespace. A
+ * name pulled in through `export *` additionally passes `fallbackVar`, the
+ * namespace of the module that *defines* it: the binder reads the aggregate
+ * namespace first (so a hook applied along the re-export chain is still
+ * observed) and falls back to the defining module when the aggregate does not
+ * hold the value. That fallback covers two cases the aggregate namespace cannot
+ * serve synchronously:
+ *   - the same binding reaching the aggregator through more than one re-export
+ *     chain, which Node leaves ambiguous (hence `undefined`) on the aggregate
+ *     namespace because under iitm the chains are distinct wrapper modules,
+ *     while the defining module always holds it (#171); and
+ *   - a circular `export *` where the defining module is still in its temporal
+ *     dead zone on the aggregate namespace when this wrapper evaluates, yet has
+ *     already hoisted the binding in its own namespace, so deferring to a late
+ *     async retry would let a synchronous importer read `undefined` (#269).
  *
  * @param {string} n The exported name.
  * @param {string} srcUrl The URL of the module the export belongs to.
- * @param {string} namespaceVar The wrapper binding holding `srcUrl`'s namespace.
+ * @param {string} namespaceVar The wrapper binding holding the primary namespace.
+ * @param {string} [fallbackVar] The wrapper binding holding the defining
+ * module's namespace, for `export *`-sourced names. Omitted for a module's own
+ * exports, which only ever read from `namespaceVar`.
  * @returns {string}
  */
-function buildSetter (n, srcUrl, namespaceVar) {
+function buildSetter (n, srcUrl, namespaceVar, fallbackVar) {
   const variableName = `$${n.replace(/[^a-zA-Z0-9_$]/g, '_')}`
   const objectKey = JSON.stringify(n)
   const reExportedName = n === 'default' ? n : objectKey
@@ -226,8 +236,10 @@ function buildSetter (n, srcUrl, namespaceVar) {
     ? ''
     : `export { ${variableName} as ${reExportedName} }`
 
+  const fallbackArg = fallbackVar === undefined ? '' : `, ${fallbackVar}`
+
   return `let ${variableName}
-__binder.bind(${objectKey}, ${namespaceVar}, v => { ${variableName} = v }, () => ${variableName}, ${useFallback})
+__binder.bind(${objectKey}, ${namespaceVar}, v => { ${variableName} = v }, () => ${variableName}, ${useFallback}${fallbackArg})
 ${reExportLine}`
 }
 
@@ -254,18 +266,18 @@ ${reExportLine}`
  * before descending into its subtree and removed once that subtree finishes, so
  * it tracks the active path rather than every URL ever visited.
  * @param {Map<string, string>} [params.originNamespaces] Shared registry mapping
- * a defining-module URL to the wrapper namespace alias a same-origin `export *`
- * collision must read it from. Absent until the first such collision; then
- * threaded through the recursion so one defining module yields one alias and
- * {@link buildWrapperSource} imports each once. Only `*`-collided names use it;
+ * a defining-module URL to the wrapper namespace alias its `export *`-sourced
+ * names fall back to. Absent until the first `export *`; then threaded through
+ * the recursion so one defining module yields one alias and
+ * {@link buildWrapperSource} imports each once. Only `*`-sourced names use it;
  * every other export reads from the wrapped module's own `namespace`.
  *
  * @returns {Generator<Array, { setters: Map<string, string>, origins: (Map<string, string> | undefined), originNamespaces: (Map<string, string> | undefined) }>}
  * A generator that yields I/O operations and ultimately returns the shimmed
  * setters for all the exports from the module and any transitive export all
  * modules. `origins` (the defining module per `*`-sourced name) is `undefined`
  * for a module with no `export *`; `originNamespaces` stays `undefined` unless a
- * same-origin `*` collision actually needed an alias.
+ * `*` re-export actually minted an alias.
  */
 function * processModule ({ srcUrl, context, excludeDefault = false, depth = 0, seen, originNamespaces }) {
   const exportNames = yield * getExports(srcUrl, context)
@@ -279,15 +291,13 @@ function * processModule ({ srcUrl, context, excludeDefault = false, depth = 0,
   // and one write per name, not two.
   let starOrigins
 
-  // A name pulled in through more than one `export *` chain that all bottom out
-  // at the same module stays exported (ECMAScript ResolveExport;
-  // tc39/ecma262#3715), but the *aggregate* namespace this wrapper imports drops
-  // it: under iitm the chains are distinct wrapped modules, so Node sees the
-  // re-export as ambiguous and the name reads back undefined. Only those names
-  // must instead read from their defining module's own namespace, which always
-  // holds the value. `originNamespaces` maps such a defining module to the alias
-  // the wrapper imports for it; it is allocated on the first surviving
-  // collision, so a module without one emits no extra import (#171).
+  // Every `export *`-sourced name falls back to its defining module's own
+  // namespace when the aggregate namespace this wrapper imports doesn't hold the
+  // value — because that binding is ambiguous across chains (#171) or still in
+  // its temporal dead zone on the aggregate mid-cycle (#269). `originNamespaces`
+  // maps each defining module to the alias the wrapper imports for it; it is
+  // allocated on the first `export *`, so a module without one emits no extra
+  // import.
   const ensureOriginNamespace = (origin) => {
     originNamespaces ??= new Map()
     let alias = originNamespaces.get(origin)
@@ -300,27 +310,26 @@ function * processModule ({ srcUrl, context, excludeDefault = false, depth = 0,
 
   const addSetter = (name, setter, isStarExport, origin) => {
     if (setters.has(name)) {
-      if (isStarExport) {
-        // `starOrigins.has(name)` means the existing entry also came from a `*`
-        // re-export (an explicit export would not be tracked here).
-        if (starOrigins.has(name)) {
-          if (starOrigins.get(name) === origin) {
-            // The same binding reached through two `*` re-export chains. It
-            // stays exported, but the aggregate namespace dropped it, so point
-            // its setter at the defining module's namespace instead.
-            setters.set(name, buildSetter(name, origin, ensureOriginNamespace(origin)))
-          } else {
-            // Genuinely ambiguous: two `*` re-exports name it from different
-            // modules. Per ResolveExport the name is excluded entirely.
-            setters.delete(name)
-            starOrigins.delete(name)
-          }
+      // `starOrigins.has(name)` means the existing entry also came from a `*`
+      // re-export (an explicit export would not be tracked here).
+      if (isStarExport && starOrigins.has(name)) {
+        if (starOrigins.get(name) !== origin) {
+          // Genuinely ambiguous: two `*` re-exports name it from different
+          // modules. Per ResolveExport the name is excluded entirely.
+          setters.delete(name)
+          starOrigins.delete(name)
         }
-        // An explicit export already shadows the `*` re-export; leave it.
+        // Same binding reached through two `*` chains: it stays exported and the
+        // existing setter already falls back to that defining module, so keep it.
       }
+      // An explicit export already shadows the `*` re-export; leave it.
     } else {
       if (isStarExport) {
         starOrigins.set(name, origin)
+        // Read from the aggregate namespace, falling back to the defining
+        // module (see ensureOriginNamespace) so the value resolves even when the
+        // aggregate drops it or holds it in a temporal dead zone.
+        setter = buildSetter(name, origin, 'namespace', ensureOriginNamespace(origin))
       }
 
       setters.set(name, setter)
@@ -384,9 +393,9 @@ function * processModule ({ srcUrl, context, excludeDefault = false, depth = 0,
         // the whole tree) rather than orphaning the child's into a second Map.
         originNamespaces ??= sub.originNamespaces
 
-        // Star targets build their setters against `namespace` like any other
-        // module; only a surviving same-origin collision (in addSetter) rewrites
-        // the affected name to read from its defining module's alias.
+        // addSetter rebuilds each `*`-sourced name to read from the aggregate
+        // namespace with a fallback to its defining module, so the sub's own
+        // setter string is only a placeholder here.
         for (const [name, setter] of sub.setters) {
           addSetter(name, setter, true, sub.origins?.get(name) ?? result.url)
         }
@@ -650,11 +659,10 @@ export function createHook (meta) {
   // iitm's proxy. Pure string generation shared by the asynchronous and
   // synchronous `load` paths.
   function buildWrapperSource (realUrl, setters, originalSpecifier, originNamespaces) {
-    // The wrapped module imports its namespace as `namespace`, which serves
-    // every export but the ones a same-origin `export *` collision forced onto
-    // their defining module (#171): the aggregate namespace drops those as
-    // ambiguous under iitm, so each such defining module gets its own alias the
-    // wrapper imports. Absent the registry (no such collision) nothing is added.
+    // The wrapped module imports its namespace as `namespace`. Every
+    // `export *`-sourced name additionally falls back to its defining module,
+    // so each such module gets its own alias the wrapper imports here (#171,
+    // #269). Absent the registry (the module has no `export *`) nothing is added.
     let originImports = ''
     if (originNamespaces !== undefined) {
       for (const [originUrl, alias] of originNamespaces) {

lib/register.js

@@ -91,12 +91,27 @@ class ModuleBinder {
    * @param {() => unknown} read Reads the wrapper's local binding.
    * @param {boolean} useFallback Fall back to `source.default` (the synthetic
    * `module.exports` name a builtin does not expose on its ESM namespace).
+   * @param {object} [fallbackSource] The defining module's namespace for a name
+   * re-exported through `export *`. Read when `source` yields `undefined` or is
+   * in its temporal dead zone, so an ambiguous (#171) or mid-cycle (#269)
+   * binding still resolves synchronously instead of only on a later retry.
    * @returns {void}
    */
-  bind (key, source, write, read, useFallback) {
-    const readSource = useFallback
+  bind (key, source, write, read, useFallback, fallbackSource) {
+    const readPrimary = useFallback
       ? () => source[key] ?? source.default
       : () => source[key]
+    const readSource = fallbackSource === undefined
+      ? readPrimary
+      : () => {
+          try {
+            const value = readPrimary()
+            if (value !== undefined) return value
+          } catch (error) {
+            if (!(error instanceof ReferenceError)) throw error
+          }
+          return fallbackSource[key]
+        }
     this.#overridden[key] = false
     let deferred = false
     try {

test/fixtures/star-reexport-tdz-barrel.mjs

@@ -0,0 +1,4 @@
+// A barrel that re-exports the leaf's bindings with `export *`. This is the
+// module whose wrapper snapshots `Foo`/`IsFoo` while they are still in the
+// leaf's temporal dead zone (see star-reexport-tdz-leaf.mjs).
+export * from './star-reexport-tdz-leaf.mjs'

test/fixtures/star-reexport-tdz-enter.mjs

@@ -0,0 +1,9 @@
+// Imported by the top module *before* the barrel, so the leaf is entered (and
+// left on the evaluation stack mid-cycle) ahead of the barrel. This is what
+// forces the barrel to evaluate before the leaf finishes, reproducing the
+// temporal-dead-zone ordering from typebox (issue #269).
+import { Foo } from './star-reexport-tdz-leaf.mjs'
+
+export function useFoo (value) {
+  return Foo(value)
+}

test/fixtures/star-reexport-tdz-leaf.mjs

@@ -0,0 +1,21 @@
+// The leaf that *defines* the exports. It pulls in the barrel that re-exports
+// it (`import * as`), which is what closes the circular `export *` loop:
+// barrel -> leaf -> barrel. Because the barrel is reached again from here, it
+// ends up deeper in the same strongly-connected component than this leaf and
+// therefore evaluates *before* it, so the leaf's bindings are still in their
+// temporal dead zone when the barrel's wrapper snapshots them (issue #269).
+import * as barrel from './star-reexport-tdz-barrel.mjs'
+
+export function Foo (value) {
+  return value
+}
+
+// Reads the barrel namespace back so the circular dependency is genuine (once
+// settled, `barrel.Foo` is this module's own `Foo`).
+export function sameAsBarrel () {
+  return barrel.Foo === Foo
+}
+
+export function IsFoo (value) {
+  return typeof value === 'object'
+}

test/fixtures/star-reexport-tdz-top.mjs

@@ -0,0 +1,6 @@
+// The entry aggregator. `enter` comes first so the leaf is entered before the
+// barrel; the barrel then contributes `Foo`/`IsFoo` through its `export *`.
+// Mirrors typebox's `type/index.js`, which `export *`s several barrels where a
+// deeply cross-imported leaf (`template_literal`) surfaces as `undefined`.
+export * from './star-reexport-tdz-enter.mjs'
+export * from './star-reexport-tdz-barrel.mjs'

test/hook/star-reexport-tdz.mjs

@@ -0,0 +1,19 @@
+import * as top from '../fixtures/star-reexport-tdz-top.mjs'
+import { strictEqual } from 'assert'
+import Hook from '../../index.js'
+
+// Regression test for issue #269 (typebox's exports coming back `undefined`
+// under the loader).
+//
+// `Foo`/`IsFoo` are function declarations defined in the leaf module and
+// re-exported through a barrel with `export *`. A circular `export *` loop
+// (barrel -> leaf -> barrel) makes the barrel's wrapper evaluate while the leaf
+// is still in its temporal dead zone, so the wrapper snapshots the bindings as
+// `undefined`. Without the loader these are always functions (hoisted
+// declarations, live bindings), so the loader must expose them the same way and
+// not lose them to a deferred read that only recovers on a later async retry.
+Hook(() => {})
+
+strictEqual(typeof top.Foo, 'function')
+strictEqual(typeof top.IsFoo, 'function')
+strictEqual(typeof top.useFoo, 'function')

test/low-level/module-binder.mjs

@@ -119,3 +119,71 @@ function makeSlot (initial) {
   const { write, read } = makeSlot(undefined)
   throws(() => binder.bind('foo', source, write, read, false), TypeError)
 }
+
+// A fallbackSource is not consulted while the primary source holds a value.
+{
+  const binder = new ModuleBinder()
+  const source = { foo: 1 }
+  const fallback = { get foo () { throw new Error('must not be read') } }
+  const { slot, write, read } = makeSlot(undefined)
+  binder.bind('foo', source, write, read, false, fallback)
+  strictEqual(slot.value, 1, 'primary value wins over the fallback')
+}
+
+// A primary source without the value (an `export *` name the aggregate
+// namespace dropped as ambiguous) reads the fallbackSource, synchronously.
+{
+  const binder = new ModuleBinder()
+  const source = {}
+  const fallback = { foo: 2 }
+  const { slot, write, read } = makeSlot(undefined)
+  binder.bind('foo', source, write, read, false, fallback)
+  strictEqual(slot.value, 2, 'undefined primary resolved from the fallback at bind time')
+}
+
+// A primary source still in its dead zone reads the fallbackSource,
+// synchronously — not deferred to a retry.
+{
+  const binder = new ModuleBinder()
+  const source = { get foo () { throw new ReferenceError('tdz') } }
+  const fallback = { foo: 3 }
+  const { slot, write, read } = makeSlot(undefined)
+  binder.bind('foo', source, write, read, false, fallback)
+  strictEqual(slot.value, 3, 'TDZ primary resolved from the fallback at bind time')
+}
+
+// A non-ReferenceError from the primary still propagates; the fallback is no
+// license to swallow real errors.
+{
+  const binder = new ModuleBinder()
+  const source = { get foo () { throw new TypeError('boom') } }
+  const fallback = { foo: 4 }
+  const { write, read } = makeSlot(undefined)
+  throws(() => binder.bind('foo', source, write, read, false, fallback), TypeError)
+}
+
+// Both sources in their dead zone still defers to the retry path, then
+// resolves from whichever source becomes live.
+{
+  const binder = new ModuleBinder()
+  let live = false
+  const dead = { get foo () { throw new ReferenceError('tdz') } }
+  const fallback = { get foo () { if (!live) throw new ReferenceError('tdz'); return 5 } }
+  const { slot, write, read } = makeSlot(undefined)
+  binder.bind('foo', dead, write, read, false, fallback)
+  strictEqual(slot.value, undefined, 'deferred while both sources are dead')
+  live = true
+  binder.flush()
+  await Promise.resolve()
+  strictEqual(slot.value, 5, 'resolved on retry once the fallback became live')
+}
+
+// useFallback (the source.default read) composes with a fallbackSource.
+{
+  const binder = new ModuleBinder()
+  const source = {}
+  const fallback = { 'module.exports': 6 }
+  const { slot, write, read } = makeSlot(undefined)
+  binder.bind('module.exports', source, write, read, true, fallback)
+  strictEqual(slot.value, 6, 'fallbackSource read when both named and default are absent')
+}