napi: change napi_instanceof() to use Symbol.hasInstance

Re nodejs#11975 (comment) Fixes nodejs/abi-stable-node#182 Closes nodejs/abi-stable-node#185
jasongin · Mar 23, 2017 · 1427b33 · 1427b33
1 parent 1b2f2db
commit 1427b33
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 3 deletions.
diff --git a/src/node_api.cc b/src/node_api.cc
@@ -2018,7 +2018,6 @@ napi_status napi_instanceof(napi_env env,
   *result = false;
 
   v8::Local<v8::Object> ctor;
-  v8::Local<v8::String> prototype_string;
   v8::Isolate* isolate = v8impl::V8IsolateFromJsEnv(env);
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
 
@@ -2030,6 +2029,47 @@ napi_status napi_instanceof(napi_env env,
     return napi_set_last_error(napi_function_expected);
   }
 
+  napi_value value, js_result;
+  napi_status status;
+  napi_valuetype value_type;
+
+  // Get "Symbol" from the global object
+  status = napi_get_global(env, &value);
+  if (status != napi_ok) return status;
+  status = napi_get_named_property(env, value, "Symbol", &value);
+  if (status != napi_ok) return status;
+  status = napi_typeof(env, value, &value_type);
+  if (status != napi_ok) return status;
+
+  // Get "hasInstance" from Symbol
+  if (value_type == napi_function) {
+    status = napi_get_named_property(env, value, "hasInstance", &value);
+    if (status != napi_ok) return status;
+    status = napi_typeof(env, value, &value_type);
+    if (status != napi_ok) return status;
+
+    // Retrieve the function at the Symbol(hasInstance) key of the constructor
+    if (value_type == napi_symbol) {
+      status = napi_get_property(env, constructor, value, &value);
+      if (status != napi_ok) return status;
+      status = napi_typeof(env, value, &value_type);
+      if (status != napi_ok) return status;
+
+      // Call the function to determine whether the object is an instance of the
+      // constructor
+      if (value_type == napi_function) {
+        status = napi_call_function(env, constructor, value, 1, &object,
+          &js_result);
+        if (status != napi_ok) return status;
+        return napi_get_value_bool(env, js_result, result);
+      }
+    }
+  }
+
+  // If running constructor[Symbol.hasInstance](object) did not work, we perform
+  // a traditional instanceof (early Node.js 6.x).
+
+  v8::Local<v8::String> prototype_string;
   CHECK_NEW_FROM_UTF8(isolate, prototype_string, "prototype");
 
   auto maybe = ctor->Get(context, prototype_string);

diff --git a/test/addons-napi/test_buffer/test_buffer.c b/test/addons-napi/test_buffer/test_buffer.c
@@ -122,7 +122,7 @@ void staticBuffer(napi_env env, napi_callback_info info) {
       env,
       napi_create_external_buffer(env,
                                   sizeof(theText),
-                                  theText,
+                                  (void *)theText,
                                   noopDeleter,
                                   NULL,  // finalize_hint
                                   &theBuffer));

diff --git a/test/addons-napi/test_conversions/test_conversions.c b/test/addons-napi/test_conversions/test_conversions.c
@@ -1,7 +1,7 @@
 #include <node_api.h>
 
 void ThrowLastError(napi_env env) {
-  napi_extended_error_info* error_info = napi_get_last_error_info();
+  const napi_extended_error_info* error_info = napi_get_last_error_info();
   if (error_info->error_code != napi_ok) {
     napi_throw_error(env, error_info->error_message);
   }

diff --git a/test/addons-napi/test_instanceof/test.js b/test/addons-napi/test_instanceof/test.js
@@ -47,3 +47,41 @@ testFile(
     path.join(path.resolve(__dirname, '..', '..', '..',
                            'deps', 'v8', 'test', 'mjsunit'),
               'instanceof-2.js'));
+
+// We can only perform this test if we have a working Symbol.hasInstance
+if (typeof Symbol !== 'undefined' && 'hasInstance' in Symbol &&
+    typeof Symbol.hasInstance === 'symbol') {
+
+  function compareToNative(theObject, theConstructor) {
+    assert.strictEqual(addon.doInstanceOf(theObject, theConstructor),
+      (theObject instanceof theConstructor));
+  }
+
+  const MyClass = function MyClass() {};
+  Object.defineProperty(MyClass, Symbol.hasInstance, {
+    value: function(candidate) {
+      return 'mark' in candidate;
+    }
+  });
+
+  const MySubClass = function MySubClass() {};
+  MySubClass.prototype = new MyClass();
+
+  let x = new MySubClass();
+  let y = new MySubClass();
+  x.mark = true;
+
+  compareToNative(x, MySubClass);
+  compareToNative(y, MySubClass);
+  compareToNative(x, MyClass);
+  compareToNative(y, MyClass);
+
+  x = new MyClass();
+  y = new MyClass();
+  x.mark = true;
+
+  compareToNative(x, MySubClass);
+  compareToNative(y, MySubClass);
+  compareToNative(x, MyClass);
+  compareToNative(y, MyClass);
+}