InstanceOf 6.2.0

[jasongin]

Adding a napi_instanceof API that checks whether a napi_value object is an instance created by a napi_value constructor function.

The V8 implementation isn't entirely trivial: V8 code would normally use FunctionTemplate::HasInstance(), except here we don't have the FunctionTemplate available because (as a V8-only concept) it is not exposed via NAPI. So instead we can implement the instance-of logic in a slightly less efficient way by looking through the prototype chain. (The JSRT implementation can simply call JsInstanceOf().)

I started porting the canvas package and found it makes frequent use of FunctionTemplate::HasInstance() checks to support sort of polymorphic behavior for drawing different kinds of things on the canvas. The sqlite3 package use the API in one place, just for a simple argument check. Based on my previous scans, not very many other packages use it.

[gabrielschulhof]

I have the V8 implementation.

…

On Sun, Jan 15, 2017 at 10:43 PM, Jason Ginchereau ***@***.*** > wrote: Adding a napi_instanceof API that checks whether a napi_value object is an instance created by a napi_value constructor function. The V8 implementation isn't entirely trivial: V8 code would normally use FunctionTemplate::HasInstance(), except here we don't have the FunctionTemplate available because (as a V8-only concept) it is not exposed via NAPI. So instead we can implement the instance-of logic in a slightly less efficient way by looking through the prototype chain. (The JSRT implementation can simply call JsInstanceOf().) I started porting the canvas package and found it makes frequent use of FunctionTemplate::HasInstance() checks to support sort of polymorphic behavior for drawing different kinds of things on the canvas. The sqlite3 package use the API in one place, just for a simple argument check. Based on my previous scans, not very many other packages use it. ------------------------------ You can view, comment on, or merge this pull request online at: #47 Commit Summary - Add instanceof API File Changes - *M* src/node_jsvmapi.cc <https://github.com/nodejs/abi-stable-node/pull/47/files#diff-0> (30) - *M* src/node_jsvmapi.h <https://github.com/nodejs/abi-stable-node/pull/47/files#diff-1> (1) Patch Links: - https://github.com/nodejs/abi-stable-node/pull/47.patch - https://github.com/nodejs/abi-stable-node/pull/47.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#47>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA7k0U7GlZDjyJcGziIqao2NZ_kPQ5cjks5rSoTkgaJpZM4LkBpI> .

[gabrielschulhof]

@jasongin if you only need instanceof because of canvas and sqlite then perhaps we need not add it to NAPI because it can be implemented using existing NAPI APIs (https://github.com/gabrielschulhof/abi-stable-node/blob/test-instanceof-master/test/addons-abi/test_instanceof/test_instanceof.cc#L15).

If we do add it to NAPI, we should make sure that it passes the test from my PR.

[gabrielschulhof]

Hmmm, given that we currently have canvas, sqlite, and even iotivity-node that need this, maybe we should have an implementation of this in NAPI after all.

[jasongin]

I think people would expect it to be part of the API. This PR worked in my limited testing, enough to unblock canvas, but it looks like you've done a lot more extensive testing than I did.

[jasongin]

Closing this in favor of @gabrielschulhof's more complete PR.