fix(cli/napi): correct name handling in napi property descriptor (#17716)

Fixes https://github.com/denoland/deno/issues/17712

3 files changed, 31 insertions, 24 deletions

diff --git a/cli/napi/js_native_api.rs b/cli/napi/js_native_api.rs
index d531c72c8..e79307714 100644
--- a/cli/napi/js_native_api.rs
+++ b/cli/napi/js_native_api.rs
@@ -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)?
     };
 
@@ -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),
diff --git a/test_napi/properties_test.js b/test_napi/properties_test.js
index 36ede1033..b5f0c0794 100644
--- a/test_napi/properties_test.js
+++ b/test_napi/properties_test.js
@@ -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);
 });
diff --git a/test_napi/src/properties.rs b/test_napi/src/properties.rs
index 1b6c9488b..339699a05 100644
--- a/test_napi/src/properties.rs
+++ b/test_napi/src/properties.rs
@@ -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(