Correctly handle __esModule for loader: "object" (#16885)

Co-authored-by: Jarred-Sumner <[email protected]>
oven-sh · Jan 31, 2025 · 9acb72d · 9acb72d
1 parent 25f6cbd
commit 9acb72d
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 7 deletions.
diff --git 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<JSPromise*>(handleVirtualModuleResult<true>(globalObject, virtualModuleResult, res, specifier, referrer, wasModuleMock));
+            JSValue promiseOrCommonJSModule = handleVirtualModuleResult<true>(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<JSPromise*>(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<JSPromise*>(handleVirtualModuleResult<true>(globalObject, virtualModuleResult, res, specifier, referrer, wasModuleMock));
+            JSValue promiseOrCommonJSModule = handleVirtualModuleResult<true>(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<JSPromise*>(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
@@ -11,13 +11,14 @@ generateObjectModuleSourceCode(JSC::JSGlobalObject* globalObject,
                Vector<JSC::Identifier, 4>& exportNames,
                JSC::MarkedArgumentBuffer& exportValues) -> void {
         auto& vm = JSC::getVM(lexicalGlobalObject);
-        GlobalObject* globalObject = reinterpret_cast<GlobalObject*>(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
@@ -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
@@ -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");
+});