fix: setters on readonly properties

Brooooooklyn · Jan 19, 2021 · adac797 · adac797
1 parent faca57b
commit adac797
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 43 deletions.
diff --git a/__test__/image-data.spec.ts b/__test__/image-data.spec.ts
@@ -38,36 +38,17 @@ test('should throw if width * height * 4 not equal to arraybuffer length', (t) =
 test('properties should be readonly', (t) => {
   const imageData = new ImageData(1024, 768)
   const fakeData = new Uint8ClampedArray()
-  t.throws(
-    () => {
-      // @ts-expect-error
-      imageData.data = fakeData
-    },
-    {
-      instanceOf: TypeError,
-      message: /Cannot assign to read only property/,
-    },
-  )
+  const expectation = {
+    instanceOf: TypeError,
+    message: /Cannot assign to read only property/,
+  }
 
-  t.throws(
-    () => {
-      // @ts-expect-error
-      imageData.width = 1111
-    },
-    {
-      instanceOf: TypeError,
-      message: /Cannot assign to read only property/,
-    },
-  )
+  // @ts-expect-error
+  t.throws(() => (imageData.data = fakeData), expectation)
 
-  t.throws(
-    () => {
-      // @ts-expect-error
-      imageData.height = 2222
-    },
-    {
-      instanceOf: TypeError,
-      message: /Cannot assign to read only property/,
-    },
-  )
+  // @ts-expect-error
+  t.throws(() => (imageData.width = 114), expectation)
+
+  // @ts-expect-error
+  t.throws(() => (imageData.height = 514), expectation)
 })
diff --git a/__test__/image.spec.ts b/__test__/image.spec.ts
@@ -44,3 +44,26 @@ test('alt state should be ok', (t) => {
   image.alt = 'hello'
   t.is(image.alt, 'hello')
 })
+
+test('properties should be readonly', (t) => {
+  const image = new Image()
+  const expectation = {
+    instanceOf: TypeError,
+    message: /Cannot set property (.)* of #<Image> which has only a getter/,
+  }
+
+  // @ts-expect-error
+  t.throws(() => (image.width = 114), expectation)
+
+  // @ts-expect-error
+  t.throws(() => (image.height = 514), expectation)
+
+  // @ts-expect-error
+  t.throws(() => (image.naturalWidth = 1919), expectation)
+
+  // @ts-expect-error
+  t.throws(() => (image.naturalHeight = 810), expectation)
+
+  // @ts-expect-error
+  t.throws(() => (image.complete = true), expectation)
+})
diff --git a/src/image.rs b/src/image.rs
@@ -133,26 +133,26 @@ impl Image {
       image_constructor,
       &vec![
         Property::new(&env, "width")?
-          .with_setter(set_noop)
-          .with_getter(get_width),
+          .with_getter(get_width)
+          .with_property_attributes(PropertyAttributes::Enumerable),
         Property::new(&env, "height")?
-          .with_setter(set_noop)
-          .with_getter(get_height),
+          .with_getter(get_height)
+          .with_property_attributes(PropertyAttributes::Enumerable),
         Property::new(&env, "naturalWidth")?
-          .with_setter(set_noop)
-          .with_getter(get_width),
+          .with_getter(get_width)
+          .with_property_attributes(PropertyAttributes::Enumerable),
         Property::new(&env, "naturalHeight")?
-          .with_setter(set_noop)
-          .with_getter(get_height),
+          .with_getter(get_height)
+          .with_property_attributes(PropertyAttributes::Enumerable),
         Property::new(&env, "complete")?
-          .with_setter(set_noop)
-          .with_getter(get_complete),
+          .with_getter(get_complete)
+          .with_property_attributes(PropertyAttributes::Enumerable),
         Property::new(&env, "alt")?
           .with_setter(set_alt)
           .with_getter(get_alt),
         Property::new(&env, "src")?
           .with_setter(set_src)
-          .with_getter(set_noop),
+          .with_getter(get_src),
       ],
     )
   }
@@ -217,8 +217,8 @@ fn set_alt(ctx: CallContext) -> Result<JsUndefined> {
   ctx.env.get_undefined()
 }
 
-#[js_function(1)]
-fn set_noop(ctx: CallContext) -> Result<JsUndefined> {
+#[js_function]
+fn get_src(ctx: CallContext) -> Result<JsUndefined> {
   ctx.env.get_undefined()
 }