Merge pull request nodejs#142 from boingoing/remove_set_function_name

napi: Remove napi_set_function_name since not all engines can rename functions
boingoing · Mar 15, 2017 · 45f84c7 · 45f84c7
2 parents 960ed1d + 5011ee1
commit 45f84c7
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 35 deletions.
diff --git a/src/node_jsvmapi.cc b/src/node_jsvmapi.cc
@@ -1,4 +1,4 @@
-/*******************************************************************************
+/*******************************************************************************
  * Experimental prototype for demonstrating VM agnostic and ABI stable API
  * for native modules to use instead of using Nan and V8 APIs directly.
  *
@@ -99,11 +99,11 @@ namespace v8impl {
     return u.l;
   }
 
-  v8::Local<v8::Value> V8LocalValueFromJsPropertyName(napi_propertyname pn) {
+  v8::Local<v8::String> V8LocalStringFromJsPropertyName(napi_propertyname pn) {
     // Likewise awkward
     union U {
       napi_propertyname pn;
-      v8::Local<v8::Value> l;
+      v8::Local<v8::String> l;
       U(napi_propertyname _pn) : pn(_pn) { }
     } u(pn);
     assert(sizeof(u.pn) == sizeof(u.l));
@@ -637,6 +637,7 @@ napi_status napi_set_last_error(napi_status error_code,
 
 napi_status napi_create_function(
     napi_env e,
+    const char* utf8name,
     napi_callback cb,
     void* data,
     napi_value* result) {
@@ -645,7 +646,7 @@ napi_status napi_create_function(
 
   v8::Isolate *isolate = v8impl::V8IsolateFromJsEnv(e);
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
-  v8::Local<v8::Object> retval;
+  v8::Local<v8::Function> retval;
 
   v8::EscapableHandleScope scope(isolate);
 
@@ -658,6 +659,13 @@ napi_status napi_create_function(
     isolate, v8impl::FunctionCallbackWrapper::Invoke, cbdata);
 
   retval = scope.Escape(tpl->GetFunction());
+
+  if (utf8name) {
+    v8::Local<v8::String> namestring;
+    CHECK_NEW_FROM_UTF8(isolate, namestring, utf8name);
+    retval->SetName(namestring);
+  }
+
   *result = v8impl::JsValueFromV8LocalValue(retval);
 
   return GET_RETURN_STATUS();
@@ -766,17 +774,6 @@ napi_status napi_define_class(
   return GET_RETURN_STATUS();
 }
 
-napi_status napi_set_function_name(napi_env e, napi_value func,
-                                   napi_propertyname name) {
-  NAPI_PREAMBLE(e);
-
-  v8::Local<v8::Function> v8func = v8impl::V8LocalFunctionFromJsValue(func);
-  v8func->SetName(
-      v8impl::V8LocalValueFromJsPropertyName(name).As<v8::String>());
-
-  return GET_RETURN_STATUS();
-}
-
 napi_status napi_set_return_value(napi_env e,
                                   napi_callback_info cbinfo, napi_value v) {
   NAPI_PREAMBLE(e);
@@ -833,7 +830,7 @@ napi_status napi_set_property(napi_env e,
 
   CHECK_TO_OBJECT(context, obj, o);
 
-  v8::Local<v8::Value> key = v8impl::V8LocalValueFromJsPropertyName(k);
+  v8::Local<v8::String> key = v8impl::V8LocalStringFromJsPropertyName(k);
   v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(v);
 
   v8::Maybe<bool> set_maybe = obj->Set(context, key, val);
@@ -852,7 +849,7 @@ napi_status napi_has_property(napi_env e, napi_value o, napi_propertyname k, boo
 
   CHECK_TO_OBJECT(context, obj, o);
 
-  v8::Local<v8::Value> key = v8impl::V8LocalValueFromJsPropertyName(k);
+  v8::Local<v8::String> key = v8impl::V8LocalStringFromJsPropertyName(k);
   v8::Maybe<bool> has_maybe = obj->Has(context, key);
 
   CHECK_MAYBE_NOTHING(has_maybe, napi_generic_failure);
@@ -870,7 +867,7 @@ napi_status napi_get_property(napi_env e,
 
   v8::Isolate *isolate = v8impl::V8IsolateFromJsEnv(e);
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
-  v8::Local<v8::Value> key = v8impl::V8LocalValueFromJsPropertyName(k);
+  v8::Local<v8::String> key = v8impl::V8LocalStringFromJsPropertyName(k);
   v8::Local<v8::Object> obj;
 
   CHECK_TO_OBJECT(context, obj, o);

diff --git a/src/node_jsvmapi.h b/src/node_jsvmapi.h
@@ -1,4 +1,4 @@
-/*******************************************************************************
+/*******************************************************************************
  * Experimental prototype for demonstrating VM agnostic and ABI stable API
  * for native modules to use instead of using Nan and V8 APIs directly.
  *
@@ -105,6 +105,7 @@ NODE_EXTERN napi_status napi_create_symbol(napi_env e,
                                            const char* s,
                                            napi_value* result);
 NODE_EXTERN napi_status napi_create_function(napi_env e,
+                                             const char* utf8name,
                                              napi_callback cb,
                                              void* data,
                                              napi_value* result);
@@ -236,9 +237,6 @@ NODE_EXTERN napi_status napi_strict_equals(napi_env e,
                                            bool* result);
 
 // Methods to work with Functions
-NODE_EXTERN napi_status napi_set_function_name(napi_env e,
-                                               napi_value func,
-                                               napi_propertyname napi_value);
 NODE_EXTERN napi_status napi_call_function(napi_env e,
                                            napi_value recv,
                                            napi_value func,

diff --git a/test/addons-abi/5_function_factory/binding.cc b/test/addons-abi/5_function_factory/binding.cc
@@ -1,4 +1,4 @@
-#include <node_jsvmapi.h>
+#include <node_jsvmapi.h>
 
 void MyFunction(napi_env env, napi_callback_info info) {
   napi_status status;
@@ -15,15 +15,7 @@ void CreateFunction(napi_env env, napi_callback_info info) {
   napi_status status;
 
   napi_value fn;
-  status = napi_create_function(env, MyFunction, nullptr, &fn);
-  if (status != napi_ok) return;
-
-  napi_propertyname name;
-  status = napi_property_name(env, "theFunction", &name);
-  if (status != napi_ok) return;
-
-  // omit this to make it anonymous
-  status = napi_set_function_name(env, fn, name);
+  status = napi_create_function(env, "theFunction", MyFunction, nullptr, &fn);
   if (status != napi_ok) return;
 
   status = napi_set_return_value(env, info, fn);
@@ -32,7 +24,7 @@ void CreateFunction(napi_env env, napi_callback_info info) {
 
 void Init(napi_env env, napi_value exports, napi_value module) {
   napi_status status;
-  napi_property_descriptor desc = { "exports", CreateFunction };
+  napi_property_descriptor desc = { "exports", CreateFunction, nullptr, nullptr, nullptr, napi_default, nullptr };
   status = napi_define_properties(env, module, 1, &desc);
   if (status != napi_ok) return;
 }

diff --git a/test/addons-abi/test_function/test_function.cc b/test/addons-abi/test_function/test_function.cc
@@ -1,4 +1,4 @@
-#include <node_jsvmapi.h>
+#include <node_jsvmapi.h>
 
 void Test(napi_env env, napi_callback_info info) {
   napi_status status;
@@ -49,7 +49,7 @@ void Init(napi_env env, napi_value exports, napi_value module) {
   if (status != napi_ok) return;
 
   napi_value fn;
-  status =  napi_create_function(env, Test, nullptr, &fn);
+  status =  napi_create_function(env, Test, nullptr, nullptr, &fn);
   if (status != napi_ok) return;
 
   status = napi_set_property(env, exports, name, fn);