diff options
author | legendecas <legendecas@gmail.com> | 2019-05-24 21:48:21 +0800 |
---|---|---|
committer | Ruben Bridgewater <ruben@bridgewater.de> | 2019-06-17 12:06:06 +0200 |
commit | 432958528ea5a68d68afe0f0bc379abd3f35e8a0 (patch) | |
tree | d90b736293bc7b6a3930c172b6f7d70a71ef137b | |
parent | 44f18d236b1bc573cbf4bb8488dda6e1b1c3cab0 (diff) | |
download | android-node-v8-432958528ea5a68d68afe0f0bc379abd3f35e8a0.tar.gz android-node-v8-432958528ea5a68d68afe0f0bc379abd3f35e8a0.tar.bz2 android-node-v8-432958528ea5a68d68afe0f0bc379abd3f35e8a0.zip |
n-api: define ECMAScript-compliant accessors on napi_define_properties
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>
-rw-r--r-- | src/js_native_api_v8.cc | 70 | ||||
-rw-r--r-- | test/js-native-api/test_properties/test.js | 16 |
2 files changed, 64 insertions, 22 deletions
diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 1131e3a6d1..b99f08bbea 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -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); + + 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(); + } - if (!set_maybe.FromMaybe(false)) { + 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); diff --git a/test/js-native-api/test_properties/test.js b/test/js-native-api/test_properties/test.js index fa4cb587d6..704002dfc7 100644 --- a/test/js-native-api/test_properties/test.js +++ b/test/js-native-api/test_properties/test.js @@ -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'), |