diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index 71d7b8b311..870e15a959 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -3,11 +3,13 @@ #include #include #include +#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 strongExports; + if (exports.isCell()) { - vm.writeBarrier(globalObject, exports.asCell()); + strongExports = { vm, exports.asCell() }; } + Strong 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, diff --git a/src/bun.js/bindings/napi.cpp b/src/bun.js/bindings/napi.cpp index adb5074ca8..69d77263e0 100644 --- a/src/bun.js/bindings/napi.cpp +++ b/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 #include +#include "CommonJSModuleRecord.h" // #include 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 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 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(cb); auto* function = NAPIFunction::create(vm, globalObject, length, name, method, data); + ASSERT(function->isCallable()); *result = toNapi(JSC::JSValue(function)); return napi_ok; diff --git a/src/symbols.txt b/src/symbols.txt index 157c0c5157..577035fa93 100644 --- a/src/symbols.txt +++ b/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 diff --git a/test/napi/napi-app/main.cpp b/test/napi/napi-app/main.cpp index 87a70c98c8..f443ee79d7 100644 --- a/test/napi/napi-app/main.cpp +++ b/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(); + 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)); diff --git a/test/napi/napi-app/main.js b/test/napi/napi-app/main.js index 64c8d8cf96..27bba50a23 100644 --- a/test/napi/napi-app/main.js +++ b/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]); diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index 266db7397f..e30fac93c7 100644 --- a/test/napi/napi.test.ts +++ b/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[]) {