Repository navigation
napi names for new types and methods #20170
Description
Activity
@nodejs/n-api
Each of those refer to the native type you are starting with (double, int32, int64) they might be better named something like napi_create_number_from_int32 etc but at this we can't easily rename.
For big int I could see using something like:
napi_create_bigint_from_XXX, where XXX is the native type you want to be able to provide, along with the corresponding napi_get_XXX_from_bigint.
For modules can you show a few examples the functions that the module types would be used as I think I need a bit more context to understand.
@mhdawson that looks like a better name scheme, but it makes me wonder if we can't just use overrides?
also for modules it's a bit like
napi_module module; napi_create_module(env, source, url, &module);
and the same for scripts but with napi_script and filename
and the get methods are actually fine because we can check what kind of value is passed (v8::Value::IsBigInt)
when you mean just have napi_create_bigint, which accepts the different native types through overrides? If so that sounds reasonable.
Does it need to be its own type as opposed to it being napi_value module in the example above ? I'm assuming it will be opaque to the native code once created.
does this api work for everyone? (TIL you can't do overrides in c :D)
Why do we need any
create_bigintother thanint64? (and also it's overloading rather than overriding)@TimothyGu no idea, why do we need any other create_number besides double... isn't that an implementation detail of v8?
alternative:
isn't that an implementation detail of v8?
Yes, but it has performance implications and (AFAIK) is relatively universal among engines.
fwiw both chakra and spidermonkey only have
doubleandint, jerryscript only takes double. i'd be fine with dropping the restI'm fine with dropping
napi_create_int64(), asnapi_create_double()suffices for that purpose. Otherwise, the current API as a least common multiple of all the different APIs out there looks just fine with me.The other consideration is if we want to drop any APIs at all, since N-API is supposed to be API/ABI-stable.
they can be deprecated forever I suppose, i assumed that major versions of node could make major changes, since this api is directly tied to node
I don’t think there’s any compelling reason to remove any API here – the existing ones work and people might have use cases for those. Also, at least for V8, I don’t know whether
napi_create_int32()is faster, but it definitely has a shorter path inside V8 for creating the handle.also i am still unsure of what to do for scripts and modules
Ignoring deprecating existing functions which we can discuss separately, the addition to the api would be:
napi_create_bigint
Right ?
I would prefer naming the new function
napi_create_bigint_int64since BigInt is arbitrary-precision and there isn’t an intrinsic bit width associated with it.@mhdawson here's what i made so far but i think there are some design decisions here i don't think should make it into the final product https://gist.github.com/devsnek/2fd71cab12f3f077f6ee70d2efa60854
i also feel this api should support multiple contexts but i have no idea how to reason with that
I'm working on patches to add bigint and scripts and modules to napi, but the current names of types and methods are causing consistency issues.
bigint:
napi_create_double,int32,int64 instead of napi_create_number makes me unsure of what to name creating a bigint so ideas are needed.
scripts/modules:
we have the type napi_module already and it would be an inconsistency to have stuff like napi_js_module and napi_js_script so ideas are needed.