Skip to content

SyntheticModuleRecord: support a live exports source for namespace reads #380

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 5 commits into
base: main
Choose a base branch
from
Open

SyntheticModuleRecord: support a live exports source for namespace reads #380

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a59646c
SyntheticModuleRecord: support a live exports source for namespace reads
robobun Aug 1, 2026
1ee717b
overrideExportValue: write through to the live-exports source
robobun Aug 1, 2026
3cf0ca7
Fall back to the env slot when the live source lacks the property
robobun Aug 1, 2026
abed2db
Invalidate the module-namespace IC when a live-exports source is inst…
robobun Aug 1, 2026
1b4afe5
setLiveExportsSource: accept null and fire the watchpoint once
robobun Aug 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions Source/JavaScriptCore/runtime/JSModuleNamespaceObject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
#include "JSCInlines.h"
#include "JSModuleEnvironment.h"
#include "JSModuleRecord.h"
#if USE(BUN_JSC_ADDITIONS)
#include "SyntheticModuleRecord.h"
#endif

namespace JSC {

Expand Down Expand Up @@ -195,6 +198,25 @@
return false;
}

#if USE(BUN_JSC_ADDITIONS)
// Bun's mock.module / loader:"object" may back a synthetic module with the
// factory-returned object so that accessor exports stay live. The module
// environment slots still hold the first-read snapshot for static imports
// (which read slots directly), but dynamic import namespace access
// re-evaluates through the source object on every read. Returning a plain
// uncacheable value here keeps the JIT's module-namespace IC (which would
// inline the raw slot) from being installed.
if (auto* synthetic = dynamicDowncast<SyntheticModuleRecord>(exportEntry.moduleRecord.get())) {
if (JSObject* source = synthetic->liveExportsSource()) [[unlikely]] {
slot.disableCaching();
JSValue liveValue = source->get(globalObject, propertyName);
RETURN_IF_EXCEPTION(scope, false);
slot.setValue(this, static_cast<unsigned>(PropertyAttribute::DontDelete), liveValue);
return true;
}

Check failure on line 216 in Source/JavaScriptCore/runtime/JSModuleNamespaceObject.cpp

View check run for this annotation

Claude / Claude Code Review

Live-source branch mishandles re-exports: wrong key + star-namespace 

The live-source lookup uses `propertyName` (the export name on *this* namespace) instead of `exportEntry.localName` (the binding name in the target synthetic module). When another module does `export { foo as bar } from './mocked'`, `propertyName` is `bar` but the source object only has `foo`, so the read returns `undefined` — a regression vs. the environment-slot fallback. Additionally, this branch must be skipped when `exportEntry.localName == vm.propertyNames->starNamespacePrivateName`: for `
Comment on lines +213 to +225

@claude claude Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The live-source lookup uses propertyName (the export name on this namespace) instead of exportEntry.localName (the binding name in the target synthetic module). When another module does export { foo as bar } from './mocked', propertyName is bar but the source object only has foo, so the read returns undefined — a regression vs. the environment-slot fallback. Additionally, this branch must be skipped when exportEntry.localName == vm.propertyNames->starNamespacePrivateName: for export * as X from './mocked' the resolved local name is the private star-namespace symbol, which the user's factory object cannot have, so nsB.X also becomes undefined instead of the mocked module's namespace object.

Extended reasoning...

What the bug is

m_exports on a JSModuleNamespaceObject maps this namespace's export names to ExportEntry { localName, moduleRecord }, where moduleRecord / localName are the resolved binding after following re-export chains (see the constructor, which stores resolution.localName and resolution.moduleRecord). The new live-source branch reads the source object with propertyName — the key on this namespace — instead of exportEntry.localName — the key in the target synthetic module's environment (and thus on its backing object).

For direct access on the synthetic module's own namespace these happen to coincide, because tryCreateWithExportNamesAndValues calls addExportEntry(ExportEntry::createLocal(exportName, exportName)). They diverge as soon as another module re-exports from the mocked module.

Step-by-step: renamed re-export

  1. ./mocked is a SyntheticModuleRecord created via mock.module(id, () => ({ get foo() { ... } })) with a liveExportsSource that has a foo accessor.
  2. Module B contains export { foo as bar } from './mocked'.
  3. resolveExportImpl walks the Indirect entry on B, enqueues (mockedRecord, 'foo'), and resolves at the synthetic module's Local entry to Resolution{ Resolved, moduleRecord: mockedSyntheticRecord, localName: 'foo' }.
  4. B's namespace stores m_exports['bar'] = { localName: 'foo', moduleRecord: mockedSyntheticRecord }.
  5. Reading nsB.bar enters getOwnPropertySlotCommon with propertyName == 'bar' and exportEntry.localName == 'foo'.
  6. dynamicDowncast<SyntheticModuleRecord>(exportEntry.moduleRecord.get()) succeeds, liveExportsSource() is non-null, and the code executes source->get(globalObject, propertyName) → source.bar → undefined.

Before this PR, step 6 fell through to setValueModuleNamespace with the environment slot value (the snapshot of foo), so this is a regression, not merely an incomplete feature. Note that the immediately preceding line already uses the correct key: getValue(environment, exportEntry.localName, scopeOffset).

Step-by-step: export * as X

  1. Module B contains export * as X from './mocked'.
  2. resolveExport / getModuleNamespace produce Resolution{ Resolved, moduleRecord: mockedSyntheticRecord, localName: starNamespacePrivateName } (AbstractModuleRecord.cpp handling for star-namespace bindings).
  3. B's namespace stores m_exports['X'] = { localName: starNamespacePrivateName, moduleRecord: mockedSyntheticRecord }.
  4. Reading nsB.X: the earlier if (exportEntry.localName == starNamespacePrivateName) block materializes the namespace, and getValue(environment, starNamespacePrivateName, ...) correctly fetches the mocked module's JSModuleNamespaceObject from the slot.
  5. But then the live-source branch matches (exportEntry.moduleRecord is the synthetic record with a live source) and calls source->get(globalObject, 'X') on the user's plain factory object → undefined.

Fixing case 1 by switching to exportEntry.localName does not fix case 2: it would call source->get(starNamespacePrivateName), a private symbol the user object cannot have, still yielding undefined.

Why nothing prevents it

The branch is guarded only by dynamicDowncast<SyntheticModuleRecord>(exportEntry.moduleRecord) and liveExportsSource(). It does not check whether this namespace is the synthetic module's own namespace, nor whether the resolved local name is the star-namespace sentinel. Because exportEntry.moduleRecord is the resolved target record (not m_moduleRecord), any namespace whose export chain terminates at the mocked module hits this path.

Impact

Both cases return undefined where the pre-PR code returned the correct value (snapshot for the rename case, the namespace object for the star case). This is user-visible incorrect behavior for anyone who re-exports from a mock.module'd module — a common pattern when mocking a package that is itself re-exported through a barrel file.

Suggested fix

if (auto* synthetic = dynamicDowncast<SyntheticModuleRecord>(exportEntry.moduleRecord.get())) {
    if (JSObject* source = synthetic->liveExportsSource();
        source && exportEntry.localName != vm.propertyNames->starNamespacePrivateName) [[unlikely]] {
        slot.disableCaching();
        JSValue liveValue = source->get(globalObject, exportEntry.localName);
        RETURN_IF_EXCEPTION(scope, false);
        slot.setValue(this, static_cast<unsigned>(PropertyAttribute::DontDelete), liveValue);
        return true;
    }
}

i.e. (a) look up by exportEntry.localName, and (b) skip the branch entirely for the star-namespace sentinel so it falls through to setValueModuleNamespace with the already-correct slot value.

@claude claude Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hasOwn fallback in 3cf0ca7 removes the undefined regression for both the renamed-re-export and export * as X cases (the source lacks bar/X, so we drop through to the env slot), but the lookup key is still propertyName rather than exportEntry.localName, so the underlying wrong-key issue remains:

  • Live behavior is lost through a rename. export { foo as bar } from './mocked' → nsB.bar misses on source.bar and returns the env-slot snapshot, while nsMocked.foo returns the live value. The feature silently stops working across a barrel/re-export.
  • Wrong value when the outer name collides. If the mocked module also exports bar, the source does own bar, so hasOwn is true and nsB.bar returns source.bar — but the resolved binding is foo. That's a correctness bug, not just a stale snapshot.

Switching both getOwnPropertySlot and getValue calls to exportEntry.localName fixes both. With that change the hasOwn fallback also happens to cover the star-namespace case (the source can't own starNamespacePrivateName), so the explicit sentinel guard from the original suggestion becomes optional — though keeping it saves a pointless method-table dispatch on every export * as X read.

}
#endif

slot.setValueModuleNamespace(this, static_cast<unsigned>(PropertyAttribute::DontDelete), value, environment, scopeOffset);
return true;
}
Expand Down
20 changes: 20 additions & 0 deletions Source/JavaScriptCore/runtime/SyntheticModuleRecord.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ void SyntheticModuleRecord::visitChildrenImpl(JSCell* cell, Visitor& visitor)
SyntheticModuleRecord* thisObject = uncheckedDowncast<SyntheticModuleRecord>(cell);
ASSERT_GC_OBJECT_INHERITS(thisObject, info());
Base::visitChildren(thisObject, visitor);
#if USE(BUN_JSC_ADDITIONS)
visitor.append(thisObject->m_liveExportsSource);
#endif
}

DEFINE_VISIT_CHILDREN(SyntheticModuleRecord);
Expand All @@ -92,7 +95,19 @@ SyntheticModuleRecord* SyntheticModuleRecord::tryCreateWithExportNamesAndValues(
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

#if USE(BUN_JSC_ADDITIONS)
// A trailing value with no matching name carries the live-exports backing
// object (Bun's mock.module / loader:"object" path).
JSObject* liveExportsSource = nullptr;
if (exportValues.size() == exportNames.size() + 1) {
JSValue extra = exportValues.at(exportNames.size());
if (extra.isObject())
liveExportsSource = asObject(extra);
}
ASSERT(exportNames.size() == exportValues.size() || liveExportsSource);
#else
ASSERT(exportNames.size() == exportValues.size());
#endif

auto* moduleRecord = create(globalObject, vm, globalObject->syntheticModuleRecordStructure(), moduleKey);
SymbolTable* exportSymbolTable = SymbolTable::create(vm);
Expand Down Expand Up @@ -121,6 +136,11 @@ SyntheticModuleRecord* SyntheticModuleRecord::tryCreateWithExportNamesAndValues(
ASSERT(putResult);
}

#if USE(BUN_JSC_ADDITIONS)
if (liveExportsSource)
moduleRecord->setLiveExportsSource(vm, liveExportsSource);
#endif

return moduleRecord;

}
Expand Down
9 changes: 9 additions & 0 deletions Source/JavaScriptCore/runtime/SyntheticModuleRecord.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,21 @@ class SyntheticModuleRecord final : public AbstractModuleRecord {

JS_EXPORT_PRIVATE static SyntheticModuleRecord* tryCreateWithExportNamesAndValues(JSGlobalObject*, const Identifier& moduleKey, const Vector<Identifier, 4>& exportNames, const MarkedArgumentBuffer& exportValues);

#if USE(BUN_JSC_ADDITIONS)
JSObject* liveExportsSource() const { return m_liveExportsSource.get(); }
void setLiveExportsSource(VM& vm, JSObject* source) { m_liveExportsSource.set(vm, this, source); }
#endif

private:
SyntheticModuleRecord(VM&, Structure*, const Identifier& moduleKey);

static SyntheticModuleRecord* tryCreateDefaultExportSyntheticModule(JSGlobalObject*, const Identifier& moduleKey, JSValue);

void finishCreation(JSGlobalObject*, VM&);

#if USE(BUN_JSC_ADDITIONS)
WriteBarrier<JSObject> m_liveExportsSource;
#endif
};

} // namespace JSC
Loading