8000 n-api: make napi_get_property_names return strings · nodejs/node@e5fdc30 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Commit e5fdc30

Browse files
addaleaxtargos
addaleax
authored and
targos
committed
n-api: make napi_get_property_names return strings
The documentation says that this method returns an array of strings. Currently, it does not do so for indices. Resolve that by telling V8 explicitly to convert to string. PR-URL: #27524 Fixes: #27496 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent 3d37414 commit e5fdc30

File tree

3 files changed

+51
-1
lines changed

3 files changed

+51
-1
lines changed

‎src/js_native_api_v8.cc

+8-1
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,14 @@ napi_status napi_get_property_names(napi_env env,
870870
v8::Local<v8::Object> obj;
871871
CHECK_TO_OBJECT(env, context, obj, object);
872872

873-
auto maybe_propertynames = obj->GetPropertyNames(context);
873+
v8::MaybeLocal<v8::Array> maybe_propertynames = obj->GetPropertyNames(
874+
context,
875+
v8::KeyCollectionMode::kIncludePrototypes,
876+
static_cast<v8::PropertyFilter>(
877+
v8::PropertyFilter::ONLY_ENUMERABLE |
878+
v8::PropertyFilter::SKIP_SYMBOLS),
879+
v8::IndexFilter::kIncludeIndices,
880+
v8::KeyConversionMode::kConvertToString);
874881

875882
CHECK_MAYBE_EMPTY(env, maybe_propertynames, napi_generic_failure);
876883

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

+23
Original file line numberDiff line numberDiff line change
@@ -202,3 +202,26 @@ assert.strictEqual(newObject.test_string, 'test string');
202202
assert.strictEqual(test_object.Delete(obj, 'foo'), true);
203203
assert.strictEqual(obj.foo, 'baz');
204204
}
205+
206+
{
207+
// Verify that napi_get_property_names gets the right set of property names,
208+
// i.e.: includes prototypes, only enumerable properties, skips symbols,
209+
// and includes indices and converts them to strings.
210+
211+
const object = Object.create({
212+
inherited: 1
213+
});
214+
215+
object.normal = 2;
216+
object[Symbol('foo')] = 3;
217+
Object.defineProperty(object, 'unenumerable', {
218+
value: 4,
219+
enumerable: false,
220+
writable: true,
221+
configurable: true
222+
});
223+
object[5] = 5;
224+
225+
assert.deepStrictEqual(test_object.GetPropertyNames(object),
226+
['5', 'normal', 'inherited']);
227+
}

‎test/js-native-api/test_object/test_object.c

+20
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,25 @@ static napi_value GetNamed(napi_env env, napi_callback_info info) {
6363
return output;
6464
}
6565

66+
static napi_value GetPropertyNames(napi_env env, napi_callback_info info) {
67+
size_t argc = 1;
68+
napi_value args[1];
69+
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
70+
71+
NAPI_ASSERT(env, argc >= 1, "Wrong number of arguments");
72+
73+
napi_valuetype value_type0;
74+
NAPI_CALL(env, napi_typeof(env, args[0], &value_type0));
75+
76+
NAPI_ASSERT(env, value_type0 == napi_object,
77+
"Wrong type of arguments. Expects an object as first argument.");
78+
79+
napi_value output;
80+
NAPI_CALL(env, napi_get_property_names(env, args[0], &output));
81+
82+
return output;
83+
}
84+
6685
static napi_value Set(napi_env env, napi_callback_info info) {
6786
size_t argc = 3;
6887
napi_value args[3];
@@ -325,6 +344,7 @@ napi_value Init(napi_env env, napi_value exports) {
325344
napi_property_descriptor descriptors[] = {
326345
DECLARE_NAPI_PROPERTY("Get", Get),
327346
DECLARE_NAPI_PROPERTY("GetNamed", GetNamed),
347+
DECLARE_NAPI_PROPERTY("GetPropertyNames", GetPropertyNames),
328348
DECLARE_NAPI_PROPERTY("Set", Set),
329349
DECLARE_NAPI_PROPERTY("SetNamed", SetNamed),
330350
DECLARE_NAPI_PROPERTY("Has", Has),

0 commit comments

Comments
 (0)
0