N-API should allow setting the 'length' property of functions

[TimothyGu]

Currently, napi_create_function() and napi_define_class() only accept name, native callback, and an opaque data pointer.

node/src/js_native_api.h

Lines 87 to 92 in c1f0cbe

	NAPI_EXTERN napi_status napi_create_function(napi_env env,
	const char* utf8name,
	size_t length,
	napi_callback cb,
	void* data,
	napi_value* result);

node/src/js_native_api.h

Lines 269 to 277 in c1f0cbe

	NAPI_EXTERN napi_status
	napi_define_class(napi_env env,
	const char* utf8name,
	size_t length,
	napi_callback constructor,
	void* data,
	size_t property_count,
	const napi_property_descriptor* properties,
	napi_value* result);

In particular, it does not accept a parameter for the length property for JavaScript functions.

((a, b, ...c) => {}).length
// 2

While it's possible to redefine the length property after the function/class has been created, doing so could be slow and error-prone. It would be nice if N-API could directly support this use case.

[mhdawson]

We discussed this a bit in the last N-API call. Our guess/understanding is that the main motivation is to allow the JavaScript engine to better optimize as today the length property is not set? The advantage to providing signatures with the length value is to avoid a second round trip to set it using a napi_set_property? Is this correct or is there something else that motivates this.

[devsnek]

Right now the length is always zero. Providing a length seems reasonable to me because it can be abstracted. In a v8 napi, we can just pass the int to FunctionTemplate::New or Function::New. In a jerryscript napi, an additional property set call can be added.

[mhdawson]

@devsnek the assumption was that the only downside of the length being zero is potentially performance right? If we added a new method with the length it could be documented as a 'hint' to the JavaScript engine that may be ignored.

[devsnek]

I'm not entirely sure about the optimization aspect, I just think this is important for consistency with functions written in js.

[mhdawson]

@devsnek to make sure I understand your concern. It's that functions created in JavaScript will always have the value set to the right number, but that those created on the native side will have it set to 0?

[devsnek]

@mhdawson correct

[NickNaso]

Hi everyone,
I tried to work on this issue and I opened a PR. I don't know if my approach is good please take a look and give me a feedback.

[NickNaso]

@mhdawson I report the v8 reference link here:
https://v8docs.nodesource.com/node-12.0/d5/d54/classv8_1_1_function.html#afb0ebb664e49eec0e4ebc17d9c09390d

https://v8docs.nodesource.com/node-12.0/d8/d83/classv8_1_1_function_template.html#aea9e604232a512505ebd1c63cb5baf41

Like you can see in both cases v8::Function::New and v8::FunctionTemplate::New the size is set to 0 by default.

[NickNaso]

I found the similar API in other JavaScript engines:

ChackraCore

• https://github.com/microsoft/ChakraCore/blob/master/lib/Jsrt/ChakraCommon.h#L2142

JsCallFunction(
            _In_ JsValueRef function,
            _In_reads_(argumentCount) JsValueRef *arguments,
            _In_ unsigned short argumentCount,
            _Out_ JsValueRef *result);

[in] function: The function to invoke.
[in] arguments: The arguments to the call.
[in] argumentCount: The number of arguments being passed in to the function.
[out] result: The value returned from the function invocation.

• https://github.com/microsoft/ChakraCore/blob/master/lib/Jsrt/ChakraCommon.h#L2162

JsConstructObject(
            _In_ JsValueRef function,
            _In_reads_(argumentCount) JsValueRef *arguments,
            _In_ unsigned short argumentCount,
            _Out_ JsValueRef *result);

[in] function: The function to invoke as a constructor.
[in] arguments: The arguments to the call.
[in] argumentCount: The number of arguments being passed in to the function.
[out] result: The value returned from the function invocation.

SpiderMonkey

• https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JSNative

• https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS::CallArgs

JavaScriptCore

• https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/API/JSObjectRef.h#L202

typedef JSValueRef  
(*JSObjectCallAsFunctionCallback) 
(JSContextRef ctx, 
 JSObjectRef function, 
 JSObjectRef thisObject, 
 size_t argumentCount, 
 const JSValueRef arguments[], 
 JSValueRef* exception);

The callback invoked when an object is called as a function.
[in] ctx: The execution context to use.
[in] function: A JSObject that is the function being called.
[in] thisObject: A JSObject that is the 'this' variable in the function's scope.
[in] argumentCount: An integer count of the number of arguments in arguments.
[in] arguments: A JSValue array of the arguments passed to the function.
[in-out] exception: A pointer to a JSValueRef in which to return an exception, if any.
[out] result A JSValue that is the function's return value.

• https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/API/JSObjectRef.h#L223

typedef JSObjectRef 
(*JSObjectCallAsConstructorCallback) 
(JSContextRef ctx,
 JSObjectRef constructor, 
 size_t argumentCount, 
 const JSValueRef arguments[], 
 JSValueRef* exception);

The callback invoked when an object is used as a constructor in a new expression.
[in] ctx: The execution context to use.
[in] constructor: A JSObject that is the constructor being called.
[in] argumentCount: An integer count of the number of arguments in arguments.
[in] arguments: A JSValue array of the arguments passed to the function.
[in-out] exception: A pointer to a JSValueRef in which to return an exception, if any.
[out] result: A JSObject that is the constructor's return value.

[mhdawson]

@NickNaso can you add the link to the PR you were working on? Just wondering what the status is in respect to this issue.

[NickNaso]

@mhdawson this is the link #29093

[mhdawson]

I see it was closed as stalled, @NickNaso if you have cycles to pick it back up I'll reopen the PR.

[github-actions]

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[github-actions]

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.