diff --git a/src/node_api.cc b/src/node_api.cc index b623580f2f7dc1..070caaa43f6040 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -1,4 +1,4 @@ -/****************************************************************************** +/****************************************************************************** * Experimental prototype for demonstrating VM agnostic and ABI stable API * for native modules to use instead of using Nan and V8 APIs directly. * @@ -35,18 +35,27 @@ class napi_env__ { namespace v8impl { // convert from n-api property attributes to v8::PropertyAttribute -static inline v8::PropertyAttribute V8PropertyAttributesFromAttributes( - napi_property_attributes attributes) { - unsigned int attribute_flags = v8::None; - if ((attributes & napi_writable) == 0) { - attribute_flags |= v8::ReadOnly; +static inline v8::PropertyAttribute V8PropertyAttributesFromDescriptor( + const napi_property_descriptor* descriptor) { + unsigned int attribute_flags = v8::PropertyAttribute::None; + + if (descriptor->getter != nullptr || descriptor->setter != nullptr) { + // The napi_writable attribute is ignored for accessor descriptors, but + // V8 requires the ReadOnly attribute to match nonexistence of a setter. + attribute_flags |= (descriptor->setter == nullptr ? + v8::PropertyAttribute::ReadOnly : v8::PropertyAttribute::None); + } + else if ((descriptor->attributes & napi_writable) == 0) { + attribute_flags |= v8::PropertyAttribute::ReadOnly; } - if ((attributes & napi_enumerable) == 0) { - attribute_flags |= v8::DontEnum; + + if ((descriptor->attributes & napi_enumerable) == 0) { + attribute_flags |= v8::PropertyAttribute::DontEnum; } - if ((attributes & napi_configurable) == 0) { - attribute_flags |= v8::DontDelete; + if ((descriptor->attributes & napi_configurable) == 0) { + attribute_flags |= v8::PropertyAttribute::DontDelete; } + return static_cast(attribute_flags); } @@ -784,34 +793,14 @@ napi_status napi_define_class(napi_env env, v8::Local property_name; CHECK_NEW_FROM_UTF8(env, property_name, p->utf8name); v8::PropertyAttribute attributes = - v8impl::V8PropertyAttributesFromAttributes(p->attributes); + v8impl::V8PropertyAttributesFromDescriptor(p); - // This code is similar to that in napi_define_property(); the + // This code is similar to that in napi_define_properties(); the // difference is it applies to a template instead of an object. - if (p->method != nullptr) { - v8::Local cbdata = - v8impl::CreateFunctionCallbackData(env, p->method, p->data); - - RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure); - - v8::Local t = - v8::FunctionTemplate::New(isolate, - v8impl::FunctionCallbackWrapper::Invoke, - cbdata, - v8::Signature::New(isolate, tpl)); - t->SetClassName(property_name); - - tpl->PrototypeTemplate()->Set(property_name, t, attributes); - } else if (p->getter != nullptr || p->setter != nullptr) { + if (p->getter != nullptr || p->setter != nullptr) { v8::Local cbdata = v8impl::CreateAccessorCallbackData( env, p->getter, p->setter, p->data); - // The napi_writable attribute is ignored for accessor descriptors, - // but V8 requires the ReadOnly attribute to match existence of a setter. - attributes = static_cast(p->setter != nullptr - ? attributes & ~v8::PropertyAttribute::ReadOnly - : attributes | v8::PropertyAttribute::ReadOnly); - tpl->PrototypeTemplate()->SetAccessor( property_name, p->getter ? v8impl::GetterCallbackWrapper::Invoke : nullptr, @@ -819,6 +808,20 @@ napi_status napi_define_class(napi_env env, cbdata, v8::AccessControl::DEFAULT, attributes); + } else if (p->method != nullptr) { + v8::Local cbdata = + v8impl::CreateFunctionCallbackData(env, p->method, p->data); + + RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure); + + v8::Local t = + v8::FunctionTemplate::New(isolate, + v8impl::FunctionCallbackWrapper::Invoke, + cbdata, + v8::Signature::New(isolate, tpl)); + t->SetClassName(property_name); + + tpl->PrototypeTemplate()->Set(property_name, t, attributes); } else { v8::Local value = v8impl::V8LocalValueFromJsValue(p->value); tpl->PrototypeTemplate()->Set(property_name, value, attributes); @@ -1100,9 +1103,28 @@ napi_status napi_define_properties(napi_env env, CHECK_NEW_FROM_UTF8(env, name, p->utf8name); v8::PropertyAttribute attributes = - v8impl::V8PropertyAttributesFromAttributes(p->attributes); + v8impl::V8PropertyAttributesFromDescriptor(p); - if (p->method != nullptr) { + if (p->getter != nullptr || p->setter != nullptr) { + v8::Local cbdata = v8impl::CreateAccessorCallbackData( + env, + p->getter, + p->setter, + p->data); + + auto set_maybe = obj->SetAccessor( + context, + name, + p->getter ? v8impl::GetterCallbackWrapper::Invoke : nullptr, + p->setter ? v8impl::SetterCallbackWrapper::Invoke : nullptr, + cbdata, + v8::AccessControl::DEFAULT, + attributes); + + if (!set_maybe.FromMaybe(false)) { + return napi_set_last_error(env, napi_invalid_arg); + } + } else if (p->method != nullptr) { v8::Local cbdata = v8impl::CreateFunctionCallbackData(env, p->method, p->data); @@ -1117,31 +1139,6 @@ napi_status napi_define_properties(napi_env env, if (!define_maybe.FromMaybe(false)) { return napi_set_last_error(env, napi_generic_failure); } - } else if (p->getter != nullptr || p->setter != nullptr) { - v8::Local cbdata = v8impl::CreateAccessorCallbackData( - env, - p->getter, - p->setter, - p->data); - - // The napi_writable attribute is ignored for accessor descriptors, - // but V8 requires the ReadOnly attribute to match existence of a setter. - attributes = static_cast(p->setter != nullptr - ? attributes & ~v8::PropertyAttribute::ReadOnly - : attributes | v8::PropertyAttribute::ReadOnly); - - auto set_maybe = obj->SetAccessor( - context, - name, - p->getter ? v8impl::GetterCallbackWrapper::Invoke : nullptr, - p->setter ? v8impl::SetterCallbackWrapper::Invoke : nullptr, - cbdata, - v8::AccessControl::DEFAULT, - attributes); - - if (!set_maybe.FromMaybe(false)) { - return napi_set_last_error(env, napi_invalid_arg); - } } else { v8::Local value = v8impl::V8LocalValueFromJsValue(p->value); diff --git a/test/addons-napi/test_constructor/test.js b/test/addons-napi/test_constructor/test.js index 9feae5a1360b2c..b1762c2253b99a 100644 --- a/test/addons-napi/test_constructor/test.js +++ b/test/addons-napi/test_constructor/test.js @@ -17,7 +17,7 @@ assert.throws(() => { test_object.readonlyValue = 3; }); assert.ok(test_object.hiddenValue); -// All properties except 'hiddenValue' should be enumerable. +// Properties with napi_enumerable attribute should be enumerable. const propertyNames = []; for (const name in test_object) { propertyNames.push(name); @@ -26,3 +26,17 @@ assert.ok(propertyNames.indexOf('echo') >= 0); assert.ok(propertyNames.indexOf('readwriteValue') >= 0); assert.ok(propertyNames.indexOf('readonlyValue') >= 0); assert.ok(propertyNames.indexOf('hiddenValue') < 0); +assert.ok(propertyNames.indexOf('readwriteAccessor1') < 0); +assert.ok(propertyNames.indexOf('readwriteAccessor2') < 0); +assert.ok(propertyNames.indexOf('readonlyAccessor1') < 0); +assert.ok(propertyNames.indexOf('readonlyAccessor2') < 0); + +// The napi_writable attribute should be ignored for accessors. +test_object.readwriteAccessor1 = 1; +assert.strictEqual(test_object.readwriteAccessor1, 1); +assert.strictEqual(test_object.readonlyAccessor1, 1); +assert.throws(() => { test_object.readonlyAccessor1 = 3; }); +test_object.readwriteAccessor2 = 2; +assert.strictEqual(test_object.readwriteAccessor2, 2); +assert.strictEqual(test_object.readonlyAccessor2, 2); +assert.throws(() => { test_object.readonlyAccessor2 = 3; }); diff --git a/test/addons-napi/test_constructor/test_constructor.c b/test/addons-napi/test_constructor/test_constructor.c index 689342f12dd43b..d0382f2c65ba5e 100644 --- a/test/addons-napi/test_constructor/test_constructor.c +++ b/test/addons-napi/test_constructor/test_constructor.c @@ -83,10 +83,13 @@ void Init(napi_env env, napi_value exports, napi_value module, void* priv) { napi_property_descriptor properties[] = { { "echo", Echo, 0, 0, 0, napi_enumerable, 0 }, - { "accessorValue", 0, GetValue, SetValue, 0, napi_enumerable, 0}, { "readwriteValue", 0, 0, 0, number, napi_enumerable | napi_writable, 0 }, { "readonlyValue", 0, 0, 0, number, napi_enumerable, 0}, { "hiddenValue", 0, 0, 0, number, napi_default, 0}, + { "readwriteAccessor1", 0, GetValue, SetValue, 0, napi_default, 0}, + { "readwriteAccessor2", 0, GetValue, SetValue, 0, napi_writable, 0}, + { "readonlyAccessor1", 0, GetValue, NULL, 0, napi_default, 0}, + { "readonlyAccessor2", 0, GetValue, NULL, 0, napi_writable, 0}, }; napi_value cons; diff --git a/test/addons-napi/test_properties/test.js b/test/addons-napi/test_properties/test.js index 8e19903dcfe6e6..fb0c19ceedb086 100644 --- a/test/addons-napi/test_properties/test.js +++ b/test/addons-napi/test_properties/test.js @@ -16,7 +16,7 @@ assert.throws(() => { test_object.readonlyValue = 3; }); assert.ok(test_object.hiddenValue); -// All properties except 'hiddenValue' should be enumerable. +// Properties with napi_enumerable attribute should be enumerable. const propertyNames = []; for (const name in test_object) { propertyNames.push(name); @@ -25,3 +25,17 @@ assert.ok(propertyNames.indexOf('echo') >= 0); assert.ok(propertyNames.indexOf('readwriteValue') >= 0); assert.ok(propertyNames.indexOf('readonlyValue') >= 0); assert.ok(propertyNames.indexOf('hiddenValue') < 0); +assert.ok(propertyNames.indexOf('readwriteAccessor1') < 0); +assert.ok(propertyNames.indexOf('readwriteAccessor2') < 0); +assert.ok(propertyNames.indexOf('readonlyAccessor1') < 0); +assert.ok(propertyNames.indexOf('readonlyAccessor2') < 0); + +// The napi_writable attribute should be ignored for accessors. +test_object.readwriteAccessor1 = 1; +assert.strictEqual(test_object.readwriteAccessor1, 1); +assert.strictEqual(test_object.readonlyAccessor1, 1); +assert.throws(() => { test_object.readonlyAccessor1 = 3; }); +test_object.readwriteAccessor2 = 2; +assert.strictEqual(test_object.readwriteAccessor2, 2); +assert.strictEqual(test_object.readonlyAccessor2, 2); +assert.throws(() => { test_object.readonlyAccessor2 = 3; }); diff --git a/test/addons-napi/test_properties/test_properties.c b/test/addons-napi/test_properties/test_properties.c index 137a45f57039bd..de6b39ba027577 100644 --- a/test/addons-napi/test_properties/test_properties.c +++ b/test/addons-napi/test_properties/test_properties.c @@ -71,10 +71,13 @@ void Init(napi_env env, napi_value exports, napi_value module, void* priv) { napi_property_descriptor properties[] = { { "echo", Echo, 0, 0, 0, napi_enumerable, 0 }, - { "accessorValue", 0, GetValue, SetValue, 0, napi_enumerable, 0}, { "readwriteValue", 0, 0, 0, number, napi_enumerable | napi_writable, 0 }, { "readonlyValue", 0, 0, 0, number, napi_enumerable, 0}, { "hiddenValue", 0, 0, 0, number, napi_default, 0}, + { "readwriteAccessor1", 0, GetValue, SetValue, 0, napi_default, 0}, + { "readwriteAccessor2", 0, GetValue, SetValue, 0, napi_writable, 0}, + { "readonlyAccessor1", 0, GetValue, NULL, 0, napi_default, 0}, + { "readonlyAccessor2", 0, GetValue, NULL, 0, napi_writable, 0}, }; status = napi_define_properties(