Skip to content

Commit

Permalink
speed up detaching
Browse files Browse the repository at this point in the history
Only remove nodes from the DOM when their parent remains
If the parent is also removed, no need to detach the nodes.
  • Loading branch information
WorldSEnder committed Jan 29, 2022
1 parent 7d87e3d commit de7df26
Show file tree
Hide file tree
Showing 12 changed files with 61 additions and 47 deletions.
2 changes: 1 addition & 1 deletion packages/yew/src/dom_bundle/app_handle.rs
Expand Up @@ -37,7 +37,7 @@ where

/// Schedule the app for destruction
pub fn destroy(self) {
self.scope.destroy()
self.scope.destroy(false)
}
}

Expand Down
14 changes: 7 additions & 7 deletions packages/yew/src/dom_bundle/bcomp.rs
Expand Up @@ -40,8 +40,8 @@ impl fmt::Debug for BComp {
}

impl DomBundle for BComp {
fn detach(self, _parent: &Element) {
self.scope.destroy_boxed();
fn detach(self, _parent: &Element, parent_remains: bool) {
self.scope.destroy_boxed(parent_remains);
}

fn shift(&self, next_parent: &Element, next_sibling: NodeRef) {
Expand Down Expand Up @@ -264,9 +264,9 @@ impl ComponentRenderState {
}
}
/// Detach the rendered content from the DOM
pub(crate) fn detach(self) {
pub(crate) fn detach(self, parent_remains: bool) {
if let Some(ref m) = self.parent {
self.root_node.detach(m);
self.root_node.detach(m, parent_remains);
}
}

Expand All @@ -282,8 +282,8 @@ pub trait Scoped {
/// Shift the node associated with this scope to a new place
fn shift_node(&self, parent: Element, next_sibling: NodeRef);
/// Process an event to destroy a component
fn destroy(self);
fn destroy_boxed(self: Box<Self>);
fn destroy(self, parent_remains: bool);
fn destroy_boxed(self: Box<Self>, parent_remains: bool);
}

#[cfg(test)]
Expand Down Expand Up @@ -565,7 +565,7 @@ mod tests {
let (_, elem) = elem.attach(&scope, &parent, NodeRef::default());
let parent_node = parent.deref();
assert_eq!(node_ref.get(), parent_node.first_child());
elem.detach(&parent);
elem.detach(&parent, true);
assert!(node_ref.get().is_none());
}
}
Expand Down
8 changes: 4 additions & 4 deletions packages/yew/src/dom_bundle/blist.rs
Expand Up @@ -151,7 +151,7 @@ impl BList {
if lefts.len() < rights.len() {
for r in rights.drain(lefts.len()..) {
test_log!("removing: {:?}", r);
r.detach(parent);
r.detach(parent, true);
}
}

Expand Down Expand Up @@ -336,7 +336,7 @@ impl BList {
// Step 2.3. Remove any extra rights
for KeyedEntry(_, r) in spare_bundles.drain() {
test_log!("removing: {:?}", r);
r.detach(parent);
r.detach(parent, true);
}

// Step 3. Diff matching children at the start
Expand All @@ -354,9 +354,9 @@ impl BList {
}

impl DomBundle for BList {
fn detach(self, parent: &Element) {
fn detach(self, parent: &Element, parent_remains: bool) {
for child in self.rev_children.into_iter() {
child.detach(parent);
child.detach(parent, parent_remains);
}
}

Expand Down
16 changes: 9 additions & 7 deletions packages/yew/src/dom_bundle/bnode.rs
Expand Up @@ -43,19 +43,21 @@ impl BNode {

impl DomBundle for BNode {
/// Remove VNode from parent.
fn detach(self, parent: &Element) {
fn detach(self, parent: &Element, parent_remains: bool) {
match self {
Self::BTag(vtag) => vtag.detach(parent),
Self::BText(btext) => btext.detach(parent),
Self::BComp(bsusp) => bsusp.detach(parent),
Self::BList(blist) => blist.detach(parent),
Self::BTag(vtag) => vtag.detach(parent, parent_remains),
Self::BText(btext) => btext.detach(parent, parent_remains),
Self::BComp(bsusp) => bsusp.detach(parent, parent_remains),
Self::BList(blist) => blist.detach(parent, parent_remains),
Self::BRef(ref node) => {
// Always remove nodes passed in by the user to disconnect it from
// internally generated nodes
if parent.remove_child(node).is_err() {
console::warn!("Node not found to remove VRef");
}
}
Self::BPortal(bportal) => bportal.detach(parent),
Self::BSuspense(bsusp) => bsusp.detach(parent),
Self::BPortal(bportal) => bportal.detach(parent, parent_remains),
Self::BSuspense(bsusp) => bsusp.detach(parent, parent_remains),
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/yew/src/dom_bundle/bportal.rs
Expand Up @@ -20,9 +20,9 @@ pub struct BPortal {
}

impl DomBundle for BPortal {
fn detach(self, _: &Element) {
fn detach(self, _: &Element, _parent_remains: bool) {
test_log!("Detaching portal from host{:?}", self.host.outer_html());
self.node.detach(&self.host);
self.node.detach(&self.host, true);
test_log!("Detached portal from host{:?}", self.host.outer_html());
}

Expand Down
14 changes: 9 additions & 5 deletions packages/yew/src/dom_bundle/bsuspense.rs
Expand Up @@ -30,12 +30,12 @@ impl BSuspense {
}

impl DomBundle for BSuspense {
fn detach(self, parent: &Element) {
fn detach(self, parent: &Element, parent_remains: bool) {
if let Some(fallback) = self.fallback_bundle {
fallback.detach(parent);
self.children_bundle.detach(&self.detached_parent);
fallback.detach(parent, parent_remains);
self.children_bundle.detach(&self.detached_parent, false);
} else {
self.children_bundle.detach(parent);
self.children_bundle.detach(parent, parent_remains);
}
}

Expand Down Expand Up @@ -164,7 +164,11 @@ impl Reconcilable for VSuspense {
}
// Freshly unsuspended. Detach fallback from the DOM, then shift children into it.
(false, Some(_)) => {
suspense.fallback_bundle.take().unwrap().detach(parent);
suspense
.fallback_bundle
.take()
.unwrap()
.detach(parent, true);

children_bundle.shift(parent, next_sibling.clone());
children.reconcile_node(parent_scope, parent, next_sibling, children_bundle)
Expand Down
13 changes: 8 additions & 5 deletions packages/yew/src/dom_bundle/btag/mod.rs
Expand Up @@ -69,16 +69,19 @@ pub struct BTag {
}

impl DomBundle for BTag {
fn detach(self, parent: &Element) {
fn detach(self, parent: &Element, parent_remains: bool) {
self.listeners.unregister();

let node = self.reference;
// recursively remove its children
if let BTagInner::Other { child_bundle, .. } = self.inner {
child_bundle.detach(&node);
child_bundle.detach(&node, false);
}
if parent.remove_child(&node).is_err() {
console::warn!("Node not found to remove VTag");
if parent_remains {
let removed_child = parent.remove_child(&node);
if removed_child.is_err() {
console::warn!("Node not found to remove VTag");
}
}
// It could be that the ref was already reused when rendering another element.
// Only unset the ref it still belongs to our node
Expand Down Expand Up @@ -761,7 +764,7 @@ mod tests {
assert_vtag_ref(&elem);
let (_, elem) = elem.attach(&scope, &parent, NodeRef::default());
assert_eq!(node_ref.get(), parent.first_child());
elem.detach(&parent);
elem.detach(&parent, true);
assert!(node_ref.get().is_none());
}

Expand Down
11 changes: 7 additions & 4 deletions packages/yew/src/dom_bundle/btext.rs
Expand Up @@ -15,10 +15,13 @@ pub struct BText {
}

impl DomBundle for BText {
fn detach(self, parent: &Element) {
let node = &self.text_node;
if parent.remove_child(node).is_err() {
console::warn!("Node not found to remove VText");
fn detach(self, parent: &Element, parent_remains: bool) {
if parent_remains {
let node = &self.text_node;
let removed_child = parent.remove_child(node);
if removed_child.is_err() {
console::warn!("Node not found to remove VText");
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/yew/src/dom_bundle/mod.rs
Expand Up @@ -41,7 +41,7 @@ use web_sys::{Element, Node};

trait DomBundle {
/// Remove self from parent.
fn detach(self, parent: &Element);
fn detach(self, parent: &Element, parent_remains: bool);

/// Move elements from one parent to another parent.
/// This is for example used by `VSuspense` to preserve component state without detaching
Expand Down Expand Up @@ -116,7 +116,7 @@ trait Reconcilable {
{
let (self_ref, self_) = self.attach(parent_scope, parent, next_sibling);
let ancestor = std::mem::replace(bundle, self_.into());
ancestor.detach(parent);
ancestor.detach(parent, true);
self_ref
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/yew/src/dom_bundle/tests/layout_tests.rs
Expand Up @@ -77,7 +77,7 @@ pub fn diff_layouts(layouts: Vec<TestLayout<'_>>) {
);

// Detach
bundle.detach(&parent_element);
bundle.detach(&parent_element, true);
assert_eq!(
parent_element.inner_html(),
"END",
Expand Down Expand Up @@ -127,7 +127,7 @@ pub fn diff_layouts(layouts: Vec<TestLayout<'_>>) {

// Detach last layout
if let Some(bundle) = bundle {
bundle.detach(&parent_element);
bundle.detach(&parent_element, true);
}
assert_eq!(
parent_element.inner_html(),
Expand Down
8 changes: 5 additions & 3 deletions packages/yew/src/html/component/lifecycle.rs
Expand Up @@ -131,10 +131,11 @@ impl<COMP: BaseComponent> ComponentState<COMP> {
}
}

pub(crate) fn push_destroy(self: &Rc<Self>) {
pub(crate) fn push_destroy(self: &Rc<Self>, parent_remains: bool) {
let mut cbs = self.callbacks.borrow_mut();
cbs.destroy = Some(DestroyPayload {
_phantom: PhantomData,
parent_remains,
});
drop(cbs);
scheduler::push_component_destroy(self.clone());
Expand Down Expand Up @@ -257,12 +258,13 @@ impl<COMP: BaseComponent> InnerComponentState<COMP> {
}

struct DestroyPayload<COMP: BaseComponent> {
parent_remains: bool,
_phantom: PhantomData<COMP>,
}

impl<COMP: BaseComponent> RunDestroy for ComponentState<COMP> {
fn run_destroy(self: Rc<Self>) {
let _payload = {
let payload = {
let mut cbs = self.callbacks.borrow_mut();
match cbs.destroy.take() {
Some(p) => p,
Expand All @@ -274,7 +276,7 @@ impl<COMP: BaseComponent> RunDestroy for ComponentState<COMP> {
super::log_event(self.vcomp_id, "destroy");

state.component.destroy(&state.context);
state.render_state.detach();
state.render_state.detach(payload.parent_remains);
state.node_ref.set(None);
}
}
Expand Down
10 changes: 5 additions & 5 deletions packages/yew/src/html/component/scope.rs
Expand Up @@ -127,14 +127,14 @@ impl<COMP: BaseComponent> Scoped for Scope<COMP> {
}))
}

fn destroy(self) {
self.state.push_destroy();
fn destroy(self, parent_remains: bool) {
self.state.push_destroy(parent_remains);
// Not guaranteed to already have the scheduler started
scheduler::start();
}

fn destroy_boxed(self: Box<Self>) {
self.destroy()
fn destroy_boxed(self: Box<Self>, parent_remains: bool) {
self.destroy(parent_remains)
}

fn shift_node(&self, parent: Element, next_sibling: NodeRef) {
Expand Down Expand Up @@ -336,7 +336,7 @@ mod feat_ssr {
let self_any_scope = self.to_any();
html.render_to_string(w, &self_any_scope).await;

self.destroy();
self.destroy(true);
}
}
}
Expand Down

0 comments on commit de7df26

Please sign in to comment.