From 261aa4271fd3d03d87d1ac75bbc02bce7e3a390a Mon Sep 17 00:00:00 2001 From: Markus Kohlhase Date: Tue, 26 Nov 2024 13:07:49 +0100 Subject: [PATCH] Refactor --- Cargo.lock | 1 + xilem_web/src/concurrent/memoized_await.rs | 134 ++++++++++++--------- xilem_web/web_examples/fetch/Cargo.toml | 1 + xilem_web/web_examples/fetch/src/main.rs | 16 ++- xilem_web/web_examples/streams/src/api.rs | 2 +- xilem_web/web_examples/streams/src/main.rs | 7 +- 6 files changed, 95 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7ab90f47d..e86384a8f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1046,6 +1046,7 @@ dependencies = [ "console_error_panic_hook", "console_log", "gloo-net", + "gloo-timers", "log", "serde", "wasm-bindgen", diff --git a/xilem_web/src/concurrent/memoized_await.rs b/xilem_web/src/concurrent/memoized_await.rs index 59b950c6d..dbb540fdc 100644 --- a/xilem_web/src/concurrent/memoized_await.rs +++ b/xilem_web/src/concurrent/memoized_await.rs @@ -1,7 +1,7 @@ // Copyright 2024 the Xilem Authors and the Druid Authors // SPDX-License-Identifier: Apache-2.0 -use std::{fmt, future::Future, marker::PhantomData}; +use std::{fmt, future::Future, marker::PhantomData, rc::Rc}; use futures::{Stream, StreamExt}; use wasm_bindgen::{closure::Closure, JsCast, UnwrapThrowExt}; @@ -9,7 +9,7 @@ use wasm_bindgen_futures::spawn_local; use crate::{ core::{MessageResult, Mut, NoElement, View, ViewId, ViewMarker, ViewPathTracker}, - DynMessage, OptionalAction, ViewCtx, + DynMessage, MessageThunk, OptionalAction, ViewCtx, }; /// Await a future returned by `init_future` invoked with the argument `data`, `callback` is called with the output of the future. @@ -38,7 +38,7 @@ impl where FOut: fmt::Debug + 'static, F: Future + 'static, - InitFuture: Fn(&Data) -> F, + InitFuture: Fn(&mut State, &Data) -> F, { /// Debounce the `init_future` function, when `data` updates, /// when `reset_debounce_on_update == false` then this throttles updates each `milliseconds` @@ -64,7 +64,7 @@ impl where StreamItem: fmt::Debug + 'static, F: Stream + 'static, - InitStream: Fn(&Data) -> F, + InitStream: Fn(&mut State, &Data) -> F, { /// Debounce the `init_stream` function, when `data` updates, /// when `reset_debounce_on_update == false` then this throttles updates each `milliseconds` @@ -87,39 +87,33 @@ where fn init_future( m: &MemoizedFuture, - ctx: &mut ViewCtx, - generation: u64, + thunk: Rc, + state: &mut State, ) where - InitFuture: Fn(&Data) -> F + 'static, + InitFuture: Fn(&mut State, &Data) -> F + 'static, FOut: fmt::Debug + 'static, F: Future + 'static, { - ctx.with_id(ViewId::new(generation), |ctx| { - let thunk = ctx.message_thunk(); - let future = (m.init_future)(&m.data); - spawn_local(async move { - thunk.push_message(MemoizedFutureMessage::::Output(future.await)); - }); + let future = (m.init_future)(state, &m.data); + spawn_local(async move { + thunk.push_message(MemoizedFutureMessage::::Output(future.await)); }); } fn init_stream( m: &MemoizedFuture, - ctx: &mut ViewCtx, - generation: u64, + thunk: Rc, + state: &mut State, ) where - InitStream: Fn(&Data) -> F + 'static, + InitStream: Fn(&mut State, &Data) -> F + 'static, StreamItem: fmt::Debug + 'static, F: Stream + 'static, { - ctx.with_id(ViewId::new(generation), |ctx| { - let thunk = ctx.message_thunk(); - let mut stream = Box::pin((m.init_future)(&m.data)); - spawn_local(async move { - while let Some(item) = stream.next().await { - thunk.push_message(MemoizedFutureMessage::::Output(item)); - } - }); + let mut stream = Box::pin((m.init_future)(state, &m.data)); + spawn_local(async move { + while let Some(item) = stream.next().await { + thunk.push_message(MemoizedFutureMessage::::Output(item)); + } }); } @@ -137,7 +131,7 @@ fn init_stream( /// div(*state), /// memoized_await( /// 10, -/// |count| std::future::ready(*count), +/// |_, count| std::future::ready(*count), /// |state, output| *state = output, /// ) /// ) @@ -154,7 +148,7 @@ where Data: PartialEq + 'static, FOut: fmt::Debug + 'static, F: Future + 'static, - InitFuture: Fn(&Data) -> F + 'static, + InitFuture: Fn(&mut State, &Data) -> F + 'static, OA: OptionalAction + 'static, Callback: Fn(&mut State, FOut) -> OA + 'static, { @@ -185,7 +179,7 @@ where /// html::div(format!("{state:?}")), /// memoized_stream( /// 10, -/// |n| { +/// |_,n| { /// let range = 0..*n; /// stream! { /// for i in range { @@ -212,7 +206,7 @@ where Data: PartialEq + 'static, StreamItem: fmt::Debug + 'static, F: Stream + 'static, - InitStream: Fn(&Data) -> F + 'static, + InitStream: Fn(&mut State, &Data) -> F + 'static, OA: OptionalAction + 'static, Callback: Fn(&mut State, StreamItem) -> OA + 'static, { @@ -226,7 +220,6 @@ where }) } -#[derive(Default)] #[allow(unnameable_types)] // reason: Implementation detail, public because of trait visibility rules pub struct MemoizedAwaitState { generation: u64, @@ -235,9 +228,20 @@ pub struct MemoizedAwaitState { schedule_update_fn: Option>, schedule_update_timeout_handle: Option, update: bool, + thunk: Rc, } impl MemoizedAwaitState { + fn new(thunk: MessageThunk) -> Self { + Self { + generation: 0, + schedule_update: false, + schedule_update_fn: None, + schedule_update_timeout_handle: None, + update: false, + thunk: Rc::new(thunk), + } + } fn clear_update_timeout(&mut self) { if let Some(handle) = self.schedule_update_timeout_handle { web_sys::window() @@ -248,11 +252,13 @@ impl MemoizedAwaitState { self.schedule_update_fn = None; } - fn reset_debounce_timeout_and_schedule_update( + fn reset_debounce_timeout_and_schedule_update( &mut self, ctx: &mut ViewCtx, debounce_duration: usize, - ) { + ) where + FOut: fmt::Debug + 'static, + { ctx.with_id(ViewId::new(self.generation), |ctx| { self.clear_update_timeout(); let thunk = ctx.message_thunk(); @@ -271,12 +277,24 @@ impl MemoizedAwaitState { self.schedule_update = true; }); } + + fn request_init(&mut self, ctx: &mut ViewCtx) + where + FOut: fmt::Debug + 'static, + { + ctx.with_id(ViewId::new(self.generation), |ctx| { + self.thunk = Rc::new(ctx.message_thunk()); + self.thunk + .enqueue_message(MemoizedFutureMessage::::RequestInit); + }); + } } #[derive(Debug)] enum MemoizedFutureMessage { Output(Output), ScheduleUpdate, + RequestInit, } impl ViewMarker @@ -294,7 +312,7 @@ where State: 'static, Action: 'static, OA: OptionalAction + 'static, - InitFuture: Fn(&Data) -> F + 'static, + InitFuture: Fn(&mut State, &Data) -> F + 'static, FOut: fmt::Debug + 'static, Data: PartialEq + 'static, F: Future + 'static, @@ -305,7 +323,7 @@ where type ViewState = MemoizedAwaitState; fn build(&self, ctx: &mut ViewCtx) -> (Self::Element, Self::ViewState) { - self.0.build(ctx, init_future) + self.0.build(ctx) } fn rebuild( @@ -315,7 +333,7 @@ where ctx: &mut ViewCtx, (): Mut, ) { - self.0.rebuild(&prev.0, view_state, ctx, init_future); + self.0.rebuild(&prev.0, view_state, ctx); } fn teardown(&self, state: &mut Self::ViewState, _: &mut ViewCtx, (): Mut) { @@ -329,7 +347,8 @@ where message: DynMessage, app_state: &mut State, ) -> MessageResult { - self.0.message(view_state, id_path, message, app_state) + self.0 + .message(view_state, id_path, message, app_state, init_future) } } @@ -340,7 +359,7 @@ where State: 'static, Action: 'static, OA: OptionalAction + 'static, - InitStream: Fn(&Data) -> F + 'static, + InitStream: Fn(&mut State, &Data) -> F + 'static, StreamItem: fmt::Debug + 'static, Data: PartialEq + 'static, F: Stream + 'static, @@ -351,7 +370,7 @@ where type ViewState = MemoizedAwaitState; fn build(&self, ctx: &mut ViewCtx) -> (Self::Element, Self::ViewState) { - self.0.build(ctx, init_stream) + self.0.build(ctx) } fn rebuild( @@ -361,7 +380,7 @@ where ctx: &mut ViewCtx, (): Mut, ) { - self.0.rebuild(&prev.0, view_state, ctx, init_stream); + self.0.rebuild(&prev.0, view_state, ctx); } fn teardown(&self, state: &mut Self::ViewState, _: &mut ViewCtx, (): Mut) { @@ -375,7 +394,8 @@ where message: DynMessage, app_state: &mut State, ) -> MessageResult { - self.0.message(view_state, id_path, message, app_state) + self.0 + .message(view_state, id_path, message, app_state, init_stream) } } @@ -385,36 +405,26 @@ where State: 'static, Action: 'static, OA: OptionalAction + 'static, - InitFuture: Fn(&Data) -> F + 'static, + InitFuture: Fn(&mut State, &Data) -> F + 'static, FOut: fmt::Debug + 'static, Data: PartialEq + 'static, F: 'static, CB: Fn(&mut State, FOut) -> OA + 'static, { - fn build(&self, ctx: &mut ViewCtx, init_future: I) -> (NoElement, MemoizedAwaitState) - where - I: Fn(&Self, &mut ViewCtx, u64), - { - let mut state = MemoizedAwaitState::default(); + fn build(&self, ctx: &mut ViewCtx) -> (NoElement, MemoizedAwaitState) { + let thunk = ctx.message_thunk(); + let mut state = MemoizedAwaitState::new(thunk); if self.debounce_ms > 0 { state.reset_debounce_timeout_and_schedule_update::(ctx, self.debounce_ms); } else { - init_future(self, ctx, state.generation); + state.request_init::(ctx); } (NoElement, state) } - fn rebuild( - &self, - prev: &Self, - view_state: &mut MemoizedAwaitState, - ctx: &mut ViewCtx, - init_future: I, - ) where - I: Fn(&Self, &mut ViewCtx, u64), - { + fn rebuild(&self, prev: &Self, view_state: &mut MemoizedAwaitState, ctx: &mut ViewCtx) { let debounce_has_changed_and_update_is_scheduled = view_state.schedule_update && (prev.reset_debounce_on_update != self.reset_debounce_on_update || prev.debounce_ms != self.debounce_ms); @@ -444,7 +454,7 @@ where // no debounce view_state.generation += 1; view_state.update = false; - init_future(self, ctx, view_state.generation); + view_state.request_init::(ctx); } } } @@ -453,13 +463,17 @@ where state.clear_update_timeout(); } - fn message( + fn message( &self, view_state: &mut MemoizedAwaitState, id_path: &[ViewId], message: DynMessage, app_state: &mut State, - ) -> MessageResult { + init_future: I, + ) -> MessageResult + where + I: Fn(&Self, Rc, &mut State), + { assert_eq!(id_path.len(), 1); if id_path[0].routing_id() == view_state.generation { match *message.downcast().unwrap_throw() { @@ -474,6 +488,10 @@ where view_state.schedule_update = false; MessageResult::RequestRebuild } + MemoizedFutureMessage::RequestInit => { + init_future(self, Rc::clone(&view_state.thunk), app_state); + MessageResult::RequestRebuild + } } } else { MessageResult::Stale(message) diff --git a/xilem_web/web_examples/fetch/Cargo.toml b/xilem_web/web_examples/fetch/Cargo.toml index a56c22d8c..2e3a3fdf6 100644 --- a/xilem_web/web_examples/fetch/Cargo.toml +++ b/xilem_web/web_examples/fetch/Cargo.toml @@ -13,6 +13,7 @@ workspace = true console_error_panic_hook = "0.1" console_log = "1" gloo-net = { version = "0.6.0", default-features = false, features = ["http", "json", "serde"] } +gloo-timers = { version = "0.3.0", features = ["futures"] } log = "0.4" serde = { version = "1", features = ["derive"] } web-sys = { version = "0.3.69", features = ["Event", "HtmlInputElement"] } diff --git a/xilem_web/web_examples/fetch/src/main.rs b/xilem_web/web_examples/fetch/src/main.rs index d5153702f..aead47c44 100644 --- a/xilem_web/web_examples/fetch/src/main.rs +++ b/xilem_web/web_examples/fetch/src/main.rs @@ -8,6 +8,7 @@ #![allow(clippy::wildcard_imports, reason = "HTML elements are an exception")] use gloo_net::http::Request; +use gloo_timers::future::TimeoutFuture; use serde::{Deserialize, Serialize}; use wasm_bindgen::{JsCast, UnwrapThrowExt}; use xilem_web::{ @@ -56,13 +57,13 @@ impl AppState { FetchState::Finished } else if self.cats_to_fetch >= TOO_MANY_CATS { FetchState::TooMany + } else if self.cats_to_fetch > 0 && self.cats_are_being_fetched { + FetchState::Fetching } else if self.debounce_in_ms > 0 && self.cats_to_fetch > 0 && self.reset_debounce_on_update { FetchState::Debounced } else if self.debounce_in_ms > 0 && self.cats_to_fetch > 0 { FetchState::Throttled - } else if self.cats_to_fetch > 0 && self.cats_are_being_fetched { - FetchState::Fetching } else { FetchState::Initial } @@ -80,6 +81,10 @@ enum FetchState { async fn fetch_cats(count: usize) -> Result, gloo_net::Error> { log::debug!("Fetch {count} cats"); + + // Simulate a delay + TimeoutFuture::new(1_000).await; + if count == 0 { return Ok(Vec::new()); } @@ -119,7 +124,11 @@ fn app_logic(state: &mut AppState) -> impl HtmlDivElement { // or debounced otherwise: // As long as updates are happening within `debounce_in_ms` ms the first closure is not invoked, and a debounce timeout which runs `debounce_in_ms` is reset. state.cats_to_fetch, - |count| fetch_cats(*count), + |state: &mut AppState, count| { + log::debug!("Create new future to fetch"); + state.cats_are_being_fetched = true; + fetch_cats(*count) + }, |state: &mut AppState, cats_result| match cats_result { Ok(cats) => { log::info!("Received {} cats", cats.len()); @@ -182,7 +191,6 @@ fn cat_fetch_controls(state: &AppState) -> impl Element { if !state.cats_are_being_fetched { state.cats.clear(); } - state.cats_are_being_fetched = true; state.cats_to_fetch = input_target(&ev).value().parse().unwrap_or(0); })), )), diff --git a/xilem_web/web_examples/streams/src/api.rs b/xilem_web/web_examples/streams/src/api.rs index 22f3c4ccf..0589d5211 100644 --- a/xilem_web/web_examples/streams/src/api.rs +++ b/xilem_web/web_examples/streams/src/api.rs @@ -18,7 +18,7 @@ pub(crate) enum StreamMessage { Finished, } -#[derive(Debug, Default, PartialEq, Clone)] +#[derive(Default)] pub(crate) struct MockConnection; impl MockConnection { diff --git a/xilem_web/web_examples/streams/src/main.rs b/xilem_web/web_examples/streams/src/main.rs index e0fdd2fc7..b62160311 100644 --- a/xilem_web/web_examples/streams/src/main.rs +++ b/xilem_web/web_examples/streams/src/main.rs @@ -27,7 +27,7 @@ fn app_logic(state: &mut AppState) -> impl Element { log::debug!("Run app logic"); let search_stream = memoized_stream( - (state.search_term.clone(), state.db.clone()), + state.search_term.clone(), create_search_stream, handle_stream_message, ) @@ -65,12 +65,13 @@ fn on_search_input_keyup(state: &mut AppState, ev: web_sys::KeyboardEvent) { } fn create_search_stream( - (term, conn): &(String, MockConnection), + state: &mut AppState, + term: &String, ) -> Pin>> { if term.is_empty() { Box::pin(stream::empty()) } else { - Box::pin(conn.search(term.clone())) + Box::pin(state.db.search(term.to_owned())) } }