review: @johnyob fixes / comments

Author: johnyob

jstz_api/src/urlpattern.rs

@@ -15,8 +15,7 @@ use boa_engine::{
     object::{builtins::JsArray, Object},
     property::Attribute,
     value::TryFromJs,
-    Context, JsArgs, JsError, JsNativeError, JsObject, JsResult, JsString, JsValue,
-    NativeFunction,
+    Context, JsArgs, JsError, JsNativeError, JsObject, JsResult, JsValue, NativeFunction,
 };
 use boa_gc::{empty_trace, Finalize, GcRefMut, Trace};
 
@@ -74,13 +73,13 @@ impl JsNativeObjectToString for UrlPattern {
 
 impl UrlPattern {
     fn process_input(
-        string_or_init: quirks::StringOrInit,
+        input: UrlPatternInput,
         base_url: Option<String>,
     ) -> JsResult<(
         InnerUrlPatternMatchInput,
         (quirks::StringOrInit, Option<String>),
     )> {
-        match quirks::process_match_input(string_or_init, base_url.as_deref()) {
+        match quirks::process_match_input(input.0, base_url.as_deref()) {
             Err(e) => Err(JsError::from_native(
                 JsNativeError::typ().with_message(e.to_string()),
             )),
@@ -98,16 +97,15 @@ impl UrlPattern {
         base_url: Option<String>,
         _context: &mut Context<'_>,
     ) -> JsResult<Self> {
-        let UrlPatternInput(string_or_init) = input;
-        let urlpatterninit =
-            quirks::process_construct_pattern_input(string_or_init, base_url.as_deref())
+        let url_pattern_init =
+            quirks::process_construct_pattern_input(input.0, base_url.as_deref())
                 .map_err(|_| {
                     JsError::from_native(
                         JsNativeError::typ()
                             .with_message("Failed to build UrlPatternInit"),
                     )
                 })?;
-        let url_pattern = InnerUrlPattern::parse(urlpatterninit).map_err(|_| {
+        let url_pattern = InnerUrlPattern::parse(url_pattern_init).map_err(|_| {
             JsError::from_native(
                 JsNativeError::typ().with_message("Failed to parse UrlPatternInit"),
             )
@@ -153,8 +151,7 @@ impl UrlPattern {
         input: UrlPatternInput,
         base_url: Option<String>,
     ) -> JsResult<bool> {
-        let UrlPatternInput(string_or_init) = input;
-        let (url_pattern_match_input, _) = Self::process_input(string_or_init, base_url)?;
+        let (url_pattern_match_input, _) = Self::process_input(input, base_url)?;
 
         self.url_pattern
             .test(url_pattern_match_input)
@@ -166,9 +163,8 @@ impl UrlPattern {
         input: UrlPatternInput,
         base_url: Option<String>,
     ) -> JsResult<Option<UrlPatternResult>> {
-        let UrlPatternInput(string_or_init) = input;
         let (url_pattern_match_input, (string_or_init, base_url)) =
-            Self::process_input(string_or_init, base_url)?;
+            Self::process_input(input, base_url)?;
 
         let mut inputs: Vec<UrlPatternInput> = Vec::new();
         inputs.push(UrlPatternInput(string_or_init));
@@ -329,7 +325,7 @@ impl TryFromJs for UrlPatternInit {
             pathname: get_optional_property!(obj, "pathname", context),
             search: get_optional_property!(obj, "search", context),
             hash: get_optional_property!(obj, "hash", context),
-            base_url: get_optional_property!(obj, "base_url", context),
+            base_url: get_optional_property!(obj, "baseURL", context),
         };
 
         Ok(Self(url_pattern_init))
@@ -372,46 +368,58 @@ impl From<quirks::UrlPatternInit> for UrlPatternInput {
 
 impl IntoJs for UrlPatternComponentResult {
     fn into_js(self, context: &mut Context<'_>) -> JsValue {
-        let UrlPatternComponentResult(url_pattern_component_result) = self;
+        let url_pattern_component_result = self.0;
         let input = url_pattern_component_result.input;
         let groups: Vec<(String, String)> =
             url_pattern_component_result.groups.into_iter().collect();
-        // Create an object with prototype set to `Object`
+        // Create an object with prototype set to `Object.prototype`
         let obj = JsObject::with_object_proto(context.intrinsics());
         // Add data property `input` to the object
+        // TODO: Support error handling (using TryIntoJs)
         let _ = obj.create_data_property(
-            JsString::from("input"),
+            js_string!("input"),
             input.into_js(context),
             context,
         );
         // Add data property `groups` to the object,
-        // which is itself another object with prototype set to `Object`
+        // which is itself another object with prototype set to `Object.prototype`
         let group_obj = JsObject::with_object_proto(context.intrinsics());
         for (key, value) in groups.iter() {
             let value = value.clone().into_js(context);
-            let _ = group_obj.create_data_property(
-                JsString::from(key.clone()),
-                value,
-                context,
-            );
+
+            // TODO: Support error handling (using TryIntoJs)
+            let _ =
+                group_obj.create_data_property(js_string!(key.clone()), value, context);
         }
-        let _ = obj.create_data_property(JsString::from("groups"), group_obj, context);
+
+        // TODO: Support error handling (using TryIntoJs)
+        let _ = obj.create_data_property(js_string!("groups"), group_obj, context);
         obj.into()
     }
 }
 
 impl IntoJs for UrlPatternInit {
     fn into_js(self, context: &mut Context<'_>) -> JsValue {
         let obj = JsObject::with_object_proto(context.intrinsics());
-        let UrlPatternInit(init) = self;
+        let init = self.0;
 
         macro_rules! create_data_properties_if_some {
             ($obj:ident, $init:ident, $field:ident, $context:ident) => {
-                let property_name = stringify!($field);
                 if let Some(s) = $init.$field {
+                    // TODO: Support error handling (using TryIntoJs)
+                    let _ = $obj.create_data_property(
+                        js_string!(stringify!($field)),
+                        js_string!(s),
+                        $context,
+                    );
+                }
+            };
+            ($obj:ident, $init:ident, $field:ident, $property_name:literal, $context:ident) => {
+                if let Some(s) = $init.$field {
+                    // TODO: Support error handling (using TryIntoJs)
                     let _ = $obj.create_data_property(
-                        JsString::from(property_name),
-                        JsString::from(s),
+                        js_string!($property_name),
+                        js_string!(s),
                         $context,
                     );
                 }
@@ -426,6 +434,7 @@ impl IntoJs for UrlPatternInit {
         create_data_properties_if_some!(obj, init, pathname, context);
         create_data_properties_if_some!(obj, init, search, context);
         create_data_properties_if_some!(obj, init, hash, context);
+        create_data_properties_if_some!(obj, init, base_url, "baseURL", context);
 
         obj.into()
     }
@@ -438,10 +447,9 @@ impl IntoJs for UrlPatternResult {
 
         macro_rules! create_data_property {
             ($obj:ident, $inner:ident, $field:ident, $context:ident) => {
-                let property_name = stringify!($field);
                 let $field = UrlPatternComponentResult($inner.$field).into_js($context);
                 let _ = $obj.create_data_property(
-                    JsString::from(property_name),
+                    js_string!(stringify!($field)),
                     $field,
                     $context,
                 );
@@ -464,7 +472,7 @@ impl IntoJs for UrlPatternResult {
             }
             array.into()
         };
-        let _ = obj.create_data_property(JsString::from("inputs"), inputs, context);
+        let _ = obj.create_data_property(js_string!("inputs"), inputs, context);
 
         obj.into()
     }