diff --git a/Cargo.lock b/Cargo.lock index ac6e8d73ad..5b111d40b5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4051,6 +4051,8 @@ dependencies = [ "serde_json", "tracing", "tracing-wasm", + "wasm-bindgen", + "web-sys", ] [[package]] diff --git a/packages/core/src/effect.rs b/packages/core/src/effect.rs index af79b055fd..7c928ed71a 100644 --- a/packages/core/src/effect.rs +++ b/packages/core/src/effect.rs @@ -1,5 +1,4 @@ use crate::innerlude::ScopeOrder; -use crate::Runtime; use std::borrow::Borrow; use std::cell::RefCell; use std::collections::VecDeque; @@ -29,13 +28,11 @@ impl Effect { self.effect.borrow_mut().push_back(Box::new(f)); } - pub(crate) fn run(&self, runtime: &Runtime) { - runtime.rendering.set(false); + pub(crate) fn run(&self) { let mut effect = self.effect.borrow_mut(); while let Some(f) = effect.pop_front() { f(); } - runtime.rendering.set(true); } } diff --git a/packages/core/src/runtime.rs b/packages/core/src/runtime.rs index eb3a2e582e..d557cd2a02 100644 --- a/packages/core/src/runtime.rs +++ b/packages/core/src/runtime.rs @@ -74,7 +74,7 @@ impl Runtime { Rc::new(Self { sender, - rendering: Cell::new(true), + rendering: Cell::new(false), scope_states: Default::default(), scope_stack: Default::default(), suspense_stack: Default::default(), @@ -108,6 +108,14 @@ impl Runtime { } } + /// Run a closure with the rendering flag set to true + pub(crate) fn while_rendering(&self, f: impl FnOnce() -> T) -> T { + self.rendering.set(true); + let result = f(); + self.rendering.set(false); + result + } + /// Create a scope context. This slab is synchronized with the scope slab. pub(crate) fn create_scope(&self, context: Scope) { let id = context.id; @@ -380,9 +388,7 @@ impl Runtime { ); for listener in listeners.into_iter().rev() { if let AttributeValue::Listener(listener) = listener { - self.rendering.set(false); listener.call(uievent.clone()); - self.rendering.set(true); let metadata = uievent.metadata.borrow(); if !metadata.propagates { @@ -420,9 +426,7 @@ impl Runtime { // Only call the listener if this is the exact target element. if attr.name.get(2..) == Some(name) && target_path == this_path { if let AttributeValue::Listener(listener) = &attr.value { - self.rendering.set(false); listener.call(uievent.clone()); - self.rendering.set(true); break; } } diff --git a/packages/core/src/tasks.rs b/packages/core/src/tasks.rs index 999a9a38e7..b77c309d6e 100644 --- a/packages/core/src/tasks.rs +++ b/packages/core/src/tasks.rs @@ -275,7 +275,6 @@ impl Runtime { // poll the future with the scope on the stack let poll_result = self.with_scope_on_stack(task.scope, || { - self.rendering.set(false); self.current_task.set(Some(id)); let poll_result = task.task.borrow_mut().as_mut().poll(&mut cx); @@ -293,7 +292,6 @@ impl Runtime { poll_result }); - self.rendering.set(true); self.current_task.set(None); poll_result diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index 6ce5718f2b..b5b55e4456 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -511,7 +511,7 @@ impl VirtualDom { // 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); + effect.run(); // Check if any new scopes are queued for rerun self.queue_events(); if self.has_dirty_scopes() { @@ -562,7 +562,10 @@ impl VirtualDom { #[instrument(skip(self, to), level = "trace", name = "VirtualDom::rebuild")] pub fn rebuild(&mut self, to: &mut impl WriteMutations) { let _runtime = RuntimeGuard::new(self.runtime.clone()); - let new_nodes = self.run_scope(ScopeId::ROOT); + let new_nodes = self + .runtime + .clone() + .while_rendering(|| self.run_scope(ScopeId::ROOT)); self.scopes[ScopeId::ROOT.0].last_rendered_node = Some(new_nodes.clone()); @@ -593,7 +596,9 @@ impl VirtualDom { } Work::RerunScope(scope) => { // If the scope is dirty, run the scope and get the mutations - self.run_and_diff_scope(Some(to), scope.id); + self.runtime.clone().while_rendering(|| { + self.run_and_diff_scope(Some(to), scope.id); + }); } } } @@ -700,7 +705,9 @@ impl VirtualDom { .is_some(); if run_scope { // If the scope is dirty, run the scope and get the mutations - self.run_and_diff_scope(None::<&mut NoOpMutations>, scope_id); + self.runtime.clone().while_rendering(|| { + self.run_and_diff_scope(None::<&mut NoOpMutations>, scope_id); + }); tracing::trace!("Ran scope {:?} during suspense", scope_id); } else { diff --git a/packages/playwright-tests/web.spec.js b/packages/playwright-tests/web.spec.js index 0b896ea537..d5cac90cb7 100644 --- a/packages/playwright-tests/web.spec.js +++ b/packages/playwright-tests/web.spec.js @@ -115,3 +115,12 @@ test("onmounted", async ({ page }) => { const mountedDiv = page.locator("div.onmounted-div"); await expect(mountedDiv).toHaveText("onmounted was called 1 times"); }); + +test("web-sys closure", async ({ page }) => { + await page.goto("http://localhost:9999"); + // wait until the div is mounted + const scrollDiv = page.locator("div#web-sys-closure-div"); + await scrollDiv.waitFor({ state: "attached" }); + await page.keyboard.press("Enter"); + await expect(scrollDiv).toHaveText("the keydown event was triggered"); +}); diff --git a/packages/playwright-tests/web/Cargo.toml b/packages/playwright-tests/web/Cargo.toml index c32c917c29..2602a564bd 100644 --- a/packages/playwright-tests/web/Cargo.toml +++ b/packages/playwright-tests/web/Cargo.toml @@ -11,3 +11,5 @@ dioxus = { workspace = true, features = ["web"]} serde_json = "1.0.96" tracing.workspace = true tracing-wasm = "0.2.1" +wasm-bindgen.workspace = true +web-sys = { workspace = true, features = ["Element", "Window"] } diff --git a/packages/playwright-tests/web/src/main.rs b/packages/playwright-tests/web/src/main.rs index 56771808ba..58163ebd78 100644 --- a/packages/playwright-tests/web/src/main.rs +++ b/packages/playwright-tests/web/src/main.rs @@ -1,6 +1,7 @@ // This test is used by playwright configured in the root of the repo use dioxus::prelude::*; +use wasm_bindgen::prelude::*; fn app() -> Element { let mut num = use_signal(|| 0); @@ -53,6 +54,7 @@ fn app() -> Element { div { class: "eval-result", "{eval_result}" } PreventDefault {} OnMounted {} + WebSysClosure {} } } @@ -86,6 +88,49 @@ fn OnMounted() -> Element { } } +// This component tests attaching an event listener to the document with a web-sys closure +// and effect +#[component] +fn WebSysClosure() -> Element { + static TRIGGERED: GlobalSignal = GlobalSignal::new(|| false); + use_effect(|| { + let window = web_sys::window().expect("window not available"); + + // Assert the component contents have been mounted + window + .document() + .unwrap() + .get_element_by_id("web-sys-closure-div") + .expect("Effects should only be run after all contents have bene mounted to the dom"); + + // Make sure passing the runtime into the closure works + let callback = Callback::new(|_| { + assert!(!dioxus::dioxus_core::vdom_is_rendering()); + *TRIGGERED.write() = true; + }); + let closure: Closure = Closure::new({ + move || { + callback(()); + } + }); + + window + .add_event_listener_with_callback("keydown", closure.as_ref().unchecked_ref()) + .expect("Failed to add keydown event listener"); + + closure.forget(); + }); + + rsx! { + div { + id: "web-sys-closure-div", + if TRIGGERED() { + "the keydown event was triggered" + } + } + } +} + fn main() { tracing_wasm::set_as_global_default_with_config( tracing_wasm::WASMLayerConfigBuilder::default()