1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

n-api: define ECMAScript-compliant accessors on napi_define_properties

Browse Source 
PR-URL: https://github.com/nodejs/node/pull/27851
Fixes: https://github.com/nodejs/node/issues/26551
Fixes: https://github.com/nodejs/node-addon-api/issues/485
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This commit is contained in:
legendecas 2019-05-24 21:48:21 +08:00 committed by Ruben Bridgewater
parent 44f18d236b
commit 432958528e
No known key found for this signature in database
GPG Key ID: F07496B3EB3C1762
2 changed files with 64 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
src/js_native_api_v8.cc
View File

@ -1178,26 +1178,45 @@ napi_status napi_define_properties(napi_env env,
return napi_set_last_error(env, status);
}
v8::PropertyAttribute attributes =
v8impl::V8PropertyAttributesFromDescriptor(p);
if (p->getter != nullptr || p->setter != nullptr) {
v8::Local<v8::Value> cbdata = v8impl::CreateAccessorCallbackData(
env,
p->getter,
p->setter,
p->data);
v8::Local<v8::Value> local_getter;
v8::Local<v8::Value> local_setter;
auto set_maybe = obj->SetAccessor(
context,
property_name,
p->getter ? v8impl::GetterCallbackWrapper::Invoke : nullptr,
p->setter ? v8impl::SetterCallbackWrapper::Invoke : nullptr,
cbdata,
v8::AccessControl::DEFAULT,
attributes);
if (p->getter != nullptr) {
v8::Local<v8::Value> getter_data =
v8impl::CreateFunctionCallbackData(env, p->getter, p->data);
CHECK_MAYBE_EMPTY(env, getter_data, napi_generic_failure);
if (!set_maybe.FromMaybe(false)) {
v8::MaybeLocal<v8::Function> maybe_getter =
v8::Function::New(context,
v8impl::FunctionCallbackWrapper::Invoke,
getter_data);
CHECK_MAYBE_EMPTY(env, maybe_getter, napi_generic_failure);
local_getter = maybe_getter.ToLocalChecked();
}
if (p->setter != nullptr) {
v8::Local<v8::Value> setter_data =
v8impl::CreateFunctionCallbackData(env, p->setter, p->data);
CHECK_MAYBE_EMPTY(env, setter_data, napi_generic_failure);
v8::MaybeLocal<v8::Function> maybe_setter =
v8::Function::New(context,
v8impl::FunctionCallbackWrapper::Invoke,
setter_data);
CHECK_MAYBE_EMPTY(env, maybe_setter, napi_generic_failure);
local_setter = maybe_setter.ToLocalChecked();
}
v8::PropertyDescriptor descriptor(local_getter, local_setter);
descriptor.set_enumerable((p->attributes & napi_enumerable) != 0);
descriptor.set_configurable((p->attributes & napi_configurable) != 0);
auto define_maybe = obj->DefineProperty(context,
property_name,
descriptor);
if (!define_maybe.FromMaybe(false)) {
return napi_set_last_error(env, napi_invalid_arg);
}
} else if (p->method != nullptr) {
@ -1213,8 +1232,14 @@ napi_status napi_define_properties(napi_env env,
CHECK_MAYBE_EMPTY(env, maybe_fn, napi_generic_failure);
auto define_maybe = obj->DefineOwnProperty(
context, property_name, maybe_fn.ToLocalChecked(), attributes);
v8::PropertyDescriptor descriptor(maybe_fn.ToLocalChecked(),
(p->attributes & napi_writable) != 0);
descriptor.set_enumerable((p->attributes & napi_enumerable) != 0);
descriptor.set_configurable((p->attributes & napi_configurable) != 0);
auto define_maybe = obj->DefineProperty(context,
property_name,
descriptor);
if (!define_maybe.FromMaybe(false)) {
return napi_set_last_error(env, napi_generic_failure);
@ -1222,8 +1247,13 @@ napi_status napi_define_properties(napi_env env,
} else {
v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(p->value);
v8::PropertyDescriptor descriptor(value,
(p->attributes & napi_writable) != 0);
descriptor.set_enumerable((p->attributes & napi_enumerable) != 0);
descriptor.set_configurable((p->attributes & napi_configurable) != 0);
auto define_maybe =
obj->DefineOwnProperty(context, property_name, value, attributes);
obj->DefineProperty(context, property_name, descriptor);
if (!define_maybe.FromMaybe(false)) {
return napi_set_last_error(env, napi_invalid_arg);

16
test/js-native-api/test_properties/test.js
View File

@ -3,6 +3,8 @@ const common = require('../../common');
const assert = require('assert');
const readonlyErrorRE =
/^TypeError: Cannot assign to read only property '.*' of object '#<Object>'$/;
const getterOnlyErrorRE =
/^TypeError: Cannot set property .* of #<Object> which has only a getter$/;
// Testing api calls for defining properties
const test_object = require(`./build/${common.buildType}/test_properties`);
@ -41,14 +43,24 @@ const symbolDescription =
assert.strictEqual(symbolDescription, 'NameKeySymbol');
// The napi_writable attribute should be ignored for accessors.
const readwriteAccessor1Descriptor =
Object.getOwnPropertyDescriptor(test_object, 'readwriteAccessor1');
const readonlyAccessor1Descriptor =
Object.getOwnPropertyDescriptor(test_object, 'readonlyAccessor1');
assert.ok(readwriteAccessor1Descriptor.get != null);
assert.ok(readwriteAccessor1Descriptor.set != null);
assert.ok(readwriteAccessor1Descriptor.value === undefined);
assert.ok(readonlyAccessor1Descriptor.get != null);
assert.ok(readonlyAccessor1Descriptor.set === undefined);
assert.ok(readonlyAccessor1Descriptor.value === undefined);
test_object.readwriteAccessor1 = 1;
assert.strictEqual(test_object.readwriteAccessor1, 1);
assert.strictEqual(test_object.readonlyAccessor1, 1);
assert.throws(() => { test_object.readonlyAccessor1 = 3; }, readonlyErrorRE);
assert.throws(() => { test_object.readonlyAccessor1 = 3; }, getterOnlyErrorRE);
test_object.readwriteAccessor2 = 2;
assert.strictEqual(test_object.readwriteAccessor2, 2);
assert.strictEqual(test_object.readonlyAccessor2, 2);
assert.throws(() => { test_object.readonlyAccessor2 = 3; }, readonlyErrorRE);
assert.throws(() => { test_object.readonlyAccessor2 = 3; }, getterOnlyErrorRE);
assert.strictEqual(test_object.hasNamedProperty(test_object, 'echo'), true);
assert.strictEqual(test_object.hasNamedProperty(test_object, 'hiddenValue'),