Repository navigation
napi_call_function (and maybe others) - we assume napi_value passed in is not NULL #227
Description
napi_status napi_call_function(napi_env env,
napi_value recv,
napi_value func,
size_t argc,
const napi_value* argv,
napi_value* result) {
NAPI_PREAMBLE(env);
v8::Isolate* isolate = env->isolate;
v8::Local<v8::Context> context = isolate->GetCurrentContext();
v8::Local<v8::Value> v8recv = v8impl::V8LocalValueFromJsValue(recv);
...What if recv is NULL? I am tempted to allow NULL here, because we forego an additional N-API call just to retrieve a JS undefined as a napi_value. Still, we should not assume that it's not NULL.
Metadata
Metadata
Assignees
Labels
No labels
Activity
jasongin commentedon Apr 13, 2017
It looks like we're missing calls to both
CHECK_ARG(recv)andCHECK_ARG(func)here.I do agree that it might make sense to allow
recvto be null, though in that case I think it should substitute theglobalobject rather thanundefined.napi_make_callback()has the same issues.addaleax commentedon Apr 13, 2017
@jasongin Why? I’ve noticed that
node-apidoes it that way too, and it looks very deliberate, but I’m still confused by that. I would say it’s an antipattern in JavaScript to letthisdefault toglobal, and I wouldn’t be surprised if this tripped up users that are used to JS strict mode.jasongin commentedon Apr 13, 2017
OK, that would be my mistake then. I didn't realize strict mode changed that behavior. (I don't normally write JavaScript code that relies on the default value of
thiseither way.)Default `this` to `undefined` for function calls
Default `this` to `undefined` for function calls
n-api: tighten null-checking and clean up last error
n-api: tighten null-checking and clean up last error
n-api: tighten null-checking and clean up last error
n-api: tighten null-checking and clean up last error
n-api: tighten null-checking and clean up last error
Default `this` to `undefined` for function calls
Default `this` to `undefined` for function calls
Default `this` to `undefined` for function calls
Default `this` to `undefined` for function calls
n-api: tighten null-checking and clean up last error