From 1ac682310f0823c56c01437dd8f97b7d75fce401 Mon Sep 17 00:00:00 2001 From: Henry Zimmerman Date: Tue, 17 Dec 2019 14:30:30 -0500 Subject: [PATCH 1/3] Remove the option wrapper from route --- src/agent/mod.rs | 22 ++++++++---------- src/components/mod.rs | 2 +- src/route.rs | 24 ++++--------------- src/service.rs | 54 ++++++++++++++++++++++++++++++++++--------- src/switch.rs | 8 +++---- 5 files changed, 62 insertions(+), 48 deletions(-) diff --git a/src/agent/mod.rs b/src/agent/mod.rs index f1011e4..a132e5f 100644 --- a/src/agent/mod.rs +++ b/src/agent/mod.rs @@ -26,7 +26,7 @@ pub use dispatcher::RouteAgentDispatcher; #[derive(Debug)] pub enum Msg { /// Message for when the route is changed. - BrowserNavigationRouteChanged((String, T)), + BrowserNavigationRouteChanged(Route), // TODO make this a route? } /// Input message type for interacting with the `RouteAgent'. @@ -105,10 +105,8 @@ where fn update(&mut self, msg: Self::Message) { match msg { - Msg::BrowserNavigationRouteChanged((_route_string, state)) => { + Msg::BrowserNavigationRouteChanged(route) => { trace!("Browser navigated"); - let mut route = Route::current_route(&self.route_service); - route.state = Some(state); for sub in &self.subscribers { self.link.respond(*sub, route.clone()); } @@ -125,8 +123,8 @@ where RouteRequest::ReplaceRoute(route) => { let route_string: String = route.to_string(); self.route_service - .replace_route(&route_string, route.state.unwrap_or_default()); - let route = Route::current_route(&self.route_service); + .replace_route(&route_string, route.state); + let route = self.route_service.get_route(); for sub in &self.subscribers { self.link.respond(*sub, route.clone()); } @@ -134,15 +132,15 @@ where RouteRequest::ReplaceRouteNoBroadcast(route) => { let route_string: String = route.to_string(); self.route_service - .replace_route(&route_string, route.state.unwrap_or_default()); + .replace_route(&route_string, route.state); } RouteRequest::ChangeRoute(route) => { let route_string: String = route.to_string(); // set the route self.route_service - .set_route(&route_string, route.state.unwrap_or_default()); - // get the new route. This will contain a default state object - let route = Route::current_route(&self.route_service); + .set_route(&route_string, route.state); + // get the new route. + let route = self.route_service.get_route(); // broadcast it to all listening components for sub in &self.subscribers { self.link.respond(*sub, route.clone()); @@ -151,10 +149,10 @@ where RouteRequest::ChangeRouteNoBroadcast(route) => { let route_string: String = route.to_string(); self.route_service - .set_route(&route_string, route.state.unwrap_or_default()); + .set_route(&route_string, route.state); } RouteRequest::GetCurrentRoute => { - let route = Route::current_route(&self.route_service); + let route = self.route_service.get_route(); self.link.respond(who, route.clone()); } } diff --git a/src/components/mod.rs b/src/components/mod.rs index 518229a..fcce6e5 100644 --- a/src/components/mod.rs +++ b/src/components/mod.rs @@ -19,7 +19,7 @@ pub struct Props { /// The route that will be set when the component is clicked. pub link: String, /// The state to set when changing the route. - pub state: Option, + pub state: T, #[deprecated(note = "Use children field instead (nested html)")] /// The text to display. pub text: String, diff --git a/src/route.rs b/src/route.rs index 5859df2..57a594c 100644 --- a/src/route.rs +++ b/src/route.rs @@ -1,5 +1,4 @@ //! Wrapper around route url string, and associated history state. -use crate::service::RouteService; use serde::{Deserialize, Serialize}; use std::{fmt, ops::Deref}; use stdweb::{unstable::TryFrom, Value}; @@ -16,7 +15,7 @@ pub struct Route { /// The route string pub route: String, /// The state stored in the history api - pub state: Option, + pub state: T, } /// Formats a path, query, and fragment into a string. @@ -32,33 +31,18 @@ pub(crate) fn format_route_string(path: &str, query: &str, fragment: &str) -> St ) } -impl Route { - /// Gets the current route from the route service. - /// - /// # Note - /// It does not get the current state. - /// That is only provided via events. - /// See [RouteService.register_callback](struct.RouteService.html#method.register_callback) to - /// acquire state. - pub fn current_route(route_service: &RouteService) -> Self { - let route = route_service.get_route(); - // TODO, should try to get the state using the history api once that is exposed through - // stdweb. https://github.com/koute/stdweb/issues/371 - Route { route, state: None } - } -} - impl fmt::Display for Route { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { std::fmt::Display::fmt(&self.route, f) } } -impl From<&str> for Route { +// This is getting removed anyway +impl From<&str> for Route { fn from(string: &str) -> Route { Route { route: string.to_string(), - state: None, + state: T::default(), } } } diff --git a/src/service.rs b/src/service.rs index c6237f8..77f4751 100644 --- a/src/service.rs +++ b/src/service.rs @@ -1,14 +1,13 @@ //! Service that interfaces with the browser to handle routing. -use stdweb::{ - web::{event::PopStateEvent, window, EventListenerHandle, History, IEventTarget, Location}, - Value, -}; +use stdweb::{web::{event::PopStateEvent, window, EventListenerHandle, History, IEventTarget, Location}, Value}; use yew::callback::Callback; -use crate::route::RouteState; +use crate::route::{RouteState, Route}; use std::marker::PhantomData; use stdweb::unstable::TryFrom; +use stdweb::unstable::TryInto; +use stdweb::js; /// A service that facilitates manipulation of the browser's URL bar and responding to browser events /// when users press 'forward' or 'back'. @@ -53,10 +52,7 @@ impl RouteService { crate::route::format_route_string(&path, &query, &fragment) } - /// Gets the concatenated path, query, and fragment. - pub fn get_route(&self) -> String { - Self::get_route_from_location(&self.location) - } + /// Gets the path name of the current url. pub fn get_path(&self) -> String { @@ -81,7 +77,7 @@ where /// Registers a callback to the route service. /// Callbacks will be called when the History API experiences a change such as /// popping a state off of its stack when the forward or back buttons are pressed. - pub fn register_callback(&mut self, callback: Callback<(String, T)>) { + pub fn register_callback(&mut self, callback: Callback>) { self.event_listener = Some(window().add_event_listener(move |event: PopStateEvent| { let state_value: Value = event.state(); let state_string: String = String::try_from(state_value).unwrap_or_default(); @@ -96,7 +92,8 @@ where let location: Location = window().location().unwrap(); let route: String = Self::get_route_from_location(&location); - callback.emit((route.clone(), state)) + + callback.emit(Route{route, state}) })); } @@ -121,4 +118,39 @@ where }); let _ = self.history.replace_state(state_string, "", Some(route)); } + + /// Gets the concatenated path, query, and fragment. + pub fn get_route(&self) -> Route { + let route_string = Self::get_route_from_location(&self.location); + let state: T = get_state_string(&self.history) + .or_else(|| { + log::trace!("History state is empty"); + None + }) + .and_then(|state_string| -> Option>{ + serde_json::from_str(&state_string) + .ok() + .or_else(|| { + log::error!("Could not deserialize state string"); + None + }) + }) + .and_then(std::convert::identity) // flatten + .unwrap_or_default(); + Route { + route: route_string, + state + } + } +} + +fn get_state(history: &History) -> Value { + js!( + return @{history}.state; + ) } + +fn get_state_string(history: &History) -> Option { + get_state(history).try_into().ok() +} + diff --git a/src/switch.rs b/src/switch.rs index 4ba72ec..7e36863 100644 --- a/src/switch.rs +++ b/src/switch.rs @@ -164,7 +164,7 @@ macro_rules! impl_switch_for_from_to_str { fn from_route_part(part: Route) -> (Option, Option) { ( ::std::str::FromStr::from_str(&part.route).ok(), - part.state + Some(part.state) ) } @@ -208,19 +208,19 @@ impl_switch_for_from_to_str! { } /// Builds a route from a switch. -fn build_route_from_switch(switch: T) -> Route { +fn build_route_from_switch(switch: T) -> Route { // URLs are recommended to not be over 255 characters, // although browsers frequently support up to about 2000. // Routes, being a subset of URLs should probably be smaller than 255 characters for the vast // majority of circumstances, preventing reallocation under most conditions. let mut buf = String::with_capacity(255); - let state = switch.build_route_section(&mut buf); + let state = switch.build_route_section(&mut buf).unwrap_or_default(); buf.shrink_to_fit(); Route { route: buf, state } } -impl From for Route { +impl From for Route { fn from(switch: SW) -> Self { build_route_from_switch(switch) } From bb9c1648831a2763af3b7246c2d9548e63609777 Mon Sep 17 00:00:00 2001 From: Henry Zimmerman Date: Tue, 17 Dec 2019 15:35:26 -0500 Subject: [PATCH 2/3] fix macros and test --- .../yew_router_macro/src/switch/enum_impl.rs | 17 +++---- .../src/switch/struct_impl.rs | 17 +++---- examples/minimal/src/main.rs | 11 ++--- src/switch.rs | 49 +++++++++---------- 4 files changed, 38 insertions(+), 56 deletions(-) diff --git a/crates/yew_router_macro/src/switch/enum_impl.rs b/crates/yew_router_macro/src/switch/enum_impl.rs index e5e869e..a417b3f 100644 --- a/crates/yew_router_macro/src/switch/enum_impl.rs +++ b/crates/yew_router_macro/src/switch/enum_impl.rs @@ -33,9 +33,8 @@ pub fn generate_enum_impl( let token_stream = quote! { #impl_line { - fn from_route_part<__T: ::yew_router::route::RouteState>(route: ::yew_router::route::Route<__T>) -> (::std::option::Option, ::std::option::Option<__T>) { - let mut state = route.state; - let route_string = route.route; + fn from_route_part<__T: ::yew_router::route::RouteState>(route: String, mut state: Option<__T>) -> (::std::option::Option, ::std::option::Option<__T>) { + let route_string = route; #(#variant_matchers)* return (::std::option::Option::None, state) @@ -73,10 +72,8 @@ fn build_variant_from_captures( let (v, s) = match captures.remove(#key) { ::std::option::Option::Some(value) => { <#field_ty as ::yew_router::Switch>::from_route_part( - ::yew_router::route::Route { - route: value, - state, - } + value, + state, ) } ::std::option::Option::None => { @@ -129,10 +126,8 @@ fn build_variant_from_captures( let (v, s) = match drain.next() { ::std::option::Option::Some(value) => { <#field_ty as ::yew_router::Switch>::from_route_part( - ::yew_router::route::Route { - route: value, - state, - } + value, + state, ) }, ::std::option::Option::None => { diff --git a/crates/yew_router_macro/src/switch/struct_impl.rs b/crates/yew_router_macro/src/switch/struct_impl.rs index a1c553b..9dbcf28 100644 --- a/crates/yew_router_macro/src/switch/struct_impl.rs +++ b/crates/yew_router_macro/src/switch/struct_impl.rs @@ -24,11 +24,10 @@ pub fn generate_struct_impl(item: SwitchItem, generics: Generics) -> TokenStream let token_stream = quote! { #impl_line { - fn from_route_part<__T: ::yew_router::route::RouteState>(route: ::yew_router::route::Route<__T>) -> (::std::option::Option, ::std::option::Option<__T>) { + fn from_route_part<__T: ::yew_router::route::RouteState>(route: String, mut state: Option<__T>) -> (::std::option::Option, ::std::option::Option<__T>) { #matcher - let mut state = route.state; - let route_string = route.route; + let route_string = route; #build_from_captures @@ -62,10 +61,8 @@ fn build_struct_from_captures(ident: &Ident, fields: &Fields) -> TokenStream2 { let (v, s) = match captures.remove(#key) { ::std::option::Option::Some(value) => { <#field_ty as ::yew_router::Switch>::from_route_part( - ::yew_router::route::Route { - route: value, - state, - } + value, + state, ) } ::std::option::Option::None => { @@ -108,10 +105,8 @@ fn build_struct_from_captures(ident: &Ident, fields: &Fields) -> TokenStream2 { let (v, s) = match drain.next() { ::std::option::Option::Some(value) => { <#field_ty as ::yew_router::Switch>::from_route_part( - ::yew_router::route::Route { - route: value, - state, - } + value, + state, ) }, ::std::option::Option::None => { diff --git a/examples/minimal/src/main.rs b/examples/minimal/src/main.rs index 7b38050..19cea31 100644 --- a/examples/minimal/src/main.rs +++ b/examples/minimal/src/main.rs @@ -52,12 +52,7 @@ impl Component for Model { let mut route_service: RouteService<()> = RouteService::new(); let route = route_service.get_route(); let route = Route::from(route); - let callback = link.callback(|(route, state)| -> Msg { - Msg::RouteChanged(Route { - route, - state: Some(state), - }) - }); + let callback = link.callback(Msg::RouteChanged); route_service.register_callback(callback); Model { @@ -78,9 +73,9 @@ impl Component for Model { AppRoute::C => format!("/c"), }; self.route_service.set_route(&route_string, ()); - self.route = Route { + self.route = Route{ route: route_string, - state: None, + state: (), }; } } diff --git a/src/switch.rs b/src/switch.rs index 7e36863..8ea456f 100644 --- a/src/switch.rs +++ b/src/switch.rs @@ -50,11 +50,11 @@ pub type Routable = Switch; pub trait Switch: Sized { /// Based on a route, possibly produce an itself. fn switch(route: Route) -> Option { - Self::from_route_part(route).0 + Self::from_route_part(route.route, Some(route.state)).0 } /// Get self from a part of the state - fn from_route_part(part: Route) -> (Option, Option); + fn from_route_part(part: String, state: Option) -> (Option, Option); /// Build part of a route from itself. fn build_route_section(self, route: &mut String) -> Option; @@ -81,13 +81,10 @@ pub trait Switch: Sized { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct LeadingSlash(pub T); impl Switch for LeadingSlash { - fn from_route_part(part: Route) -> (Option, Option) { - if part.route.starts_with('/') { - let route = Route { - route: part.route[1..].to_string(), - state: part.state, - }; - let (inner, state) = U::from_route_part(route); + fn from_route_part(part: String, state: Option) -> (Option, Option) { + if part.starts_with('/') { + let part = part[1..].to_string(); + let (inner, state) = U::from_route_part(part, state); (inner.map(LeadingSlash), state) } else { (None, None) @@ -102,8 +99,8 @@ impl Switch for LeadingSlash { impl Switch for Option { /// Option is very permissive in what is allowed. - fn from_route_part(part: Route) -> (Option, Option) { - let (inner, inner_state) = U::from_route_part(part); + fn from_route_part(part: String, state: Option) -> (Option, Option) { + let (inner, inner_state) = U::from_route_part(part, state); if inner.is_some() { (Some(inner), inner_state) } else { @@ -130,9 +127,9 @@ impl Switch for Option { #[derive(Debug, PartialEq, Clone, Copy)] pub struct AllowMissing(pub Option); impl Switch for AllowMissing { - fn from_route_part(part: Route) -> (Option, Option) { - let route = part.route.clone(); - let (inner, inner_state) = U::from_route_part(part); + fn from_route_part(part: String, state: Option) -> (Option, Option) { + let route = part.clone(); + let (inner, inner_state) = U::from_route_part(part, state); if inner.is_some() { (Some(AllowMissing(inner)), inner_state) @@ -161,10 +158,10 @@ macro_rules! impl_switch_for_from_to_str { ($($SelfT: ty),*) => { $( impl Switch for $SelfT { - fn from_route_part(part: Route) -> (Option, Option) { + fn from_route_part(part: String, state: Option) -> (Option, Option) { ( - ::std::str::FromStr::from_str(&part.route).ok(), - Some(part.state) + ::std::str::FromStr::from_str(&part).ok(), + state ) } @@ -239,10 +236,10 @@ mod test { #[test] fn can_get_string_from_empty_str() { - let (s, _state) = String::from_route_part::<()>(Route { - route: "".to_string(), - state: None, - }); + let (s, _state) = String::from_route_part::<()>( + "".to_string(), + Some(()), + ); assert_eq!(s, Some("".to_string())) } @@ -250,7 +247,7 @@ mod test { fn uuid_from_route() { let x = uuid::Uuid::switch::<()>(Route { route: "5dc48134-35b5-4b8c-aa93-767bf00ae1d8".to_string(), - state: None, + state: (), }); assert!(x.is_some()) } @@ -266,10 +263,10 @@ mod test { #[test] fn can_get_option_string_from_empty_str() { - let (s, _state): (Option>, Option<()>) = Option::from_route_part(Route { - route: "".to_string(), - state: None, - }); + let (s, _state): (Option>, Option<()>) = Option::from_route_part( + "".to_string(), + Some(()), + ); assert_eq!(s, Some(Some("".to_string()))) } } From 0455ac856758cbd104ba83d9102fcbb86a63268c Mon Sep 17 00:00:00 2001 From: Henry Zimmerman Date: Tue, 17 Dec 2019 15:52:45 -0500 Subject: [PATCH 3/3] add todo --- src/components/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/mod.rs b/src/components/mod.rs index fcce6e5..a3cdf41 100644 --- a/src/components/mod.rs +++ b/src/components/mod.rs @@ -13,6 +13,7 @@ pub use self::{router_button::RouterButton, router_link::RouterAnchor, router_li use crate::RouterState; // TODO This should also be PartialEq and Clone. Its blocked on Children not supporting that. +// TODO This should no longer take link & String, and instead take a route: T implementing Switch /// Properties for `RouterButton` and `RouterLink`. #[derive(Properties, Default, Debug)] pub struct Props {