From 082765f6873d9ba72d67e3383dfab6c901eb772a Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Mon, 23 Aug 2021 13:41:47 -0400 Subject: [PATCH] wip: refactor non_keyed diff --- packages/core/.vscode/settings.json | 2 +- packages/core/src/diff.rs | 186 +++++++++------------------- packages/core/src/diff_stack.rs | 10 +- 3 files changed, 62 insertions(+), 136 deletions(-) diff --git a/packages/core/.vscode/settings.json b/packages/core/.vscode/settings.json index 1c68f97cc..79505b018 100644 --- a/packages/core/.vscode/settings.json +++ b/packages/core/.vscode/settings.json @@ -1,3 +1,3 @@ { - "rust-analyzer.inlayHints.enable": true + "rust-analyzer.inlayHints.enable": false } diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index cc46650c5..d048fb094 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -185,14 +185,6 @@ impl<'bump> DiffMachine<'bump> { self.create_node(node); } - DiffInstruction::Remove { child } => { - self.remove_nodes(Some(child)); - } - - DiffInstruction::RemoveChildren { children } => { - self.remove_nodes(children); - } - DiffInstruction::Mount { and } => { self.mount(and); } @@ -219,7 +211,7 @@ impl<'bump> DiffMachine<'bump> { let root = todo!(); self.mutations.replace_with(root, nodes_created as u32); } - MountType::ReplaceByElementId { old } => { + MountType::ReplaceByElementId { el: old } => { self.mutations.replace_with(old, nodes_created as u32); } @@ -584,73 +576,79 @@ impl<'bump> DiffMachine<'bump> { // Frament nodes cannot generate empty children lists, so we can assume that when a list is empty, it belongs only // to an element, and appending makes sense. fn diff_children(&mut self, old: &'bump [VNode<'bump>], new: &'bump [VNode<'bump>]) { - const IS_EMPTY: bool = true; - const IS_NOT_EMPTY: bool = false; - // Remember, fragments can never be empty (they always have a single child) - match (old.is_empty(), new.is_empty()) { - (IS_EMPTY, IS_EMPTY) => {} - - // Completely adding new nodes, removing any placeholder if it exists - (IS_EMPTY, IS_NOT_EMPTY) => { + match (old, new) { + ([], []) => {} + ([], _) => { self.stack.create_children(new, MountType::Append); } - - // Completely removing old nodes and putting an anchor in its place - // no anchor (old has nodes) and the new is empty - // remove all the old nodes - (IS_NOT_EMPTY, IS_EMPTY) => { + (_, []) => { for node in old { self.remove_nodes(Some(node)); } } + ([VNode::Anchor(old_anchor)], [VNode::Anchor(new_anchor)]) => { + old_anchor.dom_id.set(new_anchor.dom_id.get()); + } + ([VNode::Anchor(anchor)], _) => { + let el = anchor.dom_id.get().unwrap(); + self.stack + .create_children(new, MountType::ReplaceByElementId { el }); + } + (_, [VNode::Anchor(_)]) => { + self.replace_and_create_many_with_one(old, &new[0]); + } + _ => { + let new_is_keyed = new[0].key().is_some(); + let old_is_keyed = old[0].key().is_some(); - (IS_NOT_EMPTY, IS_NOT_EMPTY) => { - let first_old = &old[0]; - let first_new = &new[0]; + debug_assert!( + new.iter().all(|n| n.key().is_some() == new_is_keyed), + "all siblings must be keyed or all siblings must be non-keyed" + ); + debug_assert!( + old.iter().all(|o| o.key().is_some() == old_is_keyed), + "all siblings must be keyed or all siblings must be non-keyed" + ); - match (&first_old, &first_new) { - // Anchors can only appear in empty fragments - (VNode::Anchor(old_anchor), VNode::Anchor(new_anchor)) => { - old_anchor.dom_id.set(new_anchor.dom_id.get()); - } - - // Replace the anchor with whatever new nodes are coming down the pipe - (VNode::Anchor(anchor), _) => { - self.stack - .create_children(new, MountType::Replace { old: first_old }); - } - - // Replace whatever nodes are sitting there with the anchor - (_, VNode::Anchor(anchor)) => { - self.replace_and_create_many_with_one(old, first_new); - } - - // Use the complex diff algorithm to diff the nodes - _ => { - let new_is_keyed = new[0].key().is_some(); - let old_is_keyed = old[0].key().is_some(); - - debug_assert!( - new.iter().all(|n| n.key().is_some() == new_is_keyed), - "all siblings must be keyed or all siblings must be non-keyed" - ); - debug_assert!( - old.iter().all(|o| o.key().is_some() == old_is_keyed), - "all siblings must be keyed or all siblings must be non-keyed" - ); - - if new_is_keyed && old_is_keyed { - self.diff_keyed_children(old, new); - } else { - self.diff_non_keyed_children(old, new); - } - } + if new_is_keyed && old_is_keyed { + self.diff_keyed_children(old, new); + } else { + self.diff_non_keyed_children(old, new); } } } } + // Diff children that are not keyed. + // + // The parent must be on the top of the change list stack when entering this + // function: + // + // [... parent] + // + // the change list stack is in the same state when this function returns. + fn diff_non_keyed_children(&mut self, old: &'bump [VNode<'bump>], new: &'bump [VNode<'bump>]) { + // Handled these cases in `diff_children` before calling this function. + debug_assert!(!new.is_empty()); + debug_assert!(!old.is_empty()); + + for (new, old) in new.iter().zip(old.iter()).rev() { + self.stack.push(DiffInstruction::DiffNode { new, old }); + } + + if old.len() > new.len() { + self.remove_nodes(&old[new.len()..]); + } else { + self.stack.create_children( + &new[old.len()..], + MountType::InsertAfter { + other_node: old.last().unwrap(), + }, + ); + } + } + // Diffing "keyed" children. // // With keyed children, we care about whether we delete, move, or create nodes @@ -1027,70 +1025,6 @@ impl<'bump> DiffMachine<'bump> { } } - // Diff children that are not keyed. - // - // The parent must be on the top of the change list stack when entering this - // function: - // - // [... parent] - // - // the change list stack is in the same state when this function returns. - fn diff_non_keyed_children(&mut self, old: &'bump [VNode<'bump>], new: &'bump [VNode<'bump>]) { - // Handled these cases in `diff_children` before calling this function. - // - debug_assert!(!new.is_empty()); - debug_assert!(!old.is_empty()); - - match old.len().cmp(&new.len()) { - // old.len > new.len -> removing some nodes - Ordering::Greater => { - // Generate instructions to diff the existing elements - for (new_child, old_child) in new.iter().zip(old.iter()).rev() { - self.stack.push(DiffInstruction::DiffNode { - new: new_child, - old: old_child, - }); - } - - self.stack.push(DiffInstruction::RemoveChildren { - children: &old[new.len()..], - }); - } - - // old.len < new.len -> adding some nodes - // this is wrong in the case where we're diffing fragments - // - // we need to save the last old element and then replace it with all the new ones - Ordering::Less => { - // Generate instructions to diff the existing elements - for (new_child, old_child) in new.iter().zip(old.iter()).rev() { - self.stack.push(DiffInstruction::DiffNode { - new: new_child, - old: old_child, - }); - } - - // Generate instructions to add in the new elements - self.stack.create_children( - &new[old.len()..], - MountType::InsertAfter { - other_node: old.last().unwrap(), - }, - ); - } - - // old.len == new.len -> no nodes added/removed, but perhaps changed - Ordering::Equal => { - for (new_child, old_child) in new.iter().zip(old.iter()).rev() { - self.stack.push(DiffInstruction::DiffNode { - new: new_child, - old: old_child, - }); - } - } - } - } - // ===================== // Utilities // ===================== diff --git a/packages/core/src/diff_stack.rs b/packages/core/src/diff_stack.rs index b82658e29..a0528e947 100644 --- a/packages/core/src/diff_stack.rs +++ b/packages/core/src/diff_stack.rs @@ -18,14 +18,6 @@ pub enum DiffInstruction<'a> { node: &'a VNode<'a>, }, - Remove { - child: &'a VNode<'a>, - }, - - RemoveChildren { - children: &'a [VNode<'a>], - }, - Mount { and: MountType<'a>, }, @@ -40,7 +32,7 @@ pub enum MountType<'a> { Absorb, Append, Replace { old: &'a VNode<'a> }, - ReplaceByElementId { old: ElementId }, + ReplaceByElementId { el: ElementId }, InsertAfter { other_node: &'a VNode<'a> }, InsertBefore { other_node: &'a VNode<'a> }, }