Skip to content

Commit

Permalink
fix(cli/napi): correct name handling in napi property descriptor (den…
Browse files Browse the repository at this point in the history
  • Loading branch information
littledivy committed Feb 10, 2023
1 parent 46817a0 commit 39f131c
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 24 deletions.
8 changes: 5 additions & 3 deletions cli/napi/js_native_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1437,10 +1437,12 @@ fn napi_define_properties(
for property in properties {
let name = if !property.utf8name.is_null() {
let name_str = CStr::from_ptr(property.utf8name).to_str().unwrap();
v8::String::new(scope, name_str).ok_or(Error::GenericFailure)?
v8::String::new(scope, name_str)
.ok_or(Error::GenericFailure)?
.into()
} else {
let property_value = napi_value_unchecked(property.name);
v8::Local::<v8::String>::try_from(property_value)
v8::Local::<v8::Name>::try_from(property_value)
.map_err(|_| Error::NameExpected)?
};

Expand All @@ -1461,7 +1463,7 @@ fn napi_define_properties(
desc.set_enumerable(property.attributes & napi_enumerable != 0);
desc.set_configurable(property.attributes & napi_configurable != 0);

let define_maybe = object.define_property(scope, name.into(), &desc);
let define_maybe = object.define_property(scope, name, &desc);
return_status_if_false!(
env_ptr,
!define_maybe.unwrap_or(false),
Expand Down
6 changes: 6 additions & 0 deletions test_napi/properties_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,10 @@ Deno.test("napi properties", () => {
assertEquals(properties.test_simple_property, {
nice: 69,
});

assertEquals(properties.key_v8_string, 1);
const symbols = Object.getOwnPropertySymbols(properties);
assertEquals(symbols.length, 1);
assertEquals(symbols[0].description, "key_v8_symbol");
assertEquals(properties[symbols[0]], 1);
});
41 changes: 20 additions & 21 deletions test_napi/src/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,27 +82,26 @@ pub fn init(env: napi_env, exports: napi_value) {
attributes: enumerable,
value: number,
},
// TODO(@littledivy): Fix this.
// napi_property_descriptor {
// utf8name: ptr::null(),
// name: name_value,
// method: None,
// getter: None,
// setter: None,
// data: ptr::null_mut(),
// attributes: enumerable,
// value: number,
// },
// napi_property_descriptor {
// utf8name: ptr::null(),
// name: name_symbol,
// method: None,
// getter: None,
// setter: None,
// data: ptr::null_mut(),
// attributes: enumerable,
// value: number,
// },
napi_property_descriptor {
utf8name: ptr::null(),
name: name_value,
method: None,
getter: None,
setter: None,
data: ptr::null_mut(),
attributes: enumerable,
value: number,
},
napi_property_descriptor {
utf8name: ptr::null(),
name: name_symbol,
method: None,
getter: None,
setter: None,
data: ptr::null_mut(),
attributes: enumerable,
value: number,
},
];

assert_napi_ok!(napi_define_properties(
Expand Down

0 comments on commit 39f131c

Please sign in to comment.