n-api: stop creating references to primitives

doc/api/n-api.md

@@ -786,7 +786,7 @@ NODE_EXTERN napi_status napi_create_reference(napi_env env,
 
 - `[in] env`: The environment that the API is invoked under.
 - `[in] value`: `napi_value` representing the Object to which we want
-a reference to.
+a reference.
 - `[in] initial_refcount`: Initial reference count for the new reference.
 - `[out] result`: `napi_ref` pointing to the new reference.
 

src/node_api.cc

@@ -2458,8 +2458,14 @@ napi_status napi_create_reference(napi_env env,
   CHECK_ARG(env, value);
   CHECK_ARG(env, result);
 
-  v8impl::Reference* reference = v8impl::Reference::New(
-      env, v8impl::V8LocalValueFromJsValue(value), initial_refcount, false);
+  v8::Local<v8::Value> v8_value = v8impl::V8LocalValueFromJsValue(value);
+
+  if (!(v8_value->IsObject() || v8_value->IsFunction())) {
+    return napi_set_last_error(env, napi_object_expected);
+  }
+
+  v8impl::Reference* reference =
+      v8impl::Reference::New(env, v8_value, initial_refcount, false);
 
   *result = reinterpret_cast<napi_ref>(reference);
   return napi_clear_last_error(env);

test/addons-napi/test_general/test.js

@@ -60,7 +60,7 @@ assert.strictEqual(test_general.testNapiTypeof(null), 'null');
 // Ensure that garbage collecting an object with a wrapped native item results
 // in the finalize callback being called.
 let w = {};
-test_general.wrap(w, []);
+test_general.wrap(w);
 w = null;
 global.gc();
 assert.strictEqual(test_general.derefItemWasCalled(), true,
@@ -69,17 +69,17 @@ assert.strictEqual(test_general.derefItemWasCalled(), true,
 
 // Assert that wrapping twice fails.
 const x = {};
-test_general.wrap(x, 25);
+test_general.wrap(x);
 assert.throws(function() {
-  test_general.wrap(x, 'Blah');
+  test_general.wrap(x);
 }, Error);
 
 // Ensure that wrapping, removing the wrap, and then wrapping again works.
 const y = {};
-test_general.wrap(y, -12);
+test_general.wrap(y);
 test_general.removeWrap(y);
 assert.doesNotThrow(function() {
-  test_general.wrap(y, 're-wrap!');
+  test_general.wrap(y);
 }, Error, 'Wrapping twice succeeds if a remove_wrap() separates the instances');
 
 // Ensure that removing a wrap and garbage collecting does not fire the

test/addons-napi/test_general/test_general.c

@@ -142,8 +142,10 @@ static bool deref_item_called = false;
 static void deref_item(napi_env env, void* data, void* hint) {
   (void) hint;
 
+  NAPI_ASSERT_RETURN_VOID(env, data == &deref_item_called,
+    "Finalize callback was called with the correct pointer");
+
   deref_item_called = true;
-  NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, (napi_ref)data));
 }
 
 napi_value deref_item_was_called(napi_env env, napi_callback_info info) {
@@ -155,15 +157,13 @@ napi_value deref_item_was_called(napi_env env, napi_callback_info info) {
 }
 
 napi_value wrap(napi_env env, napi_callback_info info) {
-  size_t argc = 2;
-  napi_value argv[2];
-  napi_ref payload;
+  size_t argc = 1;
+  napi_value to_wrap;
 
   deref_item_called = false;
 
-  NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL));
-  NAPI_CALL(env, napi_create_reference(env, argv[1], 1, &payload));
-  NAPI_CALL(env, napi_wrap(env, argv[0], payload, deref_item, NULL, NULL));
+  NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &to_wrap, NULL, NULL));
+  NAPI_CALL(env, napi_wrap(env, to_wrap, &deref_item_called, deref_item, NULL, NULL));
 
   return NULL;
 }
@@ -175,9 +175,6 @@ napi_value remove_wrap(napi_env env, napi_callback_info info) {
 
   NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &wrapped, NULL, NULL));
   NAPI_CALL(env, napi_remove_wrap(env, wrapped, &data));
-  if (data != NULL) {
-    NAPI_CALL(env, napi_delete_reference(env, (napi_ref)data));
-  }
 
   return NULL;
 }