From c083943795534a3b8f387f2203097c6b51022e38 Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Wed, 20 Apr 2022 21:28:37 +0900 Subject: [PATCH] Revert "Fix issue with node refs and hydration (#2597)" This reverts commit 469cc341c340bd0093d9233847f523b66a18fd90. --- packages/yew/src/dom_bundle/bcomp.rs | 54 +++----------------- packages/yew/src/dom_bundle/blist.rs | 2 +- packages/yew/src/dom_bundle/utils.rs | 5 +- packages/yew/src/html/component/lifecycle.rs | 10 +++- packages/yew/src/html/component/scope.rs | 9 +++- packages/yew/src/html/mod.rs | 6 +-- packages/yew/src/virtual_dom/vcomp.rs | 6 +-- 7 files changed, 31 insertions(+), 61 deletions(-) diff --git a/packages/yew/src/dom_bundle/bcomp.rs b/packages/yew/src/dom_bundle/bcomp.rs index c749a6428b4..b47c24ff579 100644 --- a/packages/yew/src/dom_bundle/bcomp.rs +++ b/packages/yew/src/dom_bundle/bcomp.rs @@ -12,11 +12,6 @@ use web_sys::Element; pub(super) struct BComp { type_id: TypeId, scope: Box, - // A internal NodeRef passed around to track this components position. This - // is "stable", i.e. does not change when reconciled. - internal_ref: NodeRef, - // The user-passed NodeRef from VComp. Might change every time we reconcile. - // Gets linked to the internal ref node_ref: NodeRef, key: Option, } @@ -62,23 +57,20 @@ impl Reconcilable for VComp { node_ref, key, } = self; - let internal_ref = NodeRef::default(); - node_ref.link(internal_ref.clone()); let scope = mountable.mount( root, - internal_ref.clone(), + node_ref.clone(), parent_scope, parent.to_owned(), next_sibling, ); ( - internal_ref.clone(), + node_ref.clone(), BComp { type_id, node_ref, - internal_ref, key, scope, }, @@ -120,10 +112,10 @@ impl Reconcilable for VComp { } = self; bcomp.key = key; - let old_ref = std::mem::replace(&mut bcomp.node_ref, node_ref); + let old_ref = std::mem::replace(&mut bcomp.node_ref, node_ref.clone()); bcomp.node_ref.reuse(old_ref); - mountable.reuse(bcomp.scope.borrow(), next_sibling); - bcomp.internal_ref.clone() + mountable.reuse(node_ref.clone(), bcomp.scope.borrow(), next_sibling); + node_ref } } @@ -147,24 +139,21 @@ mod feat_hydration { node_ref, key, } = self; - let internal_ref = NodeRef::default(); - node_ref.link(internal_ref.clone()); let scoped = mountable.hydrate( root.clone(), parent_scope, parent.clone(), fragment, - internal_ref.clone(), + node_ref.clone(), ); ( - internal_ref.clone(), + node_ref.clone(), BComp { type_id, scope: scoped, node_ref, - internal_ref, key, }, ) @@ -176,7 +165,7 @@ mod feat_hydration { #[cfg(test)] mod tests { use super::*; - use crate::dom_bundle::{Bundle, Reconcilable, ReconcileTarget}; + use crate::dom_bundle::{Reconcilable, ReconcileTarget}; use crate::scheduler; use crate::{ html, @@ -453,33 +442,6 @@ mod tests { scheduler::start_now(); assert!(node_ref.get().is_none()); } - - #[test] - fn reset_ancestors_node_ref() { - let (root, scope, parent) = setup_parent(); - - let mut bundle = Bundle::new(); - let node_ref_a = NodeRef::default(); - let node_ref_b = NodeRef::default(); - let elem = html! { }; - let node_a = bundle.reconcile(&root, &scope, &parent, NodeRef::default(), elem); - scheduler::start_now(); - let node_a = node_a.get().unwrap(); - - assert!(node_ref_a.get().is_some(), "node_ref_a should be bound"); - - let elem = html! { }; - let node_b = bundle.reconcile(&root, &scope, &parent, NodeRef::default(), elem); - scheduler::start_now(); - let node_b = node_b.get().unwrap(); - - assert_eq!(node_a, node_b, "Comp should have reused the element"); - assert!(node_ref_b.get().is_some(), "node_ref_b should be bound"); - assert!( - node_ref_a.get().is_none(), - "node_ref_a should have been reset when the element was reused." - ); - } } #[cfg(feature = "wasm_test")] diff --git a/packages/yew/src/dom_bundle/blist.rs b/packages/yew/src/dom_bundle/blist.rs index 99afaf229d7..ce6c0878d28 100644 --- a/packages/yew/src/dom_bundle/blist.rs +++ b/packages/yew/src/dom_bundle/blist.rs @@ -474,7 +474,7 @@ mod feat_hydration { let (child_node_ref, child) = child.hydrate(root, parent_scope, parent, fragment); if index == 0 { - node_ref.link(child_node_ref); + node_ref.reuse(child_node_ref); } children.push(child); diff --git a/packages/yew/src/dom_bundle/utils.rs b/packages/yew/src/dom_bundle/utils.rs index ee247ab28b1..6b93a2b3023 100644 --- a/packages/yew/src/dom_bundle/utils.rs +++ b/packages/yew/src/dom_bundle/utils.rs @@ -5,10 +5,7 @@ pub(super) fn insert_node(node: &Node, parent: &Element, next_sibling: Option<&N match next_sibling { Some(next_sibling) => parent .insert_before(node, Some(next_sibling)) - .unwrap_or_else(|err| { - gloo::console::error!("failed to insert node", err, parent, next_sibling, node); - panic!("failed to insert tag before next sibling") - }), + .expect("failed to insert tag before next sibling"), None => parent.append_child(node).expect("failed to append child"), }; } diff --git a/packages/yew/src/html/component/lifecycle.rs b/packages/yew/src/html/component/lifecycle.rs index e9b39b921b4..356a66dde45 100644 --- a/packages/yew/src/html/component/lifecycle.rs +++ b/packages/yew/src/html/component/lifecycle.rs @@ -307,7 +307,7 @@ pub(crate) enum UpdateEvent { Message, /// Wraps properties, node ref, and next sibling for a component #[cfg(feature = "csr")] - Properties(Rc, NodeRef), + Properties(Rc, NodeRef, NodeRef), } pub(crate) struct UpdateRunner { @@ -322,13 +322,16 @@ impl Runnable for UpdateRunner { UpdateEvent::Message => state.inner.flush_messages(), #[cfg(feature = "csr")] - UpdateEvent::Properties(props, next_sibling) => { + UpdateEvent::Properties(props, next_node_ref, next_sibling) => { match state.render_state { #[cfg(feature = "csr")] ComponentRenderState::Render { + ref mut node_ref, next_sibling: ref mut current_next_sibling, .. } => { + // When components are updated, a new node ref could have been passed in + *node_ref = next_node_ref; // When components are updated, their siblings were likely also updated *current_next_sibling = next_sibling; // Only trigger changed if props were changed @@ -337,9 +340,12 @@ impl Runnable for UpdateRunner { #[cfg(feature = "hydration")] ComponentRenderState::Hydration { + ref mut node_ref, next_sibling: ref mut current_next_sibling, .. } => { + // When components are updated, a new node ref could have been passed in + *node_ref = next_node_ref; // When components are updated, their siblings were likely also updated *current_next_sibling = next_sibling; // Only trigger changed if props were changed diff --git a/packages/yew/src/html/component/scope.rs b/packages/yew/src/html/component/scope.rs index 1808ac1cbe3..7ba12f30ef3 100644 --- a/packages/yew/src/html/component/scope.rs +++ b/packages/yew/src/html/component/scope.rs @@ -452,11 +452,16 @@ mod feat_csr { scheduler::start(); } - pub(crate) fn reuse(&self, props: Rc, next_sibling: NodeRef) { + pub(crate) fn reuse( + &self, + props: Rc, + node_ref: NodeRef, + next_sibling: NodeRef, + ) { #[cfg(debug_assertions)] super::super::log_event(self.id, "reuse"); - self.push_update(UpdateEvent::Properties(props, next_sibling)); + self.push_update(UpdateEvent::Properties(props, node_ref, next_sibling)); } } diff --git a/packages/yew/src/html/mod.rs b/packages/yew/src/html/mod.rs index d5983f39188..e28ea170aaa 100644 --- a/packages/yew/src/html/mod.rs +++ b/packages/yew/src/html/mod.rs @@ -145,9 +145,9 @@ mod feat_csr { } let mut this = self.0.borrow_mut(); - let mut existing = node_ref.0.borrow_mut(); - this.node = existing.node.take(); - this.link = existing.link.take(); + let existing = node_ref.0.borrow(); + this.node = existing.node.clone(); + this.link = existing.link.clone(); } /// Link a downstream `NodeRef` diff --git a/packages/yew/src/virtual_dom/vcomp.rs b/packages/yew/src/virtual_dom/vcomp.rs index e556e286757..65ef0f83b5d 100644 --- a/packages/yew/src/virtual_dom/vcomp.rs +++ b/packages/yew/src/virtual_dom/vcomp.rs @@ -65,7 +65,7 @@ pub(crate) trait Mountable { ) -> Box; #[cfg(feature = "csr")] - fn reuse(self: Box, scope: &dyn Scoped, next_sibling: NodeRef); + fn reuse(self: Box, node_ref: NodeRef, scope: &dyn Scoped, next_sibling: NodeRef); #[cfg(feature = "ssr")] fn render_to_string<'a>( @@ -120,9 +120,9 @@ impl Mountable for PropsWrapper { } #[cfg(feature = "csr")] - fn reuse(self: Box, scope: &dyn Scoped, next_sibling: NodeRef) { + fn reuse(self: Box, node_ref: NodeRef, scope: &dyn Scoped, next_sibling: NodeRef) { let scope: Scope = scope.to_any().downcast::(); - scope.reuse(self.props, next_sibling); + scope.reuse(self.props, node_ref, next_sibling); } #[cfg(feature = "ssr")]