Repository: denoland/deno
Pull request: fix(ext/napi): don't mark accessor properties as read-only in napi_define_class

## Summary

- In `napi_define_class`, accessor properties (getter/setter) were incorrectly marked `READ_ONLY` when `napi_writable` was not set in the property attributes. Per the Node-API spec, `napi_writable` only applies to data properties — for accessor properties, writability is determined by the presence of a setter.
- Removed the incorrect `READ_ONLY` flag application for accessor properties in `napi_define_class`.
- Added a test class (`NapiAccessorObject`) with getter/setter properties using `napi_default` attributes to verify the fix.

Closes https://github.com/denoland/deno/issues/32100

## Test plan

- [x] New test `"napi class accessor property is writable"` verifies assignment through setter works on accessor properties defined with `napi_default` attributes
- [x] All existing NAPI tests pass (49 passed, 0 failed)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Changed files:
filename: ext/napi/js_native_api.rs
patch: @@ -285,13 +285,6 @@ fn napi_define_class<'s>(
       let setter = p
         .setter
         .map(|s| create_function_template(scope, env_ptr, None, s, p.data));
-      if getter.is_some()
-        && setter.is_some()
-        && (p.attributes & napi_writable) == 0
-      {
-        accessor_property =
-          accessor_property | v8::PropertyAttribute::READ_ONLY;
-      }
       let proto = tpl.prototype_template(scope);
       proto.set_accessor_property(name, getter, setter, accessor_property);
     } else if let Some(method) = p.method {

filename: tests/napi/object_wrap_test.js
patch: @@ -41,6 +41,16 @@ Deno.test("napi external arraybuffer", function () {
   buf = null;
 });
 
+Deno.test("napi class accessor property is writable", function () {
+  // Accessor properties (getter/setter) defined via napi_define_class
+  // should be writable via the setter even when napi_writable is not
+  // set in the attributes. napi_writable only applies to data properties.
+  const obj = new objectWrap.NapiAccessorObject();
+  assertEquals(obj.value, 0);
+  obj.value = 42;
+  assertEquals(obj.value, 42);
+});
+
 Deno.test("napi object wrap userland owned", function () {
   let obj = new objectWrap.NapiObjectOwned(1);
   assertEquals(obj.get_value(), 1);

filename: tests/napi/src/object_wrap.rs
patch: @@ -171,6 +171,88 @@ impl NapiObject {
   }
 }
 
+/// A class that uses getter/setter accessor properties defined via
+/// napi_define_class with napi_default (0) attributes. This tests that
+/// napi_writable is correctly ignored for accessor properties — the
+/// presence of a setter should make the property writable regardless
+/// of whether napi_writable is set.
+pub struct NapiAccessorObject {
+  value: i32,
+}
+
+extern "C" fn accessor_obj_constructor(
+  env: napi_env,
+  info: napi_callback_info,
+) -> napi_value {
+  let mut this: napi_value = ptr::null_mut();
+  assert_napi_ok!(napi_get_cb_info(
+    env,
+    info,
+    &mut 0,
+    ptr::null_mut(),
+    &mut this,
+    ptr::null_mut(),
+  ));
+
+  let obj = Box::new(NapiAccessorObject { value: 0 });
+  let obj_raw = Box::into_raw(obj) as *mut c_void;
+  assert_napi_ok!(napi_wrap(
+    env,
+    this,
+    obj_raw,
+    Some(accessor_obj_release),
+    ptr::null_mut(),
+    ptr::null_mut(),
+  ));
+
+  this
+}
+
+extern "C" fn accessor_obj_release(
+  _env: napi_env,
+  data: *mut c_void,
+  _hint: *mut c_void,
+) {
+  unsafe {
+    drop(Box::from_raw(data as *mut NapiAccessorObject));
+  }
+}
+
+extern "C" fn accessor_getter(
+  env: napi_env,
+  info: napi_callback_info,
+) -> napi_value {
+  let (_args, _argc, this) = napi_get_callback_info!(env, info, 0);
+  let mut obj: *mut NapiAccessorObject = ptr::null_mut();
+  assert_napi_ok!(napi_unwrap(
+    env,
+    this,
+    &mut obj as *mut _ as *mut *mut c_void
+  ));
+
+  let mut result: napi_value = ptr::null_mut();
+  assert_napi_ok!(napi_create_int32(env, (*obj).value, &mut result));
+  result
+}
+
+extern "C" fn accessor_setter(
+  env: napi_env,
+  info: napi_callback_info,
+) -> napi_value {
+  let (args, argc, this) = napi_get_callback_info!(env, info, 1);
+  assert_eq!(argc, 1);
+  let mut obj: *mut NapiAccessorObject = ptr::null_mut();
+  assert_napi_ok!(napi_unwrap(
+    env,
+    this,
+    &mut obj as *mut _ as *mut *mut c_void
+  ));
+
+  assert_napi_ok!(napi_get_value_int32(env, args[0], &mut (*obj).value));
+
+  ptr::null_mut()
+}
+
 pub fn init(env: napi_env, exports: napi_value) {
   let mut static_prop = napi_new_property!(env, "factory", NapiObject::factory);
   static_prop.attributes = PropertyAttributes::static_;
@@ -219,4 +301,36 @@ pub fn init(env: napi_env, exports: napi_value) {
     "NapiObjectOwned\0".as_ptr() as *const c_char,
     cons,
   ));
+
+  // Register NapiAccessorObject — class with getter/setter properties
+  // using napi_default attributes (napi_writable NOT set).
+  let accessor_properties = &[napi_property_descriptor {
+    utf8name: c"value".as_ptr(),
+    name: ptr::null_mut(),
+    method: None,
+    getter: Some(accessor_getter),
+    setter: Some(accessor_setter),
+    data: ptr::null_mut(),
+    attributes: PropertyAttributes::default, // napi_writable is NOT set
+    value: ptr::null_mut(),
+  }];
+
+  let mut cons: napi_value = ptr::null_mut();
+  assert_napi_ok!(napi_define_class(
+    env,
+    c"NapiAccessorObject".as_ptr(),
+    usize::MAX,
+    Some(accessor_obj_constructor),
+    ptr::null_mut(),
+    accessor_properties.len(),
+    accessor_properties.as_ptr(),
+    &mut cons,
+  ));
+
+  assert_napi_ok!(napi_set_named_property(
+    env,
+    exports,
+    c"NapiAccessorObject".as_ptr(),
+    cons,
+  ));
 }