فهرست منبع

Hash and compare templates by value in debug mode (#3630)

* Hash and compare templates by value in debug mode

* add a regression test for layout state persisting after navigation

* More reliable static item merged detection
Evan Almloff 4 ماه پیش
والد
کامیت
b443309f6e
3فایلهای تغییر یافته به همراه103 افزوده شده و 6 حذف شده
  1. 33 6
      packages/core/src/nodes.rs
  2. 1 0
      packages/router/tests/via_ssr/main.rs
  3. 69 0
      packages/router/tests/via_ssr/navigation.rs

+ 33 - 6
packages/core/src/nodes.rs

@@ -376,19 +376,46 @@ pub struct Template {
     pub attr_paths: StaticPathArray,
 }
 
+// Are identical static items merged in the current build. Rust doesn't have a cfg(merge_statics) attribute
+// so we have to check this manually
+fn static_items_merged() -> bool {
+    fn a() {}
+    fn b() {}
+
+    a as fn() == b as fn()
+}
+
 impl std::hash::Hash for Template {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
-        std::ptr::hash(self.roots as *const _, state);
-        std::ptr::hash(self.node_paths as *const _, state);
-        std::ptr::hash(self.attr_paths as *const _, state);
+        // If identical static items are merged, we can compare templates by pointer
+        if static_items_merged() {
+            std::ptr::hash(self.roots as *const _, state);
+            std::ptr::hash(self.node_paths as *const _, state);
+            std::ptr::hash(self.attr_paths as *const _, state);
+        }
+        // Otherwise, we hash by value
+        else {
+            self.roots.hash(state);
+            self.node_paths.hash(state);
+            self.attr_paths.hash(state);
+        }
     }
 }
 
 impl PartialEq for Template {
     fn eq(&self, other: &Self) -> bool {
-        std::ptr::eq(self.roots as *const _, other.roots as *const _)
-            && std::ptr::eq(self.node_paths as *const _, other.node_paths as *const _)
-            && std::ptr::eq(self.attr_paths as *const _, other.attr_paths as *const _)
+        // If identical static items are merged, we can compare templates by pointer
+        if static_items_merged() {
+            std::ptr::eq(self.roots as *const _, other.roots as *const _)
+                && std::ptr::eq(self.node_paths as *const _, other.node_paths as *const _)
+                && std::ptr::eq(self.attr_paths as *const _, other.attr_paths as *const _)
+        }
+        // Otherwise, we compare by value
+        else {
+            self.roots == other.roots
+                && self.node_paths == other.node_paths
+                && self.attr_paths == other.attr_paths
+        }
     }
 }
 

+ 1 - 0
packages/router/tests/via_ssr/main.rs

@@ -1,4 +1,5 @@
 mod link;
+mod navigation;
 mod outlet;
 mod redirect;
 mod without_index;

+ 69 - 0
packages/router/tests/via_ssr/navigation.rs

@@ -0,0 +1,69 @@
+use dioxus::prelude::*;
+use dioxus_core::NoOpMutations;
+use std::sync::atomic::AtomicUsize;
+
+// Regression test for <https://github.com/DioxusLabs/dioxus/issues/3235>
+#[test]
+fn layout_retains_state_after_navigation() {
+    let mut vdom = VirtualDom::new(app);
+    vdom.rebuild_in_place();
+
+    vdom.render_immediate(&mut NoOpMutations);
+    let as_string = dioxus_ssr::render(&vdom);
+    assert_eq!(as_string, "Other");
+}
+
+fn app() -> Element {
+    rsx! {
+        Router::<Route> {}
+    }
+}
+
+// Turn off rustfmt since we're doing layouts and routes in the same enum
+#[derive(Routable, Clone, Debug, PartialEq)]
+#[rustfmt::skip]
+enum Route {
+    // Wrap Home in a Navbar Layout
+    #[layout(NavBar)]
+        // The default route is always "/" unless otherwise specified
+        #[route("/")]
+        Home {},
+            
+        #[route("/other")]
+        Other {},
+}
+
+#[component]
+fn NavBar() -> Element {
+    static NAVBARS_CREATED: AtomicUsize = AtomicUsize::new(0);
+    use_hook(|| {
+        let navbars_created = NAVBARS_CREATED.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+        println!("creating navbar #{navbars_created}");
+        if navbars_created > 0 {
+            panic!("layouts should not be recreated when switching between two routes under the nav bar");
+        }
+    });
+
+    // Queue an effect to navigate to the other route after rebuild_in_place
+    use_effect(|| {
+        router().push(Route::Other {});
+    });
+
+    rsx! {
+        Outlet::<Route> {}
+    }
+}
+
+#[component]
+fn Home() -> Element {
+    rsx! {
+        "Home!"
+    }
+}
+
+#[component]
+fn Other() -> Element {
+    rsx! {
+        "Other"
+    }
+}