Fixes #1288 (#11991)

src/bun.js/bindings/BunProcess.cpp

@@ -3,11 +3,13 @@
 #include <JavaScriptCore/JSMicrotask.h>
 #include <JavaScriptCore/ObjectConstructor.h>
 #include <JavaScriptCore/NumberPrototype.h>
+#include "CommonJSModuleRecord.h"
 #include "JavaScriptCore/CatchScope.h"
 #include "JavaScriptCore/JSCJSValue.h"
 #include "JavaScriptCore/JSCast.h"
 #include "JavaScriptCore/JSString.h"
 #include "JavaScriptCore/Protect.h"
+#include "JavaScriptCore/PutPropertySlot.h"
 #include "ScriptExecutionContext.h"
 #include "headers-handwritten.h"
 #include "node_api.h"
@@ -285,10 +287,14 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen,
     }
 
     globalObject->pendingNapiModule = exports;
+    Strong<JSC::Unknown> strongExports;
+
     if (exports.isCell()) {
-        vm.writeBarrier(globalObject, exports.asCell());
+        strongExports = { vm, exports.asCell() };
     }
 
+    Strong<JSC::JSObject> strongModule = { vm, moduleObject };
+
     WTF::String filename = callFrame->uncheckedArgument(1).toWTFString(globalObject);
     if (filename.isEmpty()) {
         JSC::throwTypeError(globalObject, scope, "dlopen requires a non-empty string as the second argument"_s);
@@ -344,17 +350,20 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen,
     }
 
     if (callCountAtStart != globalObject->napiModuleRegisterCallCount) {
-        JSValue pendingModule = globalObject->pendingNapiModule;
+        JSValue resultValue = globalObject->pendingNapiModule;
         globalObject->pendingNapiModule = JSValue {};
         globalObject->napiModuleRegisterCallCount = 0;
 
-        if (pendingModule) {
-            if (pendingModule.isCell() && pendingModule.getObject()->isErrorInstance()) {
-                JSC::throwException(globalObject, scope, pendingModule);
+        RETURN_IF_EXCEPTION(scope, {});
+
+        if (resultValue && resultValue != strongModule.get()) {
+            if (resultValue.isCell() && resultValue.getObject()->isErrorInstance()) {
+                JSC::throwException(globalObject, scope, resultValue);
                 return JSC::JSValue::encode(JSC::JSValue {});
             }
-            return JSC::JSValue::encode(pendingModule);
         }
+
+        return JSValue::encode(jsUndefined());
     }
 
     JSC::EncodedJSValue (*napi_register_module_v1)(JSC::JSGlobalObject* globalObject,
@@ -381,7 +390,19 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen,
         return JSC::JSValue::encode(JSC::JSValue {});
     }
 
-    return napi_register_module_v1(globalObject, JSC::JSValue::encode(exports));
+    EncodedJSValue exportsValue = JSC::JSValue::encode(exports);
+    JSC::JSValue resultValue = JSValue::decode(napi_register_module_v1(globalObject, exportsValue));
+
+    RETURN_IF_EXCEPTION(scope, {});
+
+    // https://github.com/nodejs/node/blob/2eff28fb7a93d3f672f80b582f664a7c701569fb/src/node_api.cc#L734-L742
+    // https://github.com/oven-sh/bun/issues/1288
+    if (!resultValue.isEmpty() && !scope.exception() && (!strongExports || resultValue != strongExports.get())) {
+        PutPropertySlot slot(strongModule.get(), false);
+        strongModule->put(strongModule.get(), globalObject, builtinNames(vm).exportsPublicName(), resultValue, slot);
+    }
+
+    return JSValue::encode(resultValue);
 }
 
 JSC_DEFINE_HOST_FUNCTION(Process_functionUmask,

src/bun.js/bindings/napi.cpp

@@ -1,5 +1,3 @@
-
-
 #include "node_api.h"
 #include "root.h"
 
@@ -65,6 +63,7 @@
 #include "wtf/NakedPtr.h"
 #include <JavaScriptCore/JSArrayBuffer.h>
 #include <JavaScriptCore/FunctionPrototype.h>
+#include "CommonJSModuleRecord.h"
 
 // #include <iostream>
 using namespace JSC;
@@ -877,16 +876,31 @@ extern "C" void napi_module_register(napi_module* mod)
     JSObject* object = (pendingNapiModule && pendingNapiModule.isObject()) ? pendingNapiModule.getObject()
                                                                            : nullptr;
 
+    auto scope = DECLARE_THROW_SCOPE(vm);
+    JSC::Strong<JSC::JSObject> strongExportsObject;
+
     if (!object) {
-        object = JSC::constructEmptyObject(globalObject);
+        auto* exportsObject = JSC::constructEmptyObject(globalObject);
+        RETURN_IF_EXCEPTION(scope, void());
+
+        object = Bun::JSCommonJSModule::create(globalObject, keyStr, exportsObject, false, jsUndefined());
+        strongExportsObject = { vm, exportsObject };
     } else {
         globalObject->pendingNapiModule = JSC::JSValue();
+        JSValue exportsObject = object->getIfPropertyExists(globalObject, WebCore::builtinNames(vm).exportsPublicName());
+        RETURN_IF_EXCEPTION(scope, void());
+
+        if (exportsObject && exportsObject.isObject()) {
+            strongExportsObject = { vm, exportsObject.getObject() };
+        }
     }
 
-    EnsureStillAliveScope ensureAlive(object);
+    JSC::Strong<JSC::JSObject> strongObject = { vm, object };
+
     JSValue resultValue = toJS(mod->nm_register_func(toNapi(globalObject), toNapi(object)));
 
-    EnsureStillAliveScope ensureAlive2(resultValue);
+    RETURN_IF_EXCEPTION(scope, void());
+
     if (resultValue.isEmpty()) {
         JSValue errorInstance = createError(globalObject, makeString("Node-API module \""_s, keyStr, "\" returned an error"_s));
         globalObject->pendingNapiModule = errorInstance;
@@ -903,19 +917,14 @@ extern "C" void napi_module_register(napi_module* mod)
         return;
     }
 
-    // std::cout << "loaded " << mod->nm_modname << std::endl;
-
-    auto source = JSC::SourceCode(
-        JSC::SyntheticSourceProvider::create(generateObjectModuleSourceCode(
-                                                 globalObject,
-                                                 object),
-            JSC::SourceOrigin(), keyStr));
-
-    // Add it to the ESM registry
-    globalObject->moduleLoader()->provideFetch(globalObject, JSC::jsString(vm, WTFMove(keyStr)), WTFMove(source));
+    // https://github.com/nodejs/node/blob/2eff28fb7a93d3f672f80b582f664a7c701569fb/src/node_api.cc#L734-L742
+    // https://github.com/oven-sh/bun/issues/1288
+    if (!scope.exception() && strongExportsObject && strongExportsObject.get() != resultValue) {
+        PutPropertySlot slot(strongObject.get(), false);
+        strongObject->put(strongObject.get(), globalObject, WebCore::builtinNames(vm).exportsPublicName(), resultValue, slot);
+    }
 
     globalObject->pendingNapiModule = object;
-    vm.writeBarrier(globalObject, object);
 }
 
 extern "C" napi_status napi_wrap(napi_env env,
@@ -1063,6 +1072,7 @@ extern "C" napi_status napi_create_function(napi_env env, const char* utf8name,
     auto method = reinterpret_cast<Zig::FFIFunction>(cb);
     auto* function = NAPIFunction::create(vm, globalObject, length, name, method, data);
 
+    ASSERT(function->isCallable());
     *result = toNapi(JSC::JSValue(function));
 
     return napi_ok;

src/symbols.txt

@@ -50,6 +50,7 @@ _napi_create_uint32
 _napi_define_class
 _napi_define_properties
 _napi_delete_async_work
+_napi_delete_element
 _napi_delete_property
 _napi_delete_reference
 _napi_detach_arraybuffer
@@ -66,7 +67,6 @@ _napi_get_cb_info
 _napi_get_dataview_info
 _napi_get_date_value
 _napi_get_element
-_napi_delete_element
 _napi_get_global
 _napi_get_instance_data
 _napi_get_last_error_info
@@ -143,11 +143,11 @@ _napi_typeof
 _napi_unref_threadsafe_function
 _napi_unwrap
 _napi_wrap
+_node_api_create_external_string_latin1
+_node_api_create_external_string_utf16
 _node_api_create_syntax_error
 _node_api_symbol_for
 _node_api_throw_syntax_error
-_node_api_create_external_string_latin1
-_node_api_create_external_string_utf16
 __ZN2v87Isolate10GetCurrentEv
 __ZN2v87Isolate13TryGetCurrentEv
 __ZN2v87Isolate17GetCurrentContextEv

test/napi/napi-app/main.cpp

@@ -107,11 +107,26 @@ test_napi_get_value_string_utf8_with_buffer(const Napi::CallbackInfo &info) {
   return ok(env);
 }
 
-Napi::Object InitAll(Napi::Env env, Napi::Object exports) {
+Napi::Value RunCallback(const Napi::CallbackInfo &info) {
+  Napi::Env env = info.Env();
+  Napi::Function cb = info[0].As<Napi::Function>();
+  return cb.Call(env.Global(), {Napi::String::New(env, "hello world")});
+}
+
+Napi::Object Init2(Napi::Env env, Napi::Object exports) {
+  return Napi::Function::New(env, RunCallback);
+}
+
+Napi::Object InitAll(Napi::Env env, Napi::Object exports1) {
   // check that these symbols are defined
   auto *isolate = v8::Isolate::GetCurrent();
-  node::AddEnvironmentCleanupHook(isolate, [](void *) {}, isolate);
-  node::RemoveEnvironmentCleanupHook(isolate, [](void *) {}, isolate);
+
+  Napi::Object exports = Init2(env, exports1);
+
+  node::AddEnvironmentCleanupHook(
+      isolate, [](void *) {}, isolate);
+  node::RemoveEnvironmentCleanupHook(
+      isolate, [](void *) {}, isolate);
 
   exports.Set("test_issue_7685", Napi::Function::New(env, test_issue_7685));
   exports.Set("test_issue_11949", Napi::Function::New(env, test_issue_11949));

test/napi/napi-app/main.js

@@ -1,4 +1,12 @@
 const tests = require("./build/Release/napitests.node");
+if (process.argv[2] === "self") {
+  console.log(
+    tests(function (str) {
+      return str + "!";
+    }),
+  );
+  process.exit(0);
+}
 const fn = tests[process.argv[2]];
 if (typeof fn !== "function") {
   throw new Error("Unknown test:", process.argv[2]);

test/napi/napi.test.ts

@@ -58,6 +58,11 @@ describe("napi", () => {
       expect(result).toEndWith("str:");
     });
   });
+
+  it("#1288", async () => {
+    const result = checkSameOutput("self", []);
+    expect(result).toBe("hello world!");
+  });
 });
 
 function checkSameOutput(test: string, args: any[]) {