Fix onmounted event triggering many times (#3346)

* Fix event handler diffing

* Add headless tests for the onmounted event

* fix typo triggerd -> triggered

packages/core/src/diff/node.rs

@@ -439,7 +439,25 @@ impl VNode {
                                 let new = new_attributes_iter.next().unwrap();
                                 // Volatile attributes are attributes that the browser may override so we always update them
                                 let volatile = old.volatile;
-                                if volatile || old.value != new.value {
+                                // We only need to write the attribute if the attribute is volatile or the value has changed
+                                // and this is not an event listener.
+                                // Interpreters reference event listeners by name and element id, so we don't need to write them
+                                // even if the closure has changed.
+                                let attribute_changed = match (&old.value, &new.value) {
+                                    (AttributeValue::Text(l), AttributeValue::Text(r)) => l != r,
+                                    (AttributeValue::Float(l), AttributeValue::Float(r)) => l != r,
+                                    (AttributeValue::Int(l), AttributeValue::Int(r)) => l != r,
+                                    (AttributeValue::Bool(l), AttributeValue::Bool(r)) => l != r,
+                                    (AttributeValue::Any(l), AttributeValue::Any(r)) => {
+                                        !l.as_ref().any_cmp(r.as_ref())
+                                    }
+                                    (AttributeValue::None, AttributeValue::None) => false,
+                                    (AttributeValue::Listener(_), AttributeValue::Listener(_)) => {
+                                        false
+                                    }
+                                    _ => true,
+                                };
+                                if volatile || attribute_changed {
                                     self.write_attribute(
                                         path,
                                         new,

packages/desktop/headless_tests/events.rs

@@ -54,6 +54,7 @@ fn app() -> Element {
 
 fn test_mounted() -> Element {
     use_hook(|| utils::EXPECTED_EVENTS.with_mut(|x| *x += 1));
+    let mut onmounted_triggered = use_signal(|| false);
 
     rsx! {
         div {
@@ -65,6 +66,10 @@ fn test_mounted() -> Element {
                 assert_eq!(rect.width(), 100.0);
                 assert_eq!(rect.height(), 100.0);
                 RECEIVED_EVENTS.with_mut(|x| *x += 1);
+                // Onmounted should only be called once
+                let mut onmounted_triggered_write = onmounted_triggered.write();
+                assert!(!*onmounted_triggered_write);
+                *onmounted_triggered_write = true;
             }
         }
     }

packages/playwright-tests/fullstack.spec.js

@@ -32,4 +32,8 @@ test("hydration", async ({ page }) => {
   // Make sure the error that was thrown on the server is shown in the error boundary on the client
   const errors = page.locator("#errors");
   await expect(errors).toContainText("Hmm, something went wrong.");
+
+  // Expect the onmounted event to be called exactly once.
+  const mountedDiv = page.locator("div.onmounted-div");
+  await expect(mountedDiv).toHaveText("onmounted was called 1 times");
 });

packages/playwright-tests/fullstack/src/main.rs

@@ -40,6 +40,21 @@ fn app() -> Element {
             id: "errors",
             Errors {}
         }
+        OnMounted {}
+    }
+}
+
+#[component]
+fn OnMounted() -> Element {
+    let mut mounted_triggered_count = use_signal(|| 0);
+    rsx! {
+        div {
+            class: "onmounted-div",
+            onmounted: move |_| {
+                mounted_triggered_count += 1;
+            },
+            "onmounted was called {mounted_triggered_count} times"
+        }
     }
 }
 

packages/playwright-tests/liveview.spec.js

@@ -1,72 +1,80 @@
 // @ts-check
-const { test, expect } = require('@playwright/test');
+const { test, expect } = require("@playwright/test");
 
-test('button click', async ({ page }) => {
-  await page.goto('http://127.0.0.1:3030');
+test("button click", async ({ page }) => {
+  await page.goto("http://127.0.0.1:3030");
 
   // Expect the page to contain the counter text.
-  const main = page.locator('#main');
-  await expect(main).toContainText('hello axum! 0');
+  const main = page.locator("#main");
+  await expect(main).toContainText("hello axum! 0");
 
   // Click the increment button.
-  await page.getByRole('button', { name: 'Increment' }).click();
+  await page.getByRole("button", { name: "Increment" }).click();
 
   // Expect the page to contain the updated counter text.
-  await expect(main).toContainText('hello axum! 1');
+  await expect(main).toContainText("hello axum! 1");
 });
 
-test('svg', async ({ page }) => {
-  await page.goto('http://127.0.0.1:3030');
+test("svg", async ({ page }) => {
+  await page.goto("http://127.0.0.1:3030");
 
   // Expect the page to contain the svg.
-  const svg = page.locator('svg');
+  const svg = page.locator("svg");
 
   // Expect the svg to contain the circle.
-  const circle = svg.locator('circle');
-  await expect(circle).toHaveAttribute('cx', '50');
-  await expect(circle).toHaveAttribute('cy', '50');
-  await expect(circle).toHaveAttribute('r', '40');
-  await expect(circle).toHaveAttribute('stroke', 'green');
-  await expect(circle).toHaveAttribute('fill', 'yellow');
+  const circle = svg.locator("circle");
+  await expect(circle).toHaveAttribute("cx", "50");
+  await expect(circle).toHaveAttribute("cy", "50");
+  await expect(circle).toHaveAttribute("r", "40");
+  await expect(circle).toHaveAttribute("stroke", "green");
+  await expect(circle).toHaveAttribute("fill", "yellow");
 });
 
-test('raw attribute', async ({ page }) => {
-  await page.goto('http://127.0.0.1:3030');
+test("raw attribute", async ({ page }) => {
+  await page.goto("http://127.0.0.1:3030");
 
   // Expect the page to contain the div with the raw attribute.
-  const div = page.locator('div.raw-attribute-div');
-  await expect(div).toHaveAttribute('raw-attribute', 'raw-attribute-value');
+  const div = page.locator("div.raw-attribute-div");
+  await expect(div).toHaveAttribute("raw-attribute", "raw-attribute-value");
 });
 
-test('hidden attribute', async ({ page }) => {
-  await page.goto('http://127.0.0.1:3030');
+test("hidden attribute", async ({ page }) => {
+  await page.goto("http://127.0.0.1:3030");
 
   // Expect the page to contain the div with the hidden attribute.
-  const div = page.locator('div.hidden-attribute-div');
-  await expect(div).toHaveAttribute('hidden', 'true');
+  const div = page.locator("div.hidden-attribute-div");
+  await expect(div).toHaveAttribute("hidden", "true");
 });
 
-test('dangerous inner html', async ({ page }) => {
-  await page.goto('http://127.0.0.1:3030');
+test("dangerous inner html", async ({ page }) => {
+  await page.goto("http://127.0.0.1:3030");
 
   // Expect the page to contain the div with the dangerous inner html.
-  const div = page.locator('div.dangerous-inner-html-div');
-  await expect(div).toContainText('hello dangerous inner html');
+  const div = page.locator("div.dangerous-inner-html-div");
+  await expect(div).toContainText("hello dangerous inner html");
 });
 
-test('input value', async ({ page }) => {
-  await page.goto('http://127.0.0.1:3030');
+test("input value", async ({ page }) => {
+  await page.goto("http://127.0.0.1:3030");
 
   // Expect the page to contain the input with the value.
-  const input = page.locator('input');
-  await expect(input).toHaveValue('hello input');
+  const input = page.locator("input");
+  await expect(input).toHaveValue("hello input");
 });
 
-test('style', async ({ page }) => {
-  await page.goto('http://127.0.0.1:3030');
+test("style", async ({ page }) => {
+  await page.goto("http://127.0.0.1:3030");
 
   // Expect the page to contain the div with the style.
-  const div = page.locator('div.style-div');
-  await expect(div).toHaveText('colored text');
-  await expect(div).toHaveCSS('color', 'rgb(255, 0, 0)');
+  const div = page.locator("div.style-div");
+  await expect(div).toHaveText("colored text");
+  await expect(div).toHaveCSS("color", "rgb(255, 0, 0)");
+});
+
+test("onmounted", async ({ page }) => {
+  await page.goto("http://127.0.0.1:3030");
+
+  // Expect the onmounted event to be called exactly once.
+  const mountedDiv = page.locator("div.onmounted-div");
+  await expect(mountedDiv).toHaveText("onmounted was called 1 times");
 });

packages/playwright-tests/liveview/src/main.rs

@@ -20,6 +20,21 @@ fn app() -> Element {
         }
         input { value: "hello input" }
         div { class: "style-div", color: "red", "colored text" }
+        OnMounted {}
+    }
+}
+
+#[component]
+fn OnMounted() -> Element {
+    let mut mounted_triggered_count = use_signal(|| 0);
+    rsx! {
+        div {
+            class: "onmounted-div",
+            onmounted: move |_| {
+                mounted_triggered_count += 1;
+            },
+            "onmounted was called {mounted_triggered_count} times"
+        }
     }
 }
 

packages/playwright-tests/web.spec.js

@@ -107,3 +107,11 @@ test("prevent default", async ({ page }) => {
   // Check that the <a> element changed.
   await expect(a).toHaveText("Psych!");
 });
+
+test("onmounted", async ({ page }) => {
+  await page.goto("http://localhost:9999");
+
+  // Expect the onmounted event to be called exactly once.
+  const mountedDiv = page.locator("div.onmounted-div");
+  await expect(mountedDiv).toHaveText("onmounted was called 1 times");
+});

packages/playwright-tests/web/src/main.rs

@@ -52,6 +52,7 @@ fn app() -> Element {
         }
         div { class: "eval-result", "{eval_result}" }
         PreventDefault {}
+        OnMounted {}
     }
 }
 
@@ -71,6 +72,20 @@ fn PreventDefault() -> Element {
     }
 }
 
+#[component]
+fn OnMounted() -> Element {
+    let mut mounted_triggered_count = use_signal(|| 0);
+    rsx! {
+        div {
+            class: "onmounted-div",
+            onmounted: move |_| {
+                mounted_triggered_count += 1;
+            },
+            "onmounted was called {mounted_triggered_count} times"
+        }
+    }
+}
+
 fn main() {
     tracing_wasm::set_as_global_default_with_config(
         tracing_wasm::WASMLayerConfigBuilder::default()