Fix export * as X from './self' in scope-hoisted modules#93192
Merged
Fix export * as X from './self' in scope-hoisted modules#93192
export * as X from './self' in scope-hoisted modules#93192Conversation
sokra
commented
Apr 24, 2026
sokra
commented
Apr 24, 2026
sokra
force-pushed
the
sokra/reexport-self
branch
from
ffeef7d to
730de91
Compare
github-actions
Bot
added
created-by: Turbopack team
Contributor
Stats from current PR✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📎 Tarball URLCommit: 4692cbd |
Contributor
Tests PassedCommit: 4692cbd |
mischnic
reviewed
Apr 28, 2026
mischnic
approved these changes
Apr 28, 2026
Expands the existing `self-reexport-star` test and adds a `self-reexport-star-no-hoisting` variant (scopeHoisting disabled) that covers the same scenarios. Tests verify that the self re-exported namespace is the same object as the module's own namespace, supports recursive `Data.Data.Data` access, can be re-exported through a chained module, and enumerates all expected keys. Co-Authored-By: Claude <noreply@anthropic.com>
Replace copied data.js/reexport.js/index.js in the no-hoisting variant with a single index.js that imports the sibling self-reexport-star entry point. Also expand both tests with the failing `import * as Self` scenario and additional assertions. Co-Authored-By: Claude <noreply@anthropic.com>
`export * as X from './self'` compiled as an in-group namespace
re-export produced a `var __TURBOPACK__imported__module__<esm> =
__turbopack_context__.i("<esm> <export * as X>")`. That declaration
aliased the synthesized rename module's id to the outer module's
namespace variable, but the rename module's factory was not registered
when its exposure was `MergeableModuleExposure::None`. The runtime
lookup failed with "module factory is not available".
Two changes:
- In `ReferencedAsset::get_ident_inner`, stop recursing into
`EsmExport::ImportedNamespace` when the target is in the same scope
hoisting group. The outer asset is the correct module to import; the
inner asset can be a different module (e.g. a rename module) whose
chunk-item id doesn't match the emitted `namespace_ident`.
- Expose intra-group referenced modules as at least `Internal` and
register their factories in `additional_ids`. Intra-group references
via `__turbopack_context__.i(id)` require the runtime lookup to find
the target id.
Co-Authored-By: Claude <noreply@anthropic.com>
Reworks the previous fix which over-exposed modules. Instead of
promoting every intra-group referenced module to
`MergeableModuleExposure::Internal` and including all `Internal`
modules in `additional_ids` (which updated four unrelated snapshots),
only modules that opt in via a new `MergeableModule::requires_namespace_exposure`
hook are promoted — and they are promoted all the way to `External` so
they appear in `additional_ids` without expanding the set of modules
exposed through that mechanism.
- Add `MergeableModule::requires_namespace_exposure` (default `false`).
- Implement it returning `true` for `EcmascriptModuleRenameModule`,
since its emitted code always contains a
`__turbopack_context__.i(<self id>)` lookup.
- In `compute_merged_modules`, promote intra-group referenced modules
that opt in to `External` exposure (wrapped in an `async {}.instrument(...)`
block to keep the future `Send` for `turbo_tasks::function`).
- Revert the `additional_ids` widening in `lib.rs` that included
`Internal` modules.
- Keep the minimal `ReferencedAsset::get_ident_inner` change for
`EsmExport::ImportedNamespace` so the `namespace_ident` matches the
outer asset's chunk item id used by callers when emitting
`var <ident> = __turbopack_context__.i(id)`.
Restores the four snapshot files touched by the previous fix to their
canary state. No other snapshots change. 213 execution and 89 snapshot
tests pass.
Co-Authored-By: Claude <noreply@anthropic.com>
… and "Fix intra-group namespace re-exports in scope hoisting" Both fixes addressed a symptom (module factory lookup failure for `export * as X from './self'`) instead of the root cause. Investigation showed the actual issue is in `ReferencedAsset::get_ident_inner` / `EsmAssetReference::code_generation`: the emitted `var <namespace_ident> = __turbopack_context__.i(<id>)` uses the inner module's ident as the variable name but the immediately-referenced (rename) module's id for the runtime lookup, causing the variable to hold the rename module's namespace instead of the inner module's. The previous fixes papered over this by exposing the rename module's factory; this revert restores the branch to a clean state so the real fix can be made narrowly in `get_ident_inner` / `code_generation`. Keeps the test coverage added in 2000ea7 and 64922a3. The self-reexport tests still fail at this point; fix will follow in a separate commit. Co-Authored-By: Claude <noreply@anthropic.com>
For `Self.Data` where `Self = import * as Self from './inner'` and `Data`
is itself exposed through `export * as Data from './inner'`, the
namespace-member-access optimization rewrites the reference to a named
import resolving to a rename module (`./inner <export * as Data>`).
`ReferencedAsset::get_ident_inner` then recurses through the rename's
`EsmExport::ImportedNamespace("Data")` and returns a `namespace_ident`
derived from the inner module's chunk-item id — while `code_generation`
initializes that same variable with `__turbopack_context__.i(<id>)`
using the directly-referenced (rename) module's id. The variable ends
up holding `ns(rename) = { Data: ns(inner) }` instead of `ns(inner)`,
and because the non-optimized `import * as Self` uses the same mangled
name, it sees the wrong namespace and `Self.foo()` is `undefined`.
Track the target module alongside the namespace ident in
`ReferencedAssetIdent::Module::import_module` and use its chunk-item id
for the `.i(...)` call, so the variable and its initializer refer to
the same module.
No additional module exposure is needed: the rename module is never
referenced at runtime, and no snapshots change.
Co-Authored-By: Claude <noreply@anthropic.com>
In the `ReferencedAsset::Some` arm, use `import_module` from the `ReferencedAssetIdent::Module` we're emitting, instead of pulling the chunk item id from `referenced_asset` and then conditionally overriding it. Keep the directly-referenced `asset` only for the hoisted-statement dedup key — two refs to the same `import_module` via different paths (direct vs. re-export rename) may arrive with different syntax contexts and must emit separate `var` declarations for AST merging to rename. Co-Authored-By: Claude <noreply@anthropic.com>
Previously `ReferencedAssetIdent::Module::import_module` was
`Option<ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>>`, and the
emitting `EsmAssetReference::code_generation` then re-inspected
`referenced_asset` to decide between module / external-esm /
external-commonjs paths. That made the emission dependent on the
directly-referenced asset, not the ident — which was exactly what the
self-reexport bug required we stop doing.
Replace the field with an `ImportSource` enum whose `Module(ModuleId)`
variant carries the chunk-item id directly, and whose `External {
request, ty, import_externals }` variant carries everything needed to
emit `__turbopack_external_import` / `__turbopack_external_require`
without re-consulting the original `ReferencedAsset`. Plumb
`import_externals` through `ReferencedAsset::get_ident`; non-emitting
callers pass `false` with a comment explaining the field is unread.
In the emit block, destructure the ident directly in the outer pattern
and dispatch on `ImportSource` — `referenced_asset` is no longer used
past `get_ident`.
Co-Authored-By: Claude <noreply@anthropic.com>
The emit site for `ReferencedAssetIdent::Module` in `EsmAssetReference::code_generation` already reads `this.import_externals` from the surrounding reference. Carrying it on the ident duplicated that information and forced every non-emitting `get_ident` caller to pass an unused `bool`. Remove the field and drop the `import_externals` parameter from `ReferencedAsset::get_ident` / `get_ident_inner`; the ESM-external branch in `code_generation` now reads the local `import_externals` binding directly. Co-Authored-By: Claude <noreply@anthropic.com>
Store the imported module's `asset` on `ImportSource::Module` (instead of its `ModuleId`) and the namespace variable name on each variant, exposed via `ImportSource::get_namespace_ident()`. The field on `ReferencedAssetIdent::Module` is renamed `import_module` -> `import_source`. The chunk item id is computed on demand at the emit site.
Drop the stored `namespace_ident` from `ImportSource` variants and compute it on demand in `ImportSource::get_namespace_ident`, which now takes a `ChunkingContext` (needed to resolve the module id for `ImportSource::Module`). `ReferencedAssetIdent::Module` keeps a cached `namespace_ident` populated at resolution time so sync visitor closures don't need async access.
Instead of storing a generic Module and upcasting, store the EcmascriptChunkPlaceable directly. This simplifies the code and allows direct access to chunk_item_id without needing a separate hoist_key variable.
Prevents incorrect deduplication when multiple merged modules import the same target but have different syntax contexts, which would cause hygiene to rename one of them.
sokra
force-pushed
the
sokra/reexport-self
branch
from
c895f99 to
4692cbd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Scope-hoisted execution of a module that re-exports its own namespace
(
export * as X from './self') returned the wrong namespace for namedimports of the module.
Given:
Any binding imported from
./datathat relied on the module's ownnamespace (e.g.
Self.foo,Data.foo,Data.Data.foo) wasundefinedat runtime. Without scope hoisting the same code worked correctly.
This PR fixes the bug and adds execution tests covering self-namespace
re-exports — with and without scope hoisting — including chained
re-exports (
Data.Data.foo,Data.Data.Data.bar).Why?
For an access like
Self.DatawhereSelf = import * as Self from './data'and
Datais exposed throughexport * as Data from './data', thenamespace-member-access optimization rewrites the reference to a named
import resolving to a synthesized rename module
(
./data <export * as Data>).ReferencedAsset::get_ident_innerthenrecurses through the rename's
EsmExport::ImportedNamespace("Data")andreturns a
namespace_identderived from the inner module's chunk-itemid — but
EsmAssetReference::code_generationindependently tookid = referenced_asset.chunk_item_idand emittedso the variable named like
ns(data.js)actually heldns(rename) = { Data: ns(data.js) }. The non-optimizedimport * as Selfuses the same mangled name and sees the rename'snamespace, so
Self.foo()evaluates toundefined.How?
Keep the variable name and the
.i(...)argument consistent by movingthe "what to import" decision onto the ident itself:
ReferencedAssetIdent::Modulegains animport_source: ImportSourcefield that describes what to import to populate the namespace variable.
ImportSourceis an enum:Module { asset }— carries a reference to the final module in anyre-export chain, from which the chunk-item id is lazily computed.
External { request, ty }— carries everything needed to emit__turbopack_external_import/__turbopack_external_require.namespace_identis cached inReferencedAssetIdent::Moduleatresolution time (computed via
ImportSource::get_namespace_ident())so downstream sync visitors can read it without re-entering the async
layer.
ReferencedAsset::get_ident/get_ident_innerpopulate the field.For in-group re-exports the inner module propagates up; for external
references the
Externalvariant is used.EsmAssetReference::code_generationdestructuresReferencedAssetIdent::Module { namespace_ident, ctxt, import_source, .. }and dispatches purely on
import_source; it no longer readsreferenced_assetafter theget_identcall. The hoisted-statementdedup key still uses the directly-referenced asset's id, so two
references that happen to resolve to the same inner module via
different paths (e.g. direct vs. through a rename) still emit
separate
vardeclarations for AST merging to rename.__turbopack_external_importvs.__turbopack_external_require) stays where it was — the emit sitereads
self.import_externalsfrom the surroundingEsmAssetReference, soImportSource::Externaldoes not carry it.No additional
MergeableModuleExposureoradditional_idschanges areneeded. The rename module is never referenced at runtime; no snapshot
files change.
Tests
turbopack/crates/turbopack-tests/tests/execution/turbopack/exports/self-reexport-star/— scope-hoisted execution test covering self-namespace re-exports,
nested access (
Data.Data.foo,Data.Data.Data.bar), chainedre-exports through another module, and namespace key enumeration.
turbopack/crates/turbopack-tests/tests/execution/turbopack/exports/self-reexport-star-no-hoisting/— same test cases, run with scope hoisting disabled, reusing the
fixtures from the sibling directory.
Verified by
cargo test --test execution(213 passed) andcargo test --test snapshot(89 passed) inturbopack-tests. Nosnapshot files are modified.
Closes NEXT-
Fixes #