Allow deserialization of missing objects properties into Option<> (#3767)

* Allow deserialization of missing objects properties into Option<>

Currently the TryFromJs derive macro requires that all properties
be defined in the object, whether they are declared as Option<> or
not. This commit makes it so if the field is not found in the
JsObject, we try to deserialize undefined.

This is more in line with JavaScript behaviour.

* Fix cargo fmt

* Fix clippies

* Empty commit to trigger rerun of workflow

* Remove circular dependency between boa_engine and boa_macros

* Missing "::" in macro

---------

Co-authored-by: jedel1043 <jedel0124@gmail.com>

Cargo.lock

@@ -517,7 +517,6 @@ name = "boa_macros_tests"
 version = "0.18.0"
 dependencies = [
  "boa_engine",
- "boa_macros",
  "trybuild",
 ]
 

Cargo.toml

@@ -71,7 +71,7 @@ serde = "1.0.197"
 static_assertions = "1.1.0"
 textwrap = "0.16.0"
 thin-vec = "0.2.13"
-time = {version = "0.3.34", no-default-features = true, features = ["local-offset", "large-dates", "wasm-bindgen", "parsing", "formatting", "macros"]}
+time = {version = "0.3.34", default-features = false, features = ["local-offset", "large-dates", "wasm-bindgen", "parsing", "formatting", "macros"]}
 tinystr = "0.7.5"
 log = "0.4.21"
 simple_logger = "4.3.3"

core/macros/src/lib.rs

@@ -418,30 +418,19 @@ fn generate_conversion(fields: FieldsNamed) -> Result<proc_macro2::TokenStream,
             .map_err(|err| vec![err])?;
         }
 
-        if let Some(method) = from_js_with {
-            let ident = Ident::new(&method.value(), method.span());
         final_fields.push(quote! {
-                let #name = #ident(props.get(&::boa_engine::js_string!(#name_str).into()).ok_or_else(|| {
+            let #name = match props.get(&::boa_engine::js_string!(#name_str).into()) {
-                    ::boa_engine::JsError::from(
+                Some(pd) => pd.value().ok_or_else(|| ::boa_engine::JsError::from(
-                        boa_engine::JsNativeError::typ().with_message(#error_str)
+                        ::boa_engine::JsNativeError::typ().with_message(#error_str)
-                    )
+                    ))?.clone().try_js_into(context)?,
-                })?.value().ok_or_else(|| {
+                None => ::boa_engine::JsValue::undefined().try_js_into(context)?,
-                    ::boa_engine::JsError::from(
+            };
-                        boa_engine::JsNativeError::typ().with_message(#error_str)
-                    )
-                })?, context)?;
         });
-        } else {
+
+        if let Some(method) = from_js_with {
+            let ident = Ident::new(&method.value(), method.span());
             final_fields.push(quote! {
-                let #name = props.get(&::boa_engine::js_string!(#name_str).into()).ok_or_else(|| {
+                let #name = #ident(&#name, context)?;
-                    ::boa_engine::JsError::from(
-                        boa_engine::JsNativeError::typ().with_message(#error_str)
-                    )
-                })?.value().ok_or_else(|| {
-                    ::boa_engine::JsError::from(
-                        boa_engine::JsNativeError::typ().with_message(#error_str)
-                    )
-                })?.clone().try_js_into(context)?;
             });
         }
     }

tests/macros/Cargo.toml

@@ -12,7 +12,6 @@ rust-version.workspace = true
 
 [dev-dependencies]
 trybuild = "1.0.90"
-boa_macros.workspace = true
 boa_engine.workspace = true
 
 [lints]

tests/macros/tests/optional.rs

@@ -0,0 +1,67 @@
+#![allow(unused_crate_dependencies)]
+
+use boa_engine::value::TryFromJs;
+use boa_engine::Source;
+
+#[derive(PartialEq, Eq, TryFromJs)]
+struct Deserialize {
+    required: String,
+    optional: Option<String>,
+}
+
+#[test]
+fn optional_missing_try_from_js() {
+    let mut context = boa_engine::Context::default();
+    let value = context
+        .eval(Source::from_bytes(
+            r#"
+            let empty = {
+                "required":"foo",
+            };
+            empty
+        "#,
+        ))
+        .unwrap();
+
+    let deserialized: Deserialize = Deserialize::try_from_js(&value, &mut context).unwrap();
+    assert_eq!(deserialized.required, "foo");
+    assert_eq!(deserialized.optional, None);
+}
+
+#[test]
+fn optional_try_from_js() {
+    let mut context = boa_engine::Context::default();
+    let value = context
+        .eval(Source::from_bytes(
+            r#"
+            let empty = {
+                "required": "foo",
+                "optional": "bar",
+            };
+            empty
+        "#,
+        ))
+        .unwrap();
+
+    let deserialized: Deserialize = Deserialize::try_from_js(&value, &mut context).unwrap();
+    assert_eq!(deserialized.required, "foo");
+    assert_eq!(deserialized.optional, Some("bar".to_string()));
+}
+
+#[test]
+fn required_missing_try_from_js() {
+    let mut context = boa_engine::Context::default();
+    let value = context
+        .eval(Source::from_bytes(
+            r"
+            let value = {};
+            value
+        ",
+        ))
+        .unwrap();
+
+    assert!(
+        Deserialize::try_from_js(&value, &mut context).is_err(),
+        "foo"
+    );
+}