From 9acb72d2adeac5f72d185aeb8ebb6c811e0b0277 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Fri, 31 Jan 2025 05:24:33 -0800 Subject: [PATCH] Correctly handle `__esModule` for `loader: "object"` (#16885) Co-authored-by: Jarred-Sumner <709451+Jarred-Sumner@users.noreply.github.com> --- src/bun.js/bindings/ModuleLoader.cpp | 39 +++++++++++++++++++++++++--- src/bun.js/modules/ObjectModule.cpp | 7 ++--- test/js/bun/plugin/module-plugins.ts | 19 ++++++++++++++ test/js/bun/plugin/plugins.test.ts | 24 +++++++++++++++++ 4 files changed, 82 insertions(+), 7 deletions(-) diff --git a/src/bun.js/bindings/ModuleLoader.cpp b/src/bun.js/bindings/ModuleLoader.cpp index 87b639063b87e8..df7aec6f5627fb 100644 --- a/src/bun.js/bindings/ModuleLoader.cpp +++ b/src/bun.js/bindings/ModuleLoader.cpp @@ -1,4 +1,5 @@ #include "root.h" + #include "headers-handwritten.h" #include "JavaScriptCore/JSGlobalObject.h" #include "ModuleLoader.h" @@ -35,7 +36,7 @@ #include "NativeModuleImpl.h" #include "../modules/ObjectModule.h" -#include "wtf/Assertions.h" +#include "CommonJSModuleRecord.h" namespace Bun { using namespace JSC; @@ -302,7 +303,8 @@ static JSValue handleVirtualModuleResult( ErrorableResolvedSource* res, BunString* specifier, BunString* referrer, - bool wasModuleMock = false) + bool wasModuleMock = false, + JSCommonJSModule* commonJSModule = nullptr) { auto onLoadResult = handleOnLoadResult(globalObject, virtualModuleResult, specifier, wasModuleMock); auto& vm = JSC::getVM(globalObject); @@ -364,6 +366,21 @@ static JSValue handleVirtualModuleResult( case OnLoadResultTypeObject: { JSC::JSObject* object = onLoadResult.value.object.getObject(); + if (commonJSModule) { + const auto& __esModuleIdentifier = vm.propertyNames->__esModule; + JSValue esModuleValue = object->getIfPropertyExists(globalObject, __esModuleIdentifier); + RETURN_IF_EXCEPTION(scope, {}); + if (esModuleValue && esModuleValue.toBoolean(globalObject)) { + JSValue defaultValue = object->getIfPropertyExists(globalObject, vm.propertyNames->defaultKeyword); + RETURN_IF_EXCEPTION(scope, {}); + if (defaultValue && !defaultValue.isUndefined()) { + commonJSModule->setExportsObject(defaultValue); + commonJSModule->hasEvaluated = true; + return commonJSModule; + } + } + } + JSC::ensureStillAliveHere(object); auto function = generateObjectModuleSourceCode( globalObject, @@ -479,7 +496,14 @@ JSValue fetchCommonJSModule( // This is important for being able to trivially mock things like the filesystem. if (isBunTest) { if (JSC::JSValue virtualModuleResult = Bun::runVirtualModule(globalObject, specifier, wasModuleMock)) { - JSPromise* promise = jsCast(handleVirtualModuleResult(globalObject, virtualModuleResult, res, specifier, referrer, wasModuleMock)); + JSValue promiseOrCommonJSModule = handleVirtualModuleResult(globalObject, virtualModuleResult, res, specifier, referrer, wasModuleMock, target); + RETURN_IF_EXCEPTION(scope, {}); + + // If we assigned module.exports to the virtual module, we're done here. + if (promiseOrCommonJSModule == target) { + RELEASE_AND_RETURN(scope, target); + } + JSPromise* promise = jsCast(promiseOrCommonJSModule); switch (promise->status(vm)) { case JSPromise::Status::Rejected: { uint32_t promiseFlags = promise->internalField(JSPromise::Field::Flags).get().asUInt32AsAnyInt(); @@ -561,7 +585,14 @@ JSValue fetchCommonJSModule( // When "bun test" is NOT enabled, disable users from overriding builtin modules if (!isBunTest) { if (JSC::JSValue virtualModuleResult = Bun::runVirtualModule(globalObject, specifier, wasModuleMock)) { - JSPromise* promise = jsCast(handleVirtualModuleResult(globalObject, virtualModuleResult, res, specifier, referrer, wasModuleMock)); + JSValue promiseOrCommonJSModule = handleVirtualModuleResult(globalObject, virtualModuleResult, res, specifier, referrer, wasModuleMock, target); + RETURN_IF_EXCEPTION(scope, {}); + + // If we assigned module.exports to the virtual module, we're done here. + if (promiseOrCommonJSModule == target) { + RELEASE_AND_RETURN(scope, target); + } + JSPromise* promise = jsCast(promiseOrCommonJSModule); switch (promise->status(vm)) { case JSPromise::Status::Rejected: { uint32_t promiseFlags = promise->internalField(JSPromise::Field::Flags).get().asUInt32AsAnyInt(); diff --git a/src/bun.js/modules/ObjectModule.cpp b/src/bun.js/modules/ObjectModule.cpp index 62ffa5da83f444..f4c363d43e0acd 100644 --- a/src/bun.js/modules/ObjectModule.cpp +++ b/src/bun.js/modules/ObjectModule.cpp @@ -11,13 +11,14 @@ generateObjectModuleSourceCode(JSC::JSGlobalObject* globalObject, Vector& exportNames, JSC::MarkedArgumentBuffer& exportValues) -> void { auto& vm = JSC::getVM(lexicalGlobalObject); - GlobalObject* globalObject = reinterpret_cast(lexicalGlobalObject); + auto throwScope = DECLARE_THROW_SCOPE(vm); + GlobalObject* globalObject = defaultGlobalObject(lexicalGlobalObject); JSC::EnsureStillAliveScope stillAlive(object); PropertyNameArray properties(vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude); - object->getPropertyNames(globalObject, properties, - DontEnumPropertiesMode::Exclude); + object->methodTable()->getOwnPropertyNames(object, globalObject, properties, DontEnumPropertiesMode::Exclude); + RETURN_IF_EXCEPTION(throwScope, void()); gcUnprotectNullTolerant(object); for (auto& entry : properties) { diff --git a/test/js/bun/plugin/module-plugins.ts b/test/js/bun/plugin/module-plugins.ts index d6034c5dfcb096..965da7b3b7b25c 100644 --- a/test/js/bun/plugin/module-plugins.ts +++ b/test/js/bun/plugin/module-plugins.ts @@ -22,6 +22,25 @@ plugin({ }; }); + builder.module("my-virtual-module-with-__esModule", () => { + return { + exports: { + default: "world", + __esModule: true, + }, + loader: "object", + }; + }); + + builder.module("my-virtual-module-with-default", () => { + return { + exports: { + default: "world", + }, + loader: "object", + }; + }); + builder.onLoad({ filter: /.*/, namespace: "rejected-promise" }, async ({ path }) => { throw new Error("Rejected Promise"); }); diff --git a/test/js/bun/plugin/plugins.test.ts b/test/js/bun/plugin/plugins.test.ts index 2da1afa169f98d..a6f57b742548ec 100644 --- a/test/js/bun/plugin/plugins.test.ts +++ b/test/js/bun/plugin/plugins.test.ts @@ -484,3 +484,27 @@ describe("errors", () => { expect(text).toBe(result); }); }); + +it("require(...).default without __esModule", () => { + { + const { default: mod } = require("my-virtual-module-with-default"); + expect(mod).toBe("world"); + } +}); + +it("require(...) with __esModule", () => { + { + const mod = require("my-virtual-module-with-__esModule"); + expect(mod).toBe("world"); + } +}); + +it("import(...) with __esModule", async () => { + const { default: mod } = await import("my-virtual-module-with-__esModule"); + expect(mod).toBe("world"); +}); + +it("import(...) without __esModule", async () => { + const { default: mod } = await import("my-virtual-module-with-default"); + expect(mod).toBe("world"); +});