diff --git a/packages/core/src/effect.rs b/packages/core/src/effect.rs new file mode 100644 index 000000000..96d4c116d --- /dev/null +++ b/packages/core/src/effect.rs @@ -0,0 +1,66 @@ +use crate::innerlude::ScopeOrder; +use crate::Runtime; +use std::borrow::Borrow; +use std::cell::RefCell; +use std::collections::VecDeque; + +/// Effects will always run after all changes to the DOM have been applied. +/// +/// Effects are the lowest priority task in the scheduler. +/// They are run after all other dirty scopes and futures have been resolved. Other dirty scopes and futures may cause the component this effect is attached to to rerun, which would update the DOM. +pub(crate) struct Effect { + // The scope that the effect is attached to + pub(crate) order: ScopeOrder, + // The callbacks that will be run when effects are rerun + effect: RefCell>>, +} + +impl Effect { + pub(crate) fn new(order: ScopeOrder, f: impl FnOnce() + 'static) -> Self { + let mut effect = VecDeque::new(); + effect.push_back(Box::new(f) as Box); + Self { + order, + effect: RefCell::new(effect), + } + } + + pub(crate) fn push_back(&self, f: impl FnOnce() + 'static) { + self.effect.borrow_mut().push_back(Box::new(f)); + } + + pub(crate) fn run(&self, runtime: &Runtime) { + runtime.rendering.set(false); + let mut effect = self.effect.borrow_mut(); + while let Some(f) = effect.pop_front() { + f(); + } + runtime.rendering.set(true); + } +} + +impl PartialOrd for Effect { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.order.cmp(&other.order)) + } +} + +impl Ord for Effect { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.order.cmp(&other.order) + } +} + +impl PartialEq for Effect { + fn eq(&self, other: &Self) -> bool { + self.order == other.order + } +} + +impl Eq for Effect {} + +impl Borrow for Effect { + fn borrow(&self) -> &ScopeOrder { + &self.order + } +} diff --git a/packages/core/src/global_context.rs b/packages/core/src/global_context.rs index 7783deccd..88ab9214a 100644 --- a/packages/core/src/global_context.rs +++ b/packages/core/src/global_context.rs @@ -90,6 +90,11 @@ pub fn spawn(fut: impl Future + 'static) -> Task { Runtime::with_current_scope(|cx| cx.spawn(fut)).expect("to be in a dioxus runtime") } +/// Queue an effect to run after the next render +pub fn queue_effect(f: impl FnOnce() + 'static) { + Runtime::with_current_scope(|cx| cx.queue_effect(f)).expect("to be in a dioxus runtime") +} + /// Spawn a future that Dioxus won't clean up when this component is unmounted /// /// This is good for tasks that need to be run after the component has been dropped. diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index 9a0b9fec2..f5c0444a4 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -6,7 +6,7 @@ mod any_props; mod arena; mod diff; -mod dirty_scope; +mod effect; mod error_boundary; mod events; mod fragment; @@ -17,6 +17,7 @@ mod nodes; mod properties; mod render_signal; mod runtime; +mod scheduler; mod scope_arena; mod scope_context; mod scopes; @@ -26,7 +27,7 @@ mod virtual_dom; pub(crate) mod innerlude { pub(crate) use crate::any_props::*; pub use crate::arena::*; - pub use crate::dirty_scope::*; + pub(crate) use crate::effect::*; pub use crate::error_boundary::*; pub use crate::events::*; pub use crate::fragment::*; @@ -36,6 +37,7 @@ pub(crate) mod innerlude { pub use crate::nodes::*; pub use crate::properties::*; pub use crate::runtime::{Runtime, RuntimeGuard}; + pub use crate::scheduler::*; pub use crate::scopes::*; pub use crate::tasks::*; pub use crate::virtual_dom::*; @@ -65,13 +67,13 @@ pub mod prelude { pub use crate::innerlude::{ consume_context, consume_context_from_scope, current_owner, current_scope_id, fc_to_builder, generation, has_context, needs_update, needs_update_any, parent_scope, - provide_context, provide_root_context, remove_future, schedule_update, schedule_update_any, - spawn, spawn_forever, spawn_isomorphic, suspend, try_consume_context, use_after_render, - use_before_render, use_drop, use_error_boundary, use_hook, use_hook_with_cleanup, - wait_for_next_render, with_owner, AnyValue, Attribute, Component, ComponentFunction, - Element, ErrorBoundary, Event, EventHandler, Fragment, HasAttributes, IntoAttributeValue, - IntoDynNode, OptionStringFromMarker, Properties, Runtime, RuntimeGuard, ScopeId, - ScopeState, SuperFrom, SuperInto, Task, Template, TemplateAttribute, TemplateNode, Throw, - VNode, VNodeInner, VirtualDom, + provide_context, provide_root_context, queue_effect, remove_future, schedule_update, + schedule_update_any, spawn, spawn_forever, spawn_isomorphic, suspend, try_consume_context, + use_after_render, use_before_render, use_drop, use_error_boundary, use_hook, + use_hook_with_cleanup, wait_for_next_render, with_owner, AnyValue, Attribute, Component, + ComponentFunction, Element, ErrorBoundary, Event, EventHandler, Fragment, HasAttributes, + IntoAttributeValue, IntoDynNode, OptionStringFromMarker, Properties, Runtime, RuntimeGuard, + ScopeId, ScopeState, SuperFrom, SuperInto, Task, Template, TemplateAttribute, TemplateNode, + Throw, VNode, VNodeInner, VirtualDom, }; } diff --git a/packages/core/src/render_signal.rs b/packages/core/src/render_signal.rs index bdf388869..36386960a 100644 --- a/packages/core/src/render_signal.rs +++ b/packages/core/src/render_signal.rs @@ -1,4 +1,4 @@ -//! In dioxus, effects are run using normal async functions after a render. [RenderSignalFuture] is a future that resolves after a render has passed. +//! TODO: We no longer run effects with async tasks. Effects are now their own type of task. We should remove this next breaking release. use std::cell::RefCell; use std::future::Future; diff --git a/packages/core/src/runtime.rs b/packages/core/src/runtime.rs index a77a4dd34..ee727f348 100644 --- a/packages/core/src/runtime.rs +++ b/packages/core/src/runtime.rs @@ -1,3 +1,4 @@ +use crate::innerlude::Effect; use crate::{ innerlude::{LocalTask, SchedulerMsg}, render_signal::RenderSignal, @@ -5,6 +6,7 @@ use crate::{ scopes::ScopeId, Task, }; +use std::collections::BTreeSet; use std::{ cell::{Cell, Ref, RefCell}, rc::Rc, @@ -36,6 +38,9 @@ pub struct Runtime { // Synchronous tasks need to be run after the next render. The virtual dom stores a list of those tasks to send a signal to them when the next render is done. pub(crate) render_signal: RenderSignal, + + // The effects that need to be run after the next render + pub(crate) pending_effects: RefCell>, } impl Runtime { @@ -49,6 +54,7 @@ impl Runtime { current_task: Default::default(), tasks: Default::default(), suspended_tasks: Default::default(), + pending_effects: Default::default(), }) } @@ -150,6 +156,19 @@ impl Runtime { pub(crate) fn with_scope(scope: ScopeId, f: impl FnOnce(&Scope) -> R) -> Option { Self::with(|rt| rt.get_state(scope).map(|sc| f(&sc))).flatten() } + + /// Finish a render. This will mark all effects as ready to run and send the render signal. + pub(crate) fn finish_render(&self) { + // If there are new effects we can run, send a message to the scheduler to run them (after the renderer has applied the mutations) + if !self.pending_effects.borrow().is_empty() { + self.sender + .unbounded_send(SchedulerMsg::EffectQueued) + .expect("Scheduler should exist"); + } + + // And send the render signal + self.render_signal.send(); + } } /// A guard for a new runtime. This must be used to override the current runtime when importing components from a dynamic library that has it's own runtime. diff --git a/packages/core/src/dirty_scope.rs b/packages/core/src/scheduler.rs similarity index 59% rename from packages/core/src/dirty_scope.rs rename to packages/core/src/scheduler.rs index 8f06096d4..a183b60b9 100644 --- a/packages/core/src/dirty_scope.rs +++ b/packages/core/src/scheduler.rs @@ -1,32 +1,81 @@ -//! Dioxus resolves scopes in a specific order to avoid unexpected behavior. All tasks are resolved in the order of height. Scopes that are higher up in the tree are resolved first. -//! When a scope that is higher up in the tree is rerendered, it may drop scopes lower in the tree along with their tasks. +//! # Dioxus uses a scheduler to run queued work in the correct order. //! -//! ```rust -//! use dioxus::prelude::*; +//! ## Goals +//! We try to prevent three different situations: +//! 1. Running queued work after it could be dropped. Related issues (https://github.com/DioxusLabs/dioxus/pull/1993) +//! +//! User code often assumes that this property is true. For example, if this code reruns the child component after signal is changed to None, it will panic +//! ```rust, ignore +//! fn ParentComponent() -> Element { +//! let signal: Signal> = use_signal(None); //! -//! fn app() -> Element { -//! let vec = use_signal(|| vec![0; 10]); //! rsx! { -//! // If the length of the vec shrinks we need to make sure that the children are dropped along with their tasks the new state of the vec is read -//! for idx in 0..vec.len() { -//! Child { idx, vec } +//! if signal.read().is_some() { +//! ChildComponent { signal } //! } //! } //! } //! //! #[component] -//! fn Child(vec: Signal>, idx: usize) -> Element { -//! use_hook(move || { -//! spawn(async move { -//! // If we let this task run after the child is dropped, it will panic. -//! println!("Task {}", vec.read()[idx]); -//! }); -//! }); -//! -//! rsx! {} +//! fn ChildComponent(signal: Signal>) -> Element { +//! // It feels safe to assume that signal is some because the parent component checked that it was some +//! rsx! { "{signal.read().unwrap()}" } //! } //! ``` +//! +//! 2. Running effects before the dom is updated. Related issues (https://github.com/DioxusLabs/dioxus/issues/2307) +//! +//! Effects can be used to run code that accesses the DOM directly. They should only run when the DOM is in an updated state. If they are run with an out of date version of the DOM, unexpected behavior can occur. +//! ```rust, ignore +//! fn EffectComponent() -> Element { +//! let id = use_signal(0); +//! use_effect(move || { +//! let id = id.read(); +//! // This will panic if the id is not written to the DOM before the effect is run +//! eval(format!(r#"document.getElementById("{id}").innerHTML = "Hello World";"#)); +//! }); +//! +//! rsx! { +//! div { id: "{id}" } +//! } +//! } +//! +//! 3. Observing out of date state. Related issues (https://github.com/DioxusLabs/dioxus/issues/1935) +//! +//! Where ever possible, updates should happen in an order that makes it impossible to observe an out of date state. +//! ```rust, ignore +//! fn OutOfDateComponent() -> Element { +//! let id = use_signal(0); +//! // When you read memo, it should **always** be two times the value of id +//! let memo = use_memo(move || id() * 2); +//! assert_eq!(memo(), id() * 2); +//! +//! // This should be true even if you update the value of id in the middle of the component +//! id += 1; +//! assert_eq!(memo(), id() * 2); +//! +//! rsx! { +//! div { id: "{id}" } +//! } +//! } +//! ``` +//! +//! ## Implementation +//! +//! There are three different types of queued work that can be run by the virtualdom: +//! 1. Dirty Scopes: +//! Description: When a scope is marked dirty, a rerun of the scope will be scheduled. This will cause the scope to rerun and update the DOM if any changes are detected during the diffing phase. +//! Priority: These are the highest priority tasks. Dirty scopes will be rerun in order from the scope closest to the root to the scope furthest from the root. We follow this order to ensure that if a higher component reruns and drops a lower component, the lower component will not be run after it should be dropped. +//! +//! 2. Tasks: +//! Description: Futures spawned in the dioxus runtime each have an unique task id. When the waker for that future is called, the task is rerun. +//! Priority: These are the second highest priority tasks. They are run after all other dirty scopes have been resolved because those dirty scopes may cause children (and the tasks those children own) to drop which should cancel the futures. +//! +//! 3. Effects: +//! Description: Effects should always run after all changes to the DOM have been applied. +//! Priority: These are the lowest priority tasks in the scheduler. They are run after all other dirty scopes and futures have been resolved. Other tasks may cause components to rerun, which would update the DOM. These effects should only run after the DOM has been updated. +use crate::innerlude::Effect; use crate::ScopeId; use crate::Task; use crate::VirtualDom; @@ -106,6 +155,19 @@ impl VirtualDom { Some(task) } + /// Take any effects from the highest scope. This should only be called if there is no pending scope reruns or tasks + pub(crate) fn pop_effect(&mut self) -> Option { + let mut pending_effects = self.runtime.pending_effects.borrow_mut(); + let mut effect = pending_effects.pop_first()?; + + // If the scope doesn't exist for whatever reason, then we should skip it + while !self.scopes.contains(effect.order.id.0) { + effect = pending_effects.pop_first()?; + } + + Some(effect) + } + /// Take any work from the highest scope. This may include rerunning the scope and/or running tasks pub(crate) fn pop_work(&mut self) -> Option { let mut dirty_scope = self.dirty_scopes.first(); diff --git a/packages/core/src/scope_context.rs b/packages/core/src/scope_context.rs index c90cdc6c0..c68d4a320 100644 --- a/packages/core/src/scope_context.rs +++ b/packages/core/src/scope_context.rs @@ -265,6 +265,11 @@ impl Scope { id } + /// Queue an effect to run after the next render + pub fn queue_effect(&self, f: impl FnOnce() + 'static) { + Runtime::with(|rt| rt.queue_effect(self.id, f)).expect("Runtime to exist"); + } + /// Mark this component as suspended on a specific task and then return None pub fn suspend(&self, task: Task) -> Option { self.last_suspendable_task.set(Some(task)); diff --git a/packages/core/src/tasks.rs b/packages/core/src/tasks.rs index 136d2fb9a..5e1ea1f4f 100644 --- a/packages/core/src/tasks.rs +++ b/packages/core/src/tasks.rs @@ -1,3 +1,5 @@ +use crate::innerlude::Effect; +use crate::innerlude::ScopeOrder; use crate::innerlude::{remove_future, spawn, Runtime}; use crate::ScopeId; use futures_util::task::ArcWake; @@ -168,6 +170,19 @@ impl Runtime { task_id } + /// Queue an effect to run after the next render + pub(crate) fn queue_effect(&self, id: ScopeId, f: impl FnOnce() + 'static) { + // Add the effect to the queue of effects to run after the next render for the given scope + let mut effects = self.pending_effects.borrow_mut(); + let scope_order = ScopeOrder::new(id.height(), id); + match effects.get(&scope_order) { + Some(effects) => effects.push_back(Box::new(f)), + None => { + effects.insert(Effect::new(scope_order, f)); + } + } + } + /// Get the currently running task pub fn current_task(&self) -> Option { self.current_task.get() @@ -282,13 +297,15 @@ impl TaskType { /// The type of message that can be sent to the scheduler. /// /// These messages control how the scheduler will process updates to the UI. -#[derive(Debug)] pub(crate) enum SchedulerMsg { /// Immediate updates from Components that mark them as dirty Immediate(ScopeId), /// A task has woken and needs to be progressed TaskNotified(Task), + + /// An effect has been queued to run after the next render + EffectQueued, } struct LocalTaskHandle { diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index d88df76c6..65402dbce 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -481,6 +481,7 @@ impl VirtualDom { // The task may be marked dirty at the same time as the scope that owns the task is dropped. self.mark_task_dirty(id); } + SchedulerMsg::EffectQueued => {} }; } @@ -491,6 +492,7 @@ impl VirtualDom { match msg { SchedulerMsg::Immediate(id) => self.mark_dirty(id), SchedulerMsg::TaskNotified(task) => self.mark_task_dirty(task), + SchedulerMsg::EffectQueued => {} } } } @@ -513,21 +515,36 @@ impl VirtualDom { fn poll_tasks(&mut self) { // Make sure we set the runtime since we're running user code let _runtime = RuntimeGuard::new(self.runtime.clone()); - // Next, run any queued tasks - // We choose not to poll the deadline since we complete pretty quickly anyways - while let Some(task) = self.pop_task() { - // Then poll any tasks that might be pending - let mut tasks = task.tasks_queued.into_inner(); - while let Some(task) = tasks.pop_front() { - let _ = self.runtime.handle_task_wakeup(task); - // Running that task, may mark a scope higher up as dirty. If it does, return from the function early + // Keep polling tasks until there are no more effects or tasks to run + // Or until we have no more dirty scopes + while !self.dirty_tasks.is_empty() || !self.runtime.pending_effects.borrow().is_empty() { + // Next, run any queued tasks + // We choose not to poll the deadline since we complete pretty quickly anyways + while let Some(task) = self.pop_task() { + // Then poll any tasks that might be pending + let mut tasks = task.tasks_queued.into_inner(); + while let Some(task) = tasks.pop_front() { + let _ = self.runtime.handle_task_wakeup(task); + + // Running that task, may mark a scope higher up as dirty. If it does, return from the function early + self.queue_events(); + if self.has_dirty_scopes() { + // requeue any remaining tasks + for task in tasks { + self.mark_task_dirty(task); + } + return; + } + } + } + + // At this point, we have finished running all tasks that are pending and we haven't found any scopes to rerun. This means it is safe to run our lowest priority work: effects + while let Some(effect) = self.pop_effect() { + effect.run(&self.runtime); + // Check if any new scopes are queued for rerun self.queue_events(); if self.has_dirty_scopes() { - // requeue any remaining tasks - for task in tasks { - self.mark_task_dirty(task); - } return; } } @@ -633,7 +650,7 @@ impl VirtualDom { // Process any events that might be pending in the queue // Signals marked with .write() need a chance to be handled by the effect driver - // This also processes futures which might progress into immediates + // This also processes futures which might progress into immediately rerunning a scope self.process_events(); // Next, diff any dirty scopes @@ -658,7 +675,7 @@ impl VirtualDom { } } - self.runtime.render_signal.send(); + self.runtime.finish_render(); } /// [`Self::render_immediate`] to a vector of mutations for testing purposes diff --git a/packages/hooks/src/use_effect.rs b/packages/hooks/src/use_effect.rs index c16a79ee6..da22ea981 100644 --- a/packages/hooks/src/use_effect.rs +++ b/packages/hooks/src/use_effect.rs @@ -41,25 +41,29 @@ use crate::use_callback; /// ``` #[track_caller] pub fn use_effect(callback: impl FnMut() + 'static) -> Effect { - // let mut run_effect = use_hook(|| CopyValue::new(true)); - // use_hook_did_run(move |did_run| run_effect.set(did_run)); - let callback = use_callback(callback); let location = std::panic::Location::caller(); use_hook(|| { + // Inside the effect, we track any reads so that we can rerun the effect if a value the effect reads changes let (rc, mut changed) = ReactiveContext::new_with_origin(location); + // Spawn a task that will run the effect when: + // 1) The component is first run + // 2) The effect is rerun due to an async read at any time + // 3) The effect is rerun in the same tick that the component is rerun: we need to wait for the component to rerun before we can run the effect again + let queue_effect_for_next_render = move || { + queue_effect(move || rc.run_in(&*callback)); + }; + + queue_effect_for_next_render(); spawn(async move { loop { - // Run the effect - rc.run_in(&*callback); - // Wait for context to change let _ = changed.next().await; - // Wait for the dom the be finished with sync work - wait_for_next_render().await; + // Run the effect + queue_effect_for_next_render(); } }); Effect { rc } diff --git a/packages/hooks/tests/effect.rs b/packages/hooks/tests/effect.rs index 13fbeca17..660904166 100644 --- a/packages/hooks/tests/effect.rs +++ b/packages/hooks/tests/effect.rs @@ -2,6 +2,7 @@ use std::cell::RefCell; use std::collections::HashMap; use std::rc::Rc; +use std::time::Duration; use dioxus::prelude::*; use dioxus_core::ElementId; @@ -42,9 +43,63 @@ async fn effects_rerun() { ); dom.rebuild_in_place(); - dom.wait_for_work().await; + tokio::select! { + _ = dom.wait_for_work() => {} + _ = tokio::time::sleep(Duration::from_millis(500)) => panic!("timed out") + }; let current_counter = counter.borrow(); assert_eq!(current_counter.component, 1); assert_eq!(current_counter.effect, 1); } + +// https://github.com/DioxusLabs/dioxus/issues/2347 +// Effects should rerun when the signal changes if there are no changes to the component +#[tokio::test] +async fn effects_rerun_without_rerender() { + #[derive(Default)] + struct RunCounter { + component: usize, + effect: usize, + } + + let counter = Rc::new(RefCell::new(RunCounter::default())); + let mut dom = VirtualDom::new_with_props( + |counter: Rc>| { + counter.borrow_mut().component += 1; + println!("component {}", counter.borrow().component); + + let mut signal = use_signal(|| 0); + use_effect({ + to_owned![counter]; + move || { + counter.borrow_mut().effect += 1; + // This will subscribe the effect to the signal + println!("Signal: {}", signal); + } + }); + use_future(move || async move { + for i in 0..10 { + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + println!("future {}", i); + signal += 1; + } + }); + + rsx! { + div {} + } + }, + counter.clone(), + ); + + dom.rebuild_in_place(); + tokio::select! { + _ = dom.wait_for_work() => {} + _ = tokio::time::sleep(Duration::from_millis(500)) => {} + }; + + let current_counter = counter.borrow(); + assert_eq!(current_counter.component, 1); + assert_eq!(current_counter.effect, 11); +}