Repository navigation
More (unnecessary?) NAPI_PREAMBLE() calls #238
Description
Activity
If you have checked and don't believe we need them there, I'm happy to have them be removed.
The only thing I'm not sure about is whether the engine would ever throw an exception in response to
IsArrayBuffer(), orIsArray()or functions of this nature. I think I'll remove theNAPI_PREAMBLE()s, because it doesn't make sense to me that it should ever throw an exception in such cases.+1
The four listed above seem the most obvious, though we may also consider several other APIs that don't have any possibility of invoking JavaScript code. Does V8 ever throw exceptions when not invoking JS code? I assume not.
OK, so while we're on the topic, what about these:
napi_get_value_string_latin1napi_get_value_string_utf8napi_get_value_string_utf16napi_get_value_externalnapi_get_buffer_infonapi_get_arraybuffer_infonapi_get_typedarray_info
We should test how these functions behave if we pass in the wrong type, or, better yetCHECK_TO_<type>at the top.
better yet CHECK_TO_ at the top
Generally we have avoided automatic type coercions, because they can have a significant perf cost. (I think there may even be a few places currently doing type coercions that shouldn't.) If the wrong type is passed into an API, it should just return an appropriate error code.
And yes, I agree with adding all those getters to the list.
They all have checks, so that's good.
Yeah, sorry, I meant type checks, not coercions.
- added a commit that references this issue
on Apr 27, 2017 - added a commit that references this issue
on May 6, 2017 - added a commit that references this issue
on Apr 10, 2018 - added a commit that references this issue
on Apr 16, 2018 - added a commit that references this issue
on Jul 27, 2026
We have
napi_is_array(val->IsArray())napi_is_buffer(node::Buffer::HasInstance())napi_is_arraybuffer(val->IsArrayBuffer())napi_is_typedarray(val->IsTypedArray())that have
NAPI_PREAMBLE(), whereasnapi_is_error(val->IsNativeError())does not have it.
I don't believe the ones above that have
NAPI_PREAMBLE()need it, for the same reason thatnapi_typeof()doesn't need it. What do @nodejs/addon-api think?