Assume the virtual dom is not rendering while not diffing components (#3406)

* Invert when the rendering flag is set

* Add a playwright test to make sure effects, and web-sys integration is working

* fix playwright test

Cargo.lock

@@ -4051,6 +4051,8 @@ dependencies = [
  "serde_json",
  "tracing",
  "tracing-wasm",
+ "wasm-bindgen",
+ "web-sys",
 ]
 
 [[package]]

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) {
+    pub(crate) fn run(&self) {
-        runtime.rendering.set(false);
         let mut effect = self.effect.borrow_mut();
         while let Some(f) = effect.pop_front() {
             f();
         }
-        runtime.rendering.set(true);
     }
 }
 

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<T>(&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;
                     }
                 }

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

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 {

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");
+});

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"] }

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<bool> = 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<dyn Fn()> = 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()