FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

n-api: define ECMAScript-compliant accessors on napi_define_properties · nodejs/node@d70fd7d · GitHub

/ node Public

Commit d70fd7d

Browse files
legendecas
authored and
BridgeAR
committed
n-api: define ECMAScript-compliant accessors on napi_define_properties
PR-URL: #27851 Fixes: #26551 Fixes: nodejs/node-addon-api#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>
1 parent edb1a3c commit d70fd7d

2 files changed

Lines changed: 64 additions & 22 deletions

File tree

‎src/js_native_api_v8.cc‎

Lines changed: 50 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,26 +1178,45 @@ napi_status napi_define_properties(napi_env env,
11781178
return napi_set_last_error(env, status);
11791179
}
11801180

1181-
v8::PropertyAttribute attributes =
1182-
v8impl::V8PropertyAttributesFromDescriptor(p);
1183-
11841181
if (p->getter != nullptr || p->setter != nullptr) {
1185-
v8::Local<v8::Value> cbdata = v8impl::CreateAccessorCallbackData(
1186-
env,
1187-
p->getter,
1188-
p->setter,
1189-
p->data);
1182+
v8::Local<v8::Value> local_getter;
1183+
v8::Local<v8::Value> local_setter;
11901184

1191-
auto set_maybe = obj->SetAccessor(
1192-
context,
1193-
property_name,
1194-
p->getter ? v8impl::GetterCallbackWrapper::Invoke : nullptr,
1195-
p->setter ? v8impl::SetterCallbackWrapper::Invoke : nullptr,
1196-
cbdata,
1197-
v8::AccessControl::DEFAULT,
1198-
attributes);
1185+
if (p->getter != nullptr) {
1186+
v8::Local<v8::Value> getter_data =
1187+
v8impl::CreateFunctionCallbackData(env, p->getter, p->data);
1188+
CHECK_MAYBE_EMPTY(env, getter_data, napi_generic_failure);
1189+
1190+
v8::MaybeLocal<v8::Function> maybe_getter =
1191+
v8::Function::New(context,
1192+
v8impl::FunctionCallbackWrapper::Invoke,
1193+
getter_data);
1194+
CHECK_MAYBE_EMPTY(env, maybe_getter, napi_generic_failure);
1195+
1196+
local_getter = maybe_getter.ToLocalChecked();
1197+
}
1198+
if (p->setter != nullptr) {
1199+
v8::Local<v8::Value> setter_data =
1200+
v8impl::CreateFunctionCallbackData(env, p->setter, p->data);
1201+
CHECK_MAYBE_EMPTY(env, setter_data, napi_generic_failure);
1202+
1203+
v8::MaybeLocal<v8::Function> maybe_setter =
1204+
v8::Function::New(context,
1205+
v8impl::FunctionCallbackWrapper::Invoke,
1206+
setter_data);
1207+
CHECK_MAYBE_EMPTY(env, maybe_setter, napi_generic_failure);
1208+
local_setter = maybe_setter.ToLocalChecked();
1209+
}
11991210

1200-
if (!set_maybe.FromMaybe(false)) {
1211+
v8::PropertyDescriptor descriptor(local_getter, local_setter);
1212+
descriptor.set_enumerable((p->attributes & napi_enumerable) != 0);
1213+
descriptor.set_configurable((p->attributes & napi_configurable) != 0);
1214+
1215+
auto define_maybe = obj->DefineProperty(context,
1216+
property_name,
1217+
descriptor);
1218+
1219+
if (!define_maybe.FromMaybe(false)) {
12011220
return napi_set_last_error(env, napi_invalid_arg);
12021221
}
12031222
} else if (p->method != nullptr) {
@@ -1213,17 +1232,28 @@ napi_status napi_define_properties(napi_env env,
12131232

12141233
CHECK_MAYBE_EMPTY(env, maybe_fn, napi_generic_failure);
12151234

1216-
auto define_maybe = obj->DefineOwnProperty(
1217-
context, property_name, maybe_fn.ToLocalChecked(), attributes);
1235+
v8::PropertyDescriptor descriptor(maybe_fn.ToLocalChecked(),
1236+
(p->attributes & napi_writable) != 0);
1237+
descriptor.set_enumerable((p->attributes & napi_enumerable) != 0);
1238+
descriptor.set_configurable((p->attributes & napi_configurable) != 0);
1239+
1240+
auto define_maybe = obj->DefineProperty(context,
1241+
property_name,
1242+
descriptor);
12181243

12191244
if (!define_maybe.FromMaybe(false)) {
12201245
return napi_set_last_error(env, napi_generic_failure);
12211246
}
12221247
} else {
12231248
v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(p->value);
12241249

1250+
v8::PropertyDescriptor descriptor(value,
1251+
(p->attributes & napi_writable) != 0);
1252+
descriptor.set_enumerable((p->attributes & napi_enumerable) != 0);
1253+
descriptor.set_configurable((p->attributes & napi_configurable) != 0);
1254+
12251255
auto define_maybe =
1226-
obj->DefineOwnProperty(context, property_name, value, attributes);
1256+
obj->DefineProperty(context, property_name, descriptor);
12271257

12281258
if (!define_maybe.FromMaybe(false)) {
12291259
return napi_set_last_error(env, napi_invalid_arg);

‎test/js-native-api/test_properties/test.js‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ const common = require('../../common');
33
const assert = require('assert');
44
const readonlyErrorRE =
55
/^TypeError: Cannot assign to read only property '.*' of object '#<Object>'$/;
6+
const getterOnlyErrorRE =
7+
/^TypeError: Cannot set property .* of #<Object> which has only a getter$/;
68

79
// Testing api calls for defining properties
810
const test_object = require(`./build/${common.buildType}/test_properties`);
@@ -41,14 +43,24 @@ const symbolDescription =
4143
assert.strictEqual(symbolDescription, 'NameKeySymbol');
4244

4345
// The napi_writable attribute should be ignored for accessors.
46+
const readwriteAccessor1Descriptor =
47+
Object.getOwnPropertyDescriptor(test_object, 'readwriteAccessor1');
48+
const readonlyAccessor1Descriptor =
49+
Object.getOwnPropertyDescriptor(test_object, 'readonlyAccessor1');
50+
assert.ok(readwriteAccessor1Descriptor.get != null);
51+
assert.ok(readwriteAccessor1Descriptor.set != null);
52+
assert.ok(readwriteAccessor1Descriptor.value === undefined);
53+
assert.ok(readonlyAccessor1Descriptor.get != null);
54+
assert.ok(readonlyAccessor1Descriptor.set === undefined);
55+
assert.ok(readonlyAccessor1Descriptor.value === undefined);
4456
test_object.readwriteAccessor1 = 1;
4557
assert.strictEqual(test_object.readwriteAccessor1, 1);
4658
assert.strictEqual(test_object.readonlyAccessor1, 1);
47-
assert.throws(() => { test_object.readonlyAccessor1 = 3; }, readonlyErrorRE);
59+
assert.throws(() => { test_object.readonlyAccessor1 = 3; }, getterOnlyErrorRE);
4860
test_object.readwriteAccessor2 = 2;
4961
assert.strictEqual(test_object.readwriteAccessor2, 2);
5062
assert.strictEqual(test_object.readonlyAccessor2, 2);
51-
assert.throws(() => { test_object.readonlyAccessor2 = 3; }, readonlyErrorRE);
63+
assert.throws(() => { test_object.readonlyAccessor2 = 3; }, getterOnlyErrorRE);
5264

5365
assert.strictEqual(test_object.hasNamedProperty(test_object, 'echo'), true);
5466
assert.strictEqual(test_object.hasNamedProperty(test_object, 'hiddenValue'),

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL