Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hash and compare templates by value in debug mode #3630

Merged
merged 4 commits into from
Feb 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions packages/core/src/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/router/tests/via_ssr/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod link;
mod navigation;
mod outlet;
mod redirect;
mod without_index;
69 changes: 69 additions & 0 deletions packages/router/tests/via_ssr/navigation.rs
Original file line number Diff line number Diff line change
@@ -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"
}
}
Loading