fix(cli): set napi object property properly (#26344)

<!--
Before submitting a PR, please read
https://docs.deno.com/runtime/manual/references/contributing

1. Give the PR a descriptive title.

  Examples of good title:
    - fix(std/http): Fix race condition in server
    - docs(console): Update docstrings
    - feat(doc): Handle nested reexports

  Examples of bad title:
    - fix #7123
    - update docs
    - fix bugs

2. Ensure there is a related issue and it is referenced in the PR text.
3. Ensure there are tests that cover the changes.
4. Ensure `cargo test` passes.
5. Ensure `./tools/format.js` passes without changing files.
6. Ensure `./tools/lint.js` passes.
7. Open as a draft PR if your work is still in progress. The CI won't
run
   all steps, but you can add '[ci]' to a commit message to force it to.
8. If you would like to run the benchmarks on the CI, add the 'ci-bench'
label.
-->

cli/napi/js_native_api.rs

@@ -264,6 +264,16 @@ fn napi_define_class<'s>(
       Err(status) => return status,
     };
 
+    let mut accessor_property = v8::PropertyAttribute::NONE;
+
+    if p.attributes & napi_enumerable == 0 {
+      accessor_property = accessor_property | v8::PropertyAttribute::DONT_ENUM;
+    }
+    if p.attributes & napi_configurable == 0 {
+      accessor_property =
+        accessor_property | v8::PropertyAttribute::DONT_DELETE;
+    }
+
     if p.getter.is_some() || p.setter.is_some() {
       let getter = p.getter.map(|g| {
         create_function_template(&mut env.scope(), env_ptr, None, g, p.data)
@@ -271,8 +281,6 @@ fn napi_define_class<'s>(
       let setter = p.setter.map(|s| {
         create_function_template(&mut env.scope(), env_ptr, None, s, p.data)
       });
-
-      let mut accessor_property = v8::PropertyAttribute::NONE;
       if getter.is_some()
         && setter.is_some()
         && (p.attributes & napi_writable) == 0
@@ -280,15 +288,6 @@ fn napi_define_class<'s>(
         accessor_property =
           accessor_property | v8::PropertyAttribute::READ_ONLY;
       }
-      if p.attributes & napi_enumerable == 0 {
-        accessor_property =
-          accessor_property | v8::PropertyAttribute::DONT_ENUM;
-      }
-      if p.attributes & napi_configurable == 0 {
-        accessor_property =
-          accessor_property | v8::PropertyAttribute::DONT_DELETE;
-      }
-
       let proto = tpl.prototype_template(&mut env.scope());
       proto.set_accessor_property(name, getter, setter, accessor_property);
     } else if let Some(method) = p.method {
@@ -300,10 +299,14 @@ fn napi_define_class<'s>(
         p.data,
       );
       let proto = tpl.prototype_template(&mut env.scope());
-      proto.set(name, function.into());
+      proto.set_with_attr(name, function.into(), accessor_property);
     } else {
       let proto = tpl.prototype_template(&mut env.scope());
-      proto.set(name, p.value.unwrap().into());
+      if (p.attributes & napi_writable) == 0 {
+        accessor_property =
+          accessor_property | v8::PropertyAttribute::READ_ONLY;
+      }
+      proto.set_with_attr(name, p.value.unwrap().into(), accessor_property);
     }
   }
 

tests/napi/object_test.js

@@ -1,6 +1,11 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
-import { assert, assertEquals, loadTestLibrary } from "./common.js";
+import {
+  assert,
+  assertEquals,
+  assertThrows,
+  loadTestLibrary,
+} from "./common.js";
 
 const object = loadTestLibrary();
 
@@ -12,4 +17,22 @@ Deno.test("napi object", function () {
 
   const r1 = object.test_object_get(r);
   assert(r === r1);
+
+  const r2 = object.test_object_attr_property(r);
+  assert(r === r2);
+  assertThrows(
+    () => {
+      r2.self = "2";
+    },
+    Error,
+    "Cannot assign to read only property 'self' of object '#<Object>'",
+  );
+
+  assertThrows(
+    () => {
+      r2.method = () => {};
+    },
+    Error,
+    "Cannot assign to read only property 'method' of object '#<Object>'",
+  );
 });

tests/napi/src/object.rs

@@ -40,10 +40,39 @@ extern "C" fn test_object_get(
   obj
 }
 
+extern "C" fn test_object_attr_property(
+  env: napi_env,
+  info: napi_callback_info,
+) -> napi_value {
+  let (args, argc, _) = napi_get_callback_info!(env, info, 1);
+  assert_eq!(argc, 1);
+
+  let obj = args[0];
+  let mut property = napi_new_property!(env, "self", test_object_new);
+  property.attributes = PropertyAttributes::enumerable;
+  property.method = None;
+  property.value = obj;
+  let mut method_property = napi_new_property!(env, "method", test_object_new);
+  method_property.attributes = PropertyAttributes::enumerable;
+  let properties = &[property, method_property];
+  assert_napi_ok!(napi_define_properties(
+    env,
+    obj,
+    properties.len(),
+    properties.as_ptr()
+  ));
+  obj
+}
+
 pub fn init(env: napi_env, exports: napi_value) {
   let properties = &[
     napi_new_property!(env, "test_object_new", test_object_new),
     napi_new_property!(env, "test_object_get", test_object_get),
+    napi_new_property!(
+      env,
+      "test_object_attr_property",
+      test_object_attr_property
+    ),
   ];
 
   assert_napi_ok!(napi_define_properties(