From a21e7d4dd168129da06f535f9dc4b1de724617cb Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 28 Jan 2022 12:00:13 -0500 Subject: [PATCH 1/9] fix: use_route should subscribe to changes to the route --- packages/router/src/hooks/use_route.rs | 32 ++++++++++++++++++++------ packages/router/src/service.rs | 14 ++++++++++- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/packages/router/src/hooks/use_route.rs b/packages/router/src/hooks/use_route.rs index 97dd6c048..ab6fad268 100644 --- a/packages/router/src/hooks/use_route.rs +++ b/packages/router/src/hooks/use_route.rs @@ -1,4 +1,4 @@ -use dioxus_core::ScopeState; +use dioxus_core::{ScopeId, ScopeState}; use gloo::history::{HistoryResult, Location}; use serde::de::DeserializeOwned; use std::{rc::Rc, str::FromStr}; @@ -74,10 +74,28 @@ impl UseRoute { /// This hook provides access to information about the current location in the /// context of a [`Router`]. If this function is called outside of a `Router` /// component it will panic. -pub fn use_route(cx: &ScopeState) -> UseRoute { - let router = cx - .consume_context::() - .expect("Cannot call use_route outside the scope of a Router component") - .clone(); - UseRoute { router } +pub fn use_route(cx: &ScopeState) -> &UseRoute { + &cx.use_hook(|_| { + let router = cx + .consume_context::() + .expect("Cannot call use_route outside the scope of a Router component"); + + router.subscribe_onchange(cx.scope_id()); + + UseRouteInner { + router: UseRoute { router }, + scope: cx.scope_id(), + } + }) + .router +} + +struct UseRouteInner { + router: UseRoute, + scope: ScopeId, +} +impl Drop for UseRouteInner { + fn drop(&mut self) { + self.router.router.unsubscribe_onchange(self.scope) + } } diff --git a/packages/router/src/service.rs b/packages/router/src/service.rs index 9da2bef7e..f2682f1f0 100644 --- a/packages/router/src/service.rs +++ b/packages/router/src/service.rs @@ -1,7 +1,7 @@ use gloo::history::{BrowserHistory, History, HistoryListener, Location}; use std::{ cell::{Cell, Ref, RefCell}, - collections::HashMap, + collections::{HashMap, HashSet}, rc::Rc, }; @@ -12,6 +12,7 @@ pub struct RouterService { pub(crate) pending_events: Rc>>, history: Rc>, slots: Rc>>, + onchange_listeners: Rc>>, root_found: Rc>>, cur_path_params: Rc>>, listener: HistoryListener, @@ -73,6 +74,7 @@ impl RouterService { regen_route, slots, pending_events, + onchange_listeners: Rc::new(RefCell::new(HashSet::new())), cur_path_params: Rc::new(RefCell::new(HashMap::new())), } } @@ -143,6 +145,16 @@ impl RouterService { pub fn current_path_params(&self) -> Ref> { self.cur_path_params.borrow() } + + pub fn subscribe_onchange(&self, id: ScopeId) { + log::trace!("Subscribing onchange for scope id {:?}", id); + self.onchange_listeners.borrow_mut().insert(id); + } + + pub fn unsubscribe_onchange(&self, id: ScopeId) { + log::trace!("Subscribing onchange for scope id {:?}", id); + self.onchange_listeners.borrow_mut().remove(&id); + } } fn clean_route(route: String) -> String { From 5ee9d6c4348a2f51adac827f715fb138918d1dc6 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 28 Jan 2022 12:04:16 -0500 Subject: [PATCH 2/9] fix: attach router listener to subscriber list --- packages/router/src/service.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/router/src/service.rs b/packages/router/src/service.rs index f2682f1f0..2378c5aee 100644 --- a/packages/router/src/service.rs +++ b/packages/router/src/service.rs @@ -43,6 +43,7 @@ impl RouterService { let location = history.location(); let path = location.path(); + let onchange_listeners = Rc::new(RefCell::new(HashSet::new())); let slots: Rc>> = Default::default(); let pending_events: Rc>> = Default::default(); let root_found = Rc::new(Cell::new(None)); @@ -52,6 +53,7 @@ impl RouterService { let regen_route = regen_route.clone(); let root_found = root_found.clone(); let slots = slots.clone(); + let onchange_listeners = onchange_listeners.clone(); move || { root_found.set(None); // checking if the route is valid is cheap, so we do it @@ -60,6 +62,13 @@ impl RouterService { regen_route(*slot); } + for listener in onchange_listeners.borrow_mut().iter() { + log::trace!("regenerating listener {:?}", listener); + regen_route(*listener); + } + + + // also regenerate the root regen_route(root_scope); @@ -74,7 +83,7 @@ impl RouterService { regen_route, slots, pending_events, - onchange_listeners: Rc::new(RefCell::new(HashSet::new())), + onchange_listeners, cur_path_params: Rc::new(RefCell::new(HashMap::new())), } } From 9da46eb7bc207997ca7779c58fcb2a9645dfa9d0 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 28 Jan 2022 12:07:10 -0500 Subject: [PATCH 3/9] chore: rustfmt --- packages/router/src/service.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/router/src/service.rs b/packages/router/src/service.rs index 2378c5aee..b62bc78d5 100644 --- a/packages/router/src/service.rs +++ b/packages/router/src/service.rs @@ -67,8 +67,6 @@ impl RouterService { regen_route(*listener); } - - // also regenerate the root regen_route(root_scope); From 79e09934aa685d03d6e0b323723bc1cd537d74d9 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 28 Jan 2022 12:14:40 -0500 Subject: [PATCH 4/9] chore: add docs to router UseRouteListener --- packages/router/src/hooks/use_route.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/router/src/hooks/use_route.rs b/packages/router/src/hooks/use_route.rs index ab6fad268..3b609f5e2 100644 --- a/packages/router/src/hooks/use_route.rs +++ b/packages/router/src/hooks/use_route.rs @@ -8,6 +8,7 @@ use crate::RouterService; /// This struct provides is a wrapper around the internal router /// implementation, with methods for getting information about the current /// route. +#[derive(Clone)] pub struct UseRoute { router: Rc, } @@ -82,7 +83,7 @@ pub fn use_route(cx: &ScopeState) -> &UseRoute { router.subscribe_onchange(cx.scope_id()); - UseRouteInner { + UseRouteListener { router: UseRoute { router }, scope: cx.scope_id(), } @@ -90,11 +91,15 @@ pub fn use_route(cx: &ScopeState) -> &UseRoute { .router } -struct UseRouteInner { +// The entire purpose of this struct is to unubscribe this component when it is unmounted. +// The UseRoute can be cloned into async contexts, so we can't rely on its drop to unubscribe. +// Instead, we hide the drop implementation on this private type exclusive to the hook, +// and reveal our cached version of UseRoute to the component. +struct UseRouteListener { router: UseRoute, scope: ScopeId, } -impl Drop for UseRouteInner { +impl Drop for UseRouteListener { fn drop(&mut self) { self.router.router.unsubscribe_onchange(self.scope) } From c092bd43edf1891c427722bc9ca04e11c9359069 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 28 Jan 2022 16:12:06 -0500 Subject: [PATCH 5/9] fix: use_state --- packages/hooks/src/usestate.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/hooks/src/usestate.rs b/packages/hooks/src/usestate.rs index eb3900981..914587c13 100644 --- a/packages/hooks/src/usestate.rs +++ b/packages/hooks/src/usestate.rs @@ -61,6 +61,8 @@ pub fn use_state<'a, T: 'static>( } }); + hook.current_val = hook.slot.borrow().clone(); + (hook.current_val.as_ref(), hook) } From 72c6bb3d0b7253f084f7e3bcf55d458cb4adeedb Mon Sep 17 00:00:00 2001 From: Dave Rolsky Date: Fri, 28 Jan 2022 15:31:43 -0600 Subject: [PATCH 6/9] Make log message in Link component trace level, not debug All the other routing-related messages are at the trace level. Leaving this at debug was an oversight on my part. --- packages/router/src/components/link.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/router/src/components/link.rs b/packages/router/src/components/link.rs index 4c0ecb996..fd8eea66d 100644 --- a/packages/router/src/components/link.rs +++ b/packages/router/src/components/link.rs @@ -41,7 +41,7 @@ pub struct LinkProps<'a> { } pub fn Link<'a>(cx: Scope<'a, LinkProps<'a>>) -> Element { - log::debug!("render Link to {}", cx.props.to); + log::trace!("render Link to {}", cx.props.to); if let Some(service) = cx.consume_context::() { return cx.render(rsx! { a { From e9792e9b95521fcf0b0b4e240d7cf0397cd79b8f Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 28 Jan 2022 21:43:01 -0500 Subject: [PATCH 7/9] tests: add miri stress test --- packages/core/tests/miri_stress.rs | 39 ++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/packages/core/tests/miri_stress.rs b/packages/core/tests/miri_stress.rs index bb4963b71..6b7805261 100644 --- a/packages/core/tests/miri_stress.rs +++ b/packages/core/tests/miri_stress.rs @@ -313,3 +313,42 @@ fn leak_thru_children() { dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); dom.work_with_deadline(|| false); } + +#[test] +fn test_pass_thru() { + #[inline_props] + fn Router<'a>(cx: Scope, children: Element<'a>) -> Element { + cx.render(rsx! { + &cx.props.children + }) + } + + fn MemoizedThing(cx: Scope) -> Element { + rsx!(cx, div { "memoized" }) + } + + fn app(cx: Scope) -> Element { + let thing = cx.use_hook(|_| "asd"); + rsx!(cx, + Router { + MemoizedThing { + } + } + ) + } + + let mut dom = new_dom(app, ()); + let _ = dom.rebuild(); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); +} From 05d68b1107677ad49b4e9aba1ef094b64c8d6262 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 28 Jan 2022 21:45:40 -0500 Subject: [PATCH 8/9] ci: fewer windows targets this commit decreases the number of windows targets we check to speed up CI --- .github/workflows/windows.yml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 183fb282d..b776be433 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -19,13 +19,7 @@ jobs: max-parallel: 2 fail-fast: false matrix: - target: - [ - i686-pc-windows-gnu, - i686-pc-windows-msvc, - x86_64-pc-windows-gnu, - x86_64-pc-windows-msvc, - ] + target: [x86_64-pc-windows-gnu, x86_64-pc-windows-msvc] cfg_release_channel: [stable] steps: From 66bc09cf2837d9efe794f13d5ba6118b91b2d8d6 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 28 Jan 2022 21:46:15 -0500 Subject: [PATCH 9/9] ci: remove i686 windows target --- .github/workflows/windows.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index b776be433..2b6ea5c23 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -41,12 +41,6 @@ jobs: rustup target add ${{ matrix.target }} shell: powershell - - name: Add mingw32 to path for i686-gnu - run: | - echo "C:\msys64\mingw32\bin" >> $GITHUB_PATH - if: matrix.target == 'i686-pc-windows-gnu' && matrix.channel == 'nightly' - shell: bash - - name: Add mingw64 to path for x86_64-gnu run: echo "C:\msys64\mingw64\bin" >> $GITHUB_PATH if: matrix.target == 'x86_64-pc-windows-gnu' && matrix.channel == 'nightly'