Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node-api: Loosen restriction on the type of value that we can create a ref on #39926

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
src: allow napi_create_reference to recv symbol values
JckXia committed Aug 29, 2021

Verified

This commit was signed with the committer’s verified signature. The key has expired.
tdmorello Tim Morello
commit 1ba9a54fb5373ac1b37263d61270306c74ea081d
6 changes: 3 additions & 3 deletions src/js_native_api_v8.cc
Original file line number Diff line number Diff line change
@@ -2460,9 +2460,9 @@ napi_status napi_create_reference(napi_env env,
CHECK_ARG(env, result);

v8::Local<v8::Value> v8_value = v8impl::V8LocalValueFromJsValue(value);

if (!(v8_value->IsObject() || v8_value->IsFunction())) {
return napi_set_last_error(env, napi_object_expected);
if (!(v8_value->IsObject() || v8_value->IsFunction() ||
v8_value->IsSymbol())) {
return napi_set_last_error(env, napi_invalid_arg);
}

v8impl::Reference* reference =
7 changes: 7 additions & 0 deletions test/js-native-api/test_reference/test.js
Original file line number Diff line number Diff line change
@@ -14,6 +14,13 @@ assert.strictEqual(test_reference.finalizeCount, 0);
// Run each test function in sequence,
// with an async delay and GC call between each.
async function runTests() {
(() => {
const symbol = test_reference.createSymbol('testSym');
test_reference.createReference(symbol, 0);
assert.strictEqual(test_reference.referenceValue, symbol);
})();
test_reference.deleteReference();

(() => {
const value = test_reference.createExternal();
assert.strictEqual(test_reference.finalizeCount, 0);
15 changes: 15 additions & 0 deletions test/js-native-api/test_reference/test_reference.c
Original file line number Diff line number Diff line change
@@ -35,6 +35,20 @@ static napi_value CreateExternal(napi_env env, napi_callback_info info) {
return result;
}

static napi_value CreateSymbol(napi_env env, napi_callback_info info) {

size_t argc = 1;
napi_value args[1];

NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL,NULL));
NODE_API_ASSERT(env, argc == 1, "Expect one argument only(symbol description)");

napi_value result_symbol;

NODE_API_CALL(env, napi_create_symbol(env, args[0], &result_symbol));
return result_symbol;
}

static napi_value
CreateExternalWithFinalize(napi_env env, napi_callback_info info) {
napi_value result;
@@ -175,6 +189,7 @@ napi_value Init(napi_env env, napi_value exports) {
CreateExternalWithFinalize),
DECLARE_NODE_API_PROPERTY("checkExternal", CheckExternal),
DECLARE_NODE_API_PROPERTY("createReference", CreateReference),
DECLARE_NODE_API_PROPERTY("createSymbol", CreateSymbol),
DECLARE_NODE_API_PROPERTY("deleteReference", DeleteReference),
DECLARE_NODE_API_PROPERTY("incrementRefcount", IncrementRefcount),
DECLARE_NODE_API_PROPERTY("decrementRefcount", DecrementRefcount),