From 71f9f4449c5459c7b9fe55f8eb82e0679a8df5fd Mon Sep 17 00:00:00 2001 From: Alistair Date: Thu, 9 Nov 2023 15:32:50 +0000 Subject: [PATCH] review: @johnyob fixes / comments --- jstz_api/src/urlpattern.rs | 68 +++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/jstz_api/src/urlpattern.rs b/jstz_api/src/urlpattern.rs index 7c33ae302..3567decce 100644 --- a/jstz_api/src/urlpattern.rs +++ b/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, ) -> JsResult<( InnerUrlPatternMatchInput, (quirks::StringOrInit, Option), )> { - 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, _context: &mut Context<'_>, ) -> JsResult { - 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, ) -> JsResult { - 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, ) -> JsResult> { - 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 = 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,30 +368,32 @@ impl From 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() } } @@ -403,15 +401,25 @@ impl IntoJs for UrlPatternComponentResult { 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() }