Change napi_callback to return napi_value instead of void #196
Conversation
|
Please take a look, I'll push a further commit to fix lint bugs found via CI. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Why isn't |
|
This is a little disruptive, yes, waiting for the PR to land is probably fine. I don't want it to get stale, though, since there are so many invasive changes to the test code. Most of the change is in the test code, which is probably not getting as much attention over in the PR. |
I thought about it but figured it didn't make sense because argc is both an in and an out parameter so it's required if argv is provided. That might be fine, though, to say if argv is provided that argc becomes non-optional. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Yes. While updating leveldown code I encountered several places where it would be more convenient to pass nullptr for both argc and argv. |
…ng napi_set_return_value When we invoke the callback, we will check the return value and call ```SetReturnValue``` ourselves. If the callback returns ```NULL```, we don't set the return value in v8 which would have the same effect as previously if the callback didn't call ```napi_set_return_value```. Seems to be a more natural way to handle return values from callbacks. As a consequence, remove ```napi_set_return_value```. Add a ```napi_value``` to ```napi_property_descriptor``` to support string values which couldn't be passed in the ```utf8name``` parameter or symbols as property names. Class names, however, cannot be symbols so this ```napi_value``` must be a string type in that case. Remove all of the ```napi_callback_info``` helpers except for ```napi_get_cb_info``` and make all the parameters to ```napi_get_cb_info``` optional except for argc. Update all the test collateral according to these changes.
boingoing
force-pushed
the
callback_return_napi_value
branch
from
850bd6d
to
c9ada56
Compare
|
Another CI job after rebasing |
|
landed in main repo closing. |
Change
napi_callbackto returnnapi_valueinstead of requiring the callback to manually callnapi_set_return_value.When we invoke the callback, we will check the return value and call
SetReturnValueourselves. If the callback returnsNULL, we don't set the return value in v8 which would have the same effect as previously if the callback didn't callnapi_set_return_value. Seems to be a more natural way to handle return values from callbacks. As a consequence, removenapi_set_return_value.Add a
napi_valuetonapi_property_descriptorto support string values which couldn't be passed in theutf8nameparameter or symbols as property names. Class names, however, cannot be symbols so thisnapi_valuemust be a string type in that case.Remove all of the
napi_callback_infohelpers except fornapi_get_cb_infoand make all the parameters tonapi_get_cb_infooptional except for argc.Update all the test collateral according to these changes.