From b7e5c26c2b81c1431cac7890b5516bec027a5d40 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Tue, 4 Jul 2023 17:48:23 -0700
Subject: [PATCH 01/25] Preliminary implementation of clone_within and
 clone_external

---
 rbx_dom_weak/src/dom.rs | 106 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index 3ce3513f..ac2e495c 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -18,6 +18,102 @@ pub struct WeakDom {
     unique_ids: HashSet<UniqueId>,
 }
 
+#[derive(Debug)]
+struct CloneContext<'dom> {
+    // TODO: Should we use a specific type for these instead of (Ref, Ref)?
+    queue: VecDeque<(Ref, Ref)>,
+
+    ref_rewrites: HashMap<Ref, Ref>,
+    dom: &'dom mut WeakDom,
+    subtree_root: Ref,
+}
+
+impl<'dom> CloneContext<'dom> {
+    fn new(dom: &'dom mut WeakDom, subtree_root: Ref) -> Self {
+        Self {
+            ref_rewrites: HashMap::default(),
+            queue: VecDeque::default(),
+            subtree_root,
+            dom,
+        }
+    }
+
+    pub fn clone_within(mut self) -> Ref {
+        let root_builder = self.instancebuilder_from_ref(self.subtree_root);
+        let root_ref = self.dom.insert(Ref::none(), root_builder);
+
+        while let Some((cloned_parent, uncloned_child)) = self.queue.pop_front() {
+            let builder = self.instancebuilder_from_ref(uncloned_child);
+            self.dom.insert(cloned_parent, builder);
+        }
+
+        for (_, new_ref) in self.ref_rewrites.iter() {
+            let instance = self
+                .dom
+                .get_by_ref_mut(*new_ref)
+                .expect("Cannot rewrite refs on an instance that does not exist");
+
+            Self::rewrite_refs(&self.ref_rewrites, instance);
+        }
+
+        root_ref
+    }
+
+    pub fn clone_external(mut self, dest: &mut WeakDom) -> Ref {
+        let root_builder = self.instancebuilder_from_ref(self.subtree_root);
+        let root_ref = dest.insert(Ref::none(), root_builder);
+
+        while let Some((cloned_parent, uncloned_child)) = self.queue.pop_front() {
+            let builder = self.instancebuilder_from_ref(uncloned_child);
+            dest.insert(cloned_parent, builder);
+        }
+
+        for (_, new_ref) in self.ref_rewrites.iter() {
+            let instance = dest
+                .get_by_ref_mut(*new_ref)
+                .expect("Cannot rewrite refs on an instance that does not exist");
+
+            Self::rewrite_refs(&self.ref_rewrites, instance);
+        }
+
+        root_ref
+    }
+
+    fn rewrite_refs(ref_rewrites: &HashMap<Ref, Ref>, instance: &mut Instance) {
+        for prop_value in instance.properties.values_mut() {
+            if let Variant::Ref(old_ref) = prop_value {
+                // NOTE: It is possible to get None here if the ref points to
+                // something outside of the newly cloned instance hierarchy
+                if let Some(new_ref) = ref_rewrites.get(old_ref) {
+                    *prop_value = Variant::Ref(*new_ref);
+                }
+            }
+        }
+    }
+
+    fn instancebuilder_from_ref(&mut self, referent: Ref) -> InstanceBuilder {
+        let (new_ref, builder, children) = {
+            let instance = self
+                .dom
+                .get_by_ref(referent)
+                .expect("Cannot clone an instance that does not exist");
+
+            let builder = InstanceBuilder::new(instance.class.to_string())
+                .with_name(instance.name.to_string())
+                .with_properties(instance.properties.clone());
+
+            (builder.referent, builder, instance.children.to_vec())
+        };
+
+        for uncloned_child in children.iter() {
+            self.queue.push_back((new_ref, *uncloned_child))
+        }
+
+        self.ref_rewrites.insert(referent, new_ref);
+        builder
+    }
+}
+
 impl WeakDom {
     /// Construct a new `WeakDom` described by the given [`InstanceBuilder`].
     pub fn new(builder: InstanceBuilder) -> WeakDom {
@@ -235,6 +331,16 @@ impl WeakDom {
         dest_parent.children.push(referent);
     }
 
+    /// Ook
+    pub fn clone_within(&mut self, referent: Ref) -> Ref {
+        CloneContext::new(self, referent).clone_within()
+    }
+
+    /// OOoook
+    pub fn clone_external(&mut self, referent: Ref, dest: &mut WeakDom) -> Ref {
+        CloneContext::new(self, referent).clone_external(dest)
+    }
+
     fn inner_insert(&mut self, referent: Ref, instance: Instance) {
         self.instances.insert(referent, instance);
 

From bdbaeb216aa871272c05bc1f8436d51a1b38d359 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 09:27:05 -0700
Subject: [PATCH 02/25] Rename CloneContext.dom -> CloneContext.source

---
 rbx_dom_weak/src/dom.rs | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index ac2e495c..dd12246d 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -19,37 +19,35 @@ pub struct WeakDom {
 }
 
 #[derive(Debug)]
-struct CloneContext<'dom> {
-    // TODO: Should we use a specific type for these instead of (Ref, Ref)?
+struct CloneContext<'src> {
     queue: VecDeque<(Ref, Ref)>,
-
     ref_rewrites: HashMap<Ref, Ref>,
-    dom: &'dom mut WeakDom,
+    source: &'src mut WeakDom,
     subtree_root: Ref,
 }
 
-impl<'dom> CloneContext<'dom> {
-    fn new(dom: &'dom mut WeakDom, subtree_root: Ref) -> Self {
+impl<'src> CloneContext<'src> {
+    fn new(dom: &'src mut WeakDom, subtree_root: Ref) -> Self {
         Self {
             ref_rewrites: HashMap::default(),
             queue: VecDeque::default(),
             subtree_root,
-            dom,
+            source: dom,
         }
     }
 
     pub fn clone_within(mut self) -> Ref {
         let root_builder = self.instancebuilder_from_ref(self.subtree_root);
-        let root_ref = self.dom.insert(Ref::none(), root_builder);
+        let root_ref = self.source.insert(Ref::none(), root_builder);
 
         while let Some((cloned_parent, uncloned_child)) = self.queue.pop_front() {
             let builder = self.instancebuilder_from_ref(uncloned_child);
-            self.dom.insert(cloned_parent, builder);
+            self.source.insert(cloned_parent, builder);
         }
 
         for (_, new_ref) in self.ref_rewrites.iter() {
             let instance = self
-                .dom
+                .source
                 .get_by_ref_mut(*new_ref)
                 .expect("Cannot rewrite refs on an instance that does not exist");
 
@@ -94,7 +92,7 @@ impl<'dom> CloneContext<'dom> {
     fn instancebuilder_from_ref(&mut self, referent: Ref) -> InstanceBuilder {
         let (new_ref, builder, children) = {
             let instance = self
-                .dom
+                .source
                 .get_by_ref(referent)
                 .expect("Cannot clone an instance that does not exist");
 

From 378ff0da61a28551ef9c1c762e82ca0a4688d7db Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 09:27:21 -0700
Subject: [PATCH 03/25] Add tests

---
 rbx_dom_weak/src/dom.rs | 73 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 71 insertions(+), 2 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index dd12246d..f5da1c3f 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -80,8 +80,8 @@ impl<'src> CloneContext<'src> {
     fn rewrite_refs(ref_rewrites: &HashMap<Ref, Ref>, instance: &mut Instance) {
         for prop_value in instance.properties.values_mut() {
             if let Variant::Ref(old_ref) = prop_value {
-                // NOTE: It is possible to get None here if the ref points to
-                // something outside of the newly cloned instance hierarchy
+                // We only want to rewrite Refs if they point to instances within the
+                // cloned subtree
                 if let Some(new_ref) = ref_rewrites.get(old_ref) {
                     *prop_value = Variant::Ref(*new_ref);
                 }
@@ -447,6 +447,75 @@ mod test {
         insta::assert_yaml_snapshot!(viewer.view_children(&dom));
     }
 
+    #[test]
+    fn clone_within() {
+        let mut dom = {
+            let mut child1 = InstanceBuilder::new("Part");
+            let mut child2 = InstanceBuilder::new("Part");
+
+            child1 = child1.with_property("RefProp", child2.referent);
+            child2 = child2.with_property("RefProp", child1.referent);
+
+            WeakDom::new(
+                InstanceBuilder::new("Folder")
+                    .with_name("Root")
+                    .with_children([child1, child2]),
+            )
+        };
+
+        let cloned_root = dom.clone_within(dom.root_ref);
+
+        assert!(
+            dom.get_by_ref(cloned_root).unwrap().parent.is_none(),
+            "parent of cloned subtree root should be none directly after a clone"
+        );
+
+        dom.transfer_within(cloned_root, dom.root_ref);
+
+        // This snapshot should have a clone of the root Folder under itself, with the ref
+        // properties in the cloned subtree pointing to the cloned instances.
+        let mut viewer = DomViewer::new();
+        insta::assert_yaml_snapshot!(viewer.view(&dom));
+    }
+
+    #[test]
+    fn clone_into_external() {
+        let mut dom = {
+            let mut child1 = InstanceBuilder::new("Part");
+            let mut child2 = InstanceBuilder::new("Part");
+
+            child1 = child1.with_property("RefProp", child2.referent);
+            child2 = child2.with_property("RefProp", child1.referent);
+
+            WeakDom::new(
+                InstanceBuilder::new("Folder")
+                    .with_name("Root")
+                    .with_children([child1, child2]),
+            )
+        };
+
+        let mut other_dom = WeakDom::new(InstanceBuilder::new("DataModel"));
+        let cloned_root = dom.clone_into_external(dom.root_ref, &mut other_dom);
+
+        assert!(
+            other_dom.get_by_ref(cloned_root).unwrap().parent.is_none(),
+            "parent of cloned subtree root should be none directly after a clone"
+        );
+
+        other_dom.transfer_within(cloned_root, other_dom.root_ref);
+
+        let mut viewer = DomViewer::new();
+
+        // This snapshot is here just to show that the ref props are rewritten after being
+        // cloned into the other dom. It should contain a Folder at the root with two
+        // Parts as children
+        insta::assert_yaml_snapshot!(viewer.view(&dom));
+
+        // This snapshot should have a clone of the root Folder under the other dom's
+        // DataModel, with the ref properties pointing to the newly cloned parts.
+        insta::assert_yaml_snapshot!(viewer.view(&other_dom));
+    }
+
     #[test]
     fn large_depth_tree() {
         // We've had issues with stack overflows when creating WeakDoms with

From 74bedb541bfd458acc5b0039d3e2235abfb509c5 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 09:41:00 -0700
Subject: [PATCH 04/25] Rename clone_external -> clone_into_external

---
 rbx_dom_weak/src/dom.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index f5da1c3f..5a2cd046 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -57,7 +57,7 @@ impl<'src> CloneContext<'src> {
         root_ref
     }
 
-    pub fn clone_external(mut self, dest: &mut WeakDom) -> Ref {
+    pub fn clone_into_external(mut self, dest: &mut WeakDom) -> Ref {
         let root_builder = self.instancebuilder_from_ref(self.subtree_root);
         let root_ref = dest.insert(Ref::none(), root_builder);
 
@@ -335,8 +335,8 @@ impl WeakDom {
     }
 
     /// OOoook
-    pub fn clone_external(&mut self, referent: Ref, dest: &mut WeakDom) -> Ref {
-        CloneContext::new(self, referent).clone_external(dest)
+    pub fn clone_into_external(&mut self, referent: Ref, dest: &mut WeakDom) -> Ref {
+        CloneContext::new(self, referent).clone_into_external(dest)
     }
 
     fn inner_insert(&mut self, referent: Ref, instance: Instance) {

From 7352f7b05c6facbf029439cf31995733f1e39049 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 09:54:49 -0700
Subject: [PATCH 05/25] Add snapshots

Looking good!
---
 ...eak__dom__test__clone_into_external-2.snap | 27 +++++++++++++
 ..._weak__dom__test__clone_into_external.snap | 22 +++++++++++
 ...rbx_dom_weak__dom__test__clone_within.snap | 39 +++++++++++++++++++
 3 files changed, 88 insertions(+)
 create mode 100644 rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external-2.snap
 create mode 100644 rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external.snap
 create mode 100644 rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_within.snap

diff --git a/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external-2.snap b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external-2.snap
new file mode 100644
index 00000000..9697b823
--- /dev/null
+++ b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external-2.snap
@@ -0,0 +1,27 @@
+---
+source: rbx_dom_weak/src/dom.rs
+expression: viewer.view(&other_dom)
+---
+referent: referent-3
+name: DataModel
+class: DataModel
+properties: {}
+children:
+  - referent: referent-4
+    name: Root
+    class: Folder
+    properties: {}
+    children:
+      - referent: referent-5
+        name: Part
+        class: Part
+        properties:
+          RefProp: referent-6
+        children: []
+      - referent: referent-6
+        name: Part
+        class: Part
+        properties:
+          RefProp: referent-5
+        children: []
+
diff --git a/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external.snap b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external.snap
new file mode 100644
index 00000000..780ca1a1
--- /dev/null
+++ b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external.snap
@@ -0,0 +1,22 @@
+---
+source: rbx_dom_weak/src/dom.rs
+expression: viewer.view(&dom)
+---
+referent: referent-0
+name: Root
+class: Folder
+properties: {}
+children:
+  - referent: referent-1
+    name: Part
+    class: Part
+    properties:
+      RefProp: referent-2
+    children: []
+  - referent: referent-2
+    name: Part
+    class: Part
+    properties:
+      RefProp: referent-1
+    children: []
+
diff --git a/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_within.snap b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_within.snap
new file mode 100644
index 00000000..b1cdf37a
--- /dev/null
+++ b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_within.snap
@@ -0,0 +1,39 @@
+---
+source: rbx_dom_weak/src/dom.rs
+expression: viewer.view(&dom)
+---
+referent: referent-0
+name: Root
+class: Folder
+properties: {}
+children:
+  - referent: referent-1
+    name: Part
+    class: Part
+    properties:
+      RefProp: referent-2
+    children: []
+  - referent: referent-2
+    name: Part
+    class: Part
+    properties:
+      RefProp: referent-1
+    children: []
+  - referent: referent-3
+    name: Root
+    class: Folder
+    properties: {}
+    children:
+      - referent: referent-4
+        name: Part
+        class: Part
+        properties:
+          RefProp: referent-5
+        children: []
+      - referent: referent-5
+        name: Part
+        class: Part
+        properties:
+          RefProp: referent-4
+        children: []
+

From d0ee59156fa8fe87b26d4fa35bc3ba5074373cff Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 10:21:36 -0700
Subject: [PATCH 06/25] Refactor: CloneContext is now POD, move implementations
 into WeakDom

No longer have to take &mut self for clone_into_external
---
 rbx_dom_weak/src/dom.rs | 161 +++++++++++++++++-----------------------
 1 file changed, 67 insertions(+), 94 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index 5a2cd046..455a3292 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -18,98 +18,10 @@ pub struct WeakDom {
     unique_ids: HashSet<UniqueId>,
 }
 
-#[derive(Debug)]
-struct CloneContext<'src> {
+#[derive(Debug, Default)]
+struct CloneContext {
     queue: VecDeque<(Ref, Ref)>,
     ref_rewrites: HashMap<Ref, Ref>,
-    source: &'src mut WeakDom,
-    subtree_root: Ref,
-}
-
-impl<'src> CloneContext<'src> {
-    fn new(dom: &'src mut WeakDom, subtree_root: Ref) -> Self {
-        Self {
-            ref_rewrites: HashMap::default(),
-            queue: VecDeque::default(),
-            subtree_root,
-            source: dom,
-        }
-    }
-
-    pub fn clone_within(mut self) -> Ref {
-        let root_builder = self.instancebuilder_from_ref(self.subtree_root);
-        let root_ref = self.source.insert(Ref::none(), root_builder);
-
-        while let Some((cloned_parent, uncloned_child)) = self.queue.pop_front() {
-            let builder = self.instancebuilder_from_ref(uncloned_child);
-            self.source.insert(cloned_parent, builder);
-        }
-
-        for (_, new_ref) in self.ref_rewrites.iter() {
-            let instance = self
-                .source
-                .get_by_ref_mut(*new_ref)
-                .expect("Cannot rewrite refs on an instance that does not exist");
-
-            Self::rewrite_refs(&self.ref_rewrites, instance);
-        }
-
-        root_ref
-    }
-
-    pub fn clone_into_external(mut self, dest: &mut WeakDom) -> Ref {
-        let root_builder = self.instancebuilder_from_ref(self.subtree_root);
-        let root_ref = dest.insert(Ref::none(), root_builder);
-
-        while let Some((cloned_parent, uncloned_child)) = self.queue.pop_front() {
-            let builder = self.instancebuilder_from_ref(uncloned_child);
-            dest.insert(cloned_parent, builder);
-        }
-
-        for (_, new_ref) in self.ref_rewrites.iter() {
-            let instance = dest
-                .get_by_ref_mut(*new_ref)
-                .expect("Cannot rewrite refs on an instance that does not exist");
-
-            Self::rewrite_refs(&self.ref_rewrites, instance);
-        }
-
-        root_ref
-    }
-
-    fn rewrite_refs(ref_rewrites: &HashMap<Ref, Ref>, instance: &mut Instance) {
-        for prop_value in instance.properties.values_mut() {
-            if let Variant::Ref(old_ref) = prop_value {
-                // We only want to rewrite Refs if they point to instances within the
-                // cloned subtree
-                if let Some(new_ref) = ref_rewrites.get(old_ref) {
-                    *prop_value = Variant::Ref(*new_ref);
-                }
-            }
-        }
-    }
-
-    fn instancebuilder_from_ref(&mut self, referent: Ref) -> InstanceBuilder {
-        let (new_ref, builder, children) = {
-            let instance = self
-                .source
-                .get_by_ref(referent)
-                .expect("Cannot clone an instance that does not exist");
-
-            let builder = InstanceBuilder::new(instance.class.to_string())
-                .with_name(instance.name.to_string())
-                .with_properties(instance.properties.clone());
-
-            (builder.referent, builder, instance.children.to_vec())
-        };
-
-        for uncloned_child in children.iter() {
-            self.queue.push_back((new_ref, *uncloned_child))
-        }
-
-        self.ref_rewrites.insert(referent, new_ref);
-        builder
-    }
 }
 
 impl WeakDom {
@@ -331,12 +243,34 @@ impl WeakDom {
 
     /// Ook
     pub fn clone_within(&mut self, referent: Ref) -> Ref {
-        CloneContext::new(self, referent).clone_within()
+        let mut ctx = CloneContext::default();
+        let root_builder = self.instancebuilder_from_ref(&mut ctx, referent);
+        let root_ref = self.insert(Ref::none(), root_builder);
+
+        while let Some((cloned_parent, uncloned_child)) = ctx.queue.pop_front() {
+            let builder = self.instancebuilder_from_ref(&mut ctx, uncloned_child);
+            self.insert(cloned_parent, builder);
+        }
+
+        Self::rewrite_refs(self, &ctx);
+
+        root_ref
     }
 
     /// OOoook
-    pub fn clone_into_external(&mut self, referent: Ref, dest: &mut WeakDom) -> Ref {
-        CloneContext::new(self, referent).clone_into_external(dest)
+    pub fn clone_into_external(&self, referent: Ref, dest: &mut WeakDom) -> Ref {
+        let mut ctx = CloneContext::default();
+        let root_builder = self.instancebuilder_from_ref(&mut ctx, referent);
+        let root_ref = dest.insert(Ref::none(), root_builder);
+
+        while let Some((cloned_parent, uncloned_child)) = ctx.queue.pop_front() {
+            let builder = self.instancebuilder_from_ref(&mut ctx, uncloned_child);
+            dest.insert(cloned_parent, builder);
+        }
+
+        Self::rewrite_refs(dest, &ctx);
+
+        root_ref
     }
 
     fn inner_insert(&mut self, referent: Ref, instance: Instance) {
@@ -381,6 +315,45 @@ impl WeakDom {
 
         instance
     }
+
+    fn instancebuilder_from_ref(&self, ctx: &mut CloneContext, referent: Ref) -> InstanceBuilder {
+        let (new_ref, builder, children) = {
+            let instance = self
+                .get_by_ref(referent)
+                .expect("Cannot clone an instance that does not exist");
+
+            let builder = InstanceBuilder::new(instance.class.to_string())
+                .with_name(instance.name.to_string())
+                .with_properties(instance.properties.clone());
+
+            (builder.referent, builder, instance.children.to_vec())
+        };
+
+        for uncloned_child in children.iter() {
+            ctx.queue.push_back((new_ref, *uncloned_child))
+        }
+
+        ctx.ref_rewrites.insert(referent, new_ref);
+        builder
+    }
+
+    fn rewrite_refs(dom: &mut WeakDom, ctx: &CloneContext) {
+        for (_, new_ref) in ctx.ref_rewrites.iter() {
+            let instance = dom
+                .get_by_ref_mut(*new_ref)
+                .expect("Cannot rewrite refs on an instance that does not exist");
+
+            for prop_value in instance.properties.values_mut() {
+                if let Variant::Ref(old_ref) = prop_value {
+                    // We only want to rewrite Refs if they point to instances within the
+                    // cloned subtree
+                    if let Some(new_ref) = ctx.ref_rewrites.get(old_ref) {
+                        *prop_value = Variant::Ref(*new_ref);
+                    }
+                }
+            }
+        }
+    }
 }
 
 #[cfg(test)]
@@ -480,7 +453,7 @@ mod test {
 
     #[test]
     fn clone_into_external() {
-        let mut dom = {
+        let dom = {
             let mut child1 = InstanceBuilder::new("Part");
             let mut child2 = InstanceBuilder::new("Part");
 

From 1643fd5e30ad3f26150637da64029ef9007d42a5 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 10:35:46 -0700
Subject: [PATCH 07/25] Add a little docs

---
 rbx_dom_weak/src/dom.rs | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index 455a3292..fe855805 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -241,7 +241,12 @@ impl WeakDom {
         dest_parent.children.push(referent);
     }
 
-    /// Ook
+    /// Clone an instance and all its descendants into the same WeakDom.
+    ///
+    /// After the operation, the root of the cloned subtree has no parent.
+    ///
+    /// Any Ref properties that point to instances contained in the subtree are
+    /// rewritten to point to the cloned instances.
     pub fn clone_within(&mut self, referent: Ref) -> Ref {
         let mut ctx = CloneContext::default();
         let root_builder = self.instancebuilder_from_ref(&mut ctx, referent);
@@ -257,7 +262,12 @@ impl WeakDom {
         root_ref
     }
 
-    /// OOoook
+    /// Clone an instance and all its descendants into a different WeakDom.
+    ///
+    /// After the operation, the root of the cloned subtree has no parent.
+    ///
+    /// Any Ref properties that point to instances contained in the subtree are
+    /// rewritten to point to the cloned instances.
     pub fn clone_into_external(&self, referent: Ref, dest: &mut WeakDom) -> Ref {
         let mut ctx = CloneContext::default();
         let root_builder = self.instancebuilder_from_ref(&mut ctx, referent);

From 6fdb7642a961639e36c18375283a3be55300bfab Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 10:41:09 -0700
Subject: [PATCH 08/25] Rename instancebuilder_from_ref ->
 ref_into_instancebuilder

---
 rbx_dom_weak/src/dom.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index fe855805..a47cd49d 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -249,11 +249,11 @@ impl WeakDom {
     /// rewritten to point to the cloned instances.
     pub fn clone_within(&mut self, referent: Ref) -> Ref {
         let mut ctx = CloneContext::default();
-        let root_builder = self.instancebuilder_from_ref(&mut ctx, referent);
+        let root_builder = self.ref_into_instancebuilder(&mut ctx, referent);
         let root_ref = self.insert(Ref::none(), root_builder);
 
         while let Some((cloned_parent, uncloned_child)) = ctx.queue.pop_front() {
-            let builder = self.instancebuilder_from_ref(&mut ctx, uncloned_child);
+            let builder = self.ref_into_instancebuilder(&mut ctx, uncloned_child);
             self.insert(cloned_parent, builder);
         }
 
@@ -270,11 +270,11 @@ impl WeakDom {
     /// rewritten to point to the cloned instances.
     pub fn clone_into_external(&self, referent: Ref, dest: &mut WeakDom) -> Ref {
         let mut ctx = CloneContext::default();
-        let root_builder = self.instancebuilder_from_ref(&mut ctx, referent);
+        let root_builder = self.ref_into_instancebuilder(&mut ctx, referent);
         let root_ref = dest.insert(Ref::none(), root_builder);
 
         while let Some((cloned_parent, uncloned_child)) = ctx.queue.pop_front() {
-            let builder = self.instancebuilder_from_ref(&mut ctx, uncloned_child);
+            let builder = self.ref_into_instancebuilder(&mut ctx, uncloned_child);
             dest.insert(cloned_parent, builder);
         }
 
@@ -326,7 +326,7 @@ impl WeakDom {
         instance
     }
 
-    fn instancebuilder_from_ref(&self, ctx: &mut CloneContext, referent: Ref) -> InstanceBuilder {
+    fn ref_into_instancebuilder(&self, ctx: &mut CloneContext, referent: Ref) -> InstanceBuilder {
         let (new_ref, builder, children) = {
             let instance = self
                 .get_by_ref(referent)

From b888d1070666483b3ace19fa71bcf7098bc87f79 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 10:46:27 -0700
Subject: [PATCH 09/25] Refactor: ref_into_instancebuilder simplification

---
 rbx_dom_weak/src/dom.rs | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index a47cd49d..8c04dde0 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -326,24 +326,26 @@ impl WeakDom {
         instance
     }
 
-    fn ref_into_instancebuilder(&self, ctx: &mut CloneContext, referent: Ref) -> InstanceBuilder {
-        let (new_ref, builder, children) = {
-            let instance = self
-                .get_by_ref(referent)
-                .expect("Cannot clone an instance that does not exist");
+    fn ref_into_instancebuilder(
+        &self,
+        ctx: &mut CloneContext,
+        original_ref: Ref,
+    ) -> InstanceBuilder {
+        let instance = self
+            .get_by_ref(original_ref)
+            .expect("Cannot clone an instance that does not exist");
 
-            let builder = InstanceBuilder::new(instance.class.to_string())
-                .with_name(instance.name.to_string())
-                .with_properties(instance.properties.clone());
+        let builder = InstanceBuilder::new(instance.class.to_string())
+            .with_name(instance.name.to_string())
+            .with_properties(instance.properties.clone());
 
-            (builder.referent, builder, instance.children.to_vec())
-        };
+        let new_ref = builder.referent;
 
-        for uncloned_child in children.iter() {
+        for uncloned_child in instance.children.iter() {
             ctx.queue.push_back((new_ref, *uncloned_child))
         }
 
-        ctx.ref_rewrites.insert(referent, new_ref);
+        ctx.ref_rewrites.insert(original_ref, new_ref);
         builder
     }
 

From c6c90a61d97c2b908d521513bed551541d317b83 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 10:46:59 -0700
Subject: [PATCH 10/25] Rename old_ref -> original_ref

---
 rbx_dom_weak/src/dom.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index 8c04dde0..d7e617c9 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -356,10 +356,10 @@ impl WeakDom {
                 .expect("Cannot rewrite refs on an instance that does not exist");
 
             for prop_value in instance.properties.values_mut() {
-                if let Variant::Ref(old_ref) = prop_value {
+                if let Variant::Ref(original_ref) = prop_value {
                     // We only want to rewrite Refs if they point to instances within the
                     // cloned subtree
-                    if let Some(new_ref) = ctx.ref_rewrites.get(old_ref) {
+                    if let Some(new_ref) = ctx.ref_rewrites.get(original_ref) {
                         *prop_value = Variant::Ref(*new_ref);
                     }
                 }

From 046ebc07fd63daee184b918a58f91cc558469611 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 10:48:28 -0700
Subject: [PATCH 11/25] Consume CloneContext in rewrite_refs

---
 rbx_dom_weak/src/dom.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index d7e617c9..935d5976 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -257,7 +257,7 @@ impl WeakDom {
             self.insert(cloned_parent, builder);
         }
 
-        Self::rewrite_refs(self, &ctx);
+        Self::rewrite_refs(self, ctx);
 
         root_ref
     }
@@ -278,7 +278,7 @@ impl WeakDom {
             dest.insert(cloned_parent, builder);
         }
 
-        Self::rewrite_refs(dest, &ctx);
+        Self::rewrite_refs(dest, ctx);
 
         root_ref
     }
@@ -349,7 +349,7 @@ impl WeakDom {
         builder
     }
 
-    fn rewrite_refs(dom: &mut WeakDom, ctx: &CloneContext) {
+    fn rewrite_refs(dom: &mut WeakDom, ctx: CloneContext) {
         for (_, new_ref) in ctx.ref_rewrites.iter() {
             let instance = dom
                 .get_by_ref_mut(*new_ref)

From 7b944561207534cf22d9abcb74b0f7fe91ad5240 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 23:13:11 -0700
Subject: [PATCH 12/25] ref_into_instancebuiler->clone_ref_as_builder, add a
 little docs

---
 rbx_dom_weak/src/dom.rs | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index 935d5976..e438b094 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -249,11 +249,11 @@ impl WeakDom {
     /// rewritten to point to the cloned instances.
     pub fn clone_within(&mut self, referent: Ref) -> Ref {
         let mut ctx = CloneContext::default();
-        let root_builder = self.ref_into_instancebuilder(&mut ctx, referent);
+        let root_builder = self.clone_ref_as_builder(&mut ctx, referent);
         let root_ref = self.insert(Ref::none(), root_builder);
 
         while let Some((cloned_parent, uncloned_child)) = ctx.queue.pop_front() {
-            let builder = self.ref_into_instancebuilder(&mut ctx, uncloned_child);
+            let builder = self.clone_ref_as_builder(&mut ctx, uncloned_child);
             self.insert(cloned_parent, builder);
         }
 
@@ -270,11 +270,11 @@ impl WeakDom {
     /// rewritten to point to the cloned instances.
     pub fn clone_into_external(&self, referent: Ref, dest: &mut WeakDom) -> Ref {
         let mut ctx = CloneContext::default();
-        let root_builder = self.ref_into_instancebuilder(&mut ctx, referent);
+        let root_builder = self.clone_ref_as_builder(&mut ctx, referent);
         let root_ref = dest.insert(Ref::none(), root_builder);
 
         while let Some((cloned_parent, uncloned_child)) = ctx.queue.pop_front() {
-            let builder = self.ref_into_instancebuilder(&mut ctx, uncloned_child);
+            let builder = self.clone_ref_as_builder(&mut ctx, uncloned_child);
             dest.insert(cloned_parent, builder);
         }
 
@@ -326,11 +326,12 @@ impl WeakDom {
         instance
     }
 
-    fn ref_into_instancebuilder(
-        &self,
-        ctx: &mut CloneContext,
-        original_ref: Ref,
-    ) -> InstanceBuilder {
+    /// Clones the instance with the given referent and context into a new
+    /// InstanceBuilder.
+    ///
+    /// This method only clones the instance's class name, name, and properties; it
+    /// does not clone any children.
+    fn clone_ref_as_builder(&self, ctx: &mut CloneContext, original_ref: Ref) -> InstanceBuilder {
         let instance = self
             .get_by_ref(original_ref)
             .expect("Cannot clone an instance that does not exist");

From 5e327659ef3a3b6f559a1f4984c9325e277f4ef7 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 23:13:37 -0700
Subject: [PATCH 13/25] Refactor: WeakDom::rewrite_refs ->
 CloneContext::rewrite_refs

CloneContext is still sorta POD...
---
 rbx_dom_weak/src/dom.rs | 44 ++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index e438b094..de265926 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -24,6 +24,26 @@ struct CloneContext {
     ref_rewrites: HashMap<Ref, Ref>,
 }
 
+impl CloneContext {
+    fn rewrite_refs(self, dom: &mut WeakDom) {
+        for (_, new_ref) in self.ref_rewrites.iter() {
+            let instance = dom
+                .get_by_ref_mut(*new_ref)
+                .expect("Cannot rewrite refs on an instance that does not exist");
+
+            for prop_value in instance.properties.values_mut() {
+                if let Variant::Ref(original_ref) = prop_value {
+                    // We only want to rewrite Refs if they point to instances within the
+                    // cloned subtree
+                    if let Some(new_ref) = self.ref_rewrites.get(original_ref) {
+                        *prop_value = Variant::Ref(*new_ref);
+                    }
+                }
+            }
+        }
+    }
+}
+
 impl WeakDom {
     /// Construct a new `WeakDom` described by the given [`InstanceBuilder`].
     pub fn new(builder: InstanceBuilder) -> WeakDom {
@@ -257,8 +277,7 @@ impl WeakDom {
             self.insert(cloned_parent, builder);
         }
 
-        Self::rewrite_refs(self, ctx);
-
+        ctx.rewrite_refs(self);
         root_ref
     }
 
@@ -278,8 +297,7 @@ impl WeakDom {
             dest.insert(cloned_parent, builder);
         }
 
-        Self::rewrite_refs(dest, ctx);
-
+        ctx.rewrite_refs(dest);
         root_ref
     }
 
@@ -349,24 +367,6 @@ impl WeakDom {
         ctx.ref_rewrites.insert(original_ref, new_ref);
         builder
     }
-
-    fn rewrite_refs(dom: &mut WeakDom, ctx: CloneContext) {
-        for (_, new_ref) in ctx.ref_rewrites.iter() {
-            let instance = dom
-                .get_by_ref_mut(*new_ref)
-                .expect("Cannot rewrite refs on an instance that does not exist");
-
-            for prop_value in instance.properties.values_mut() {
-                if let Variant::Ref(original_ref) = prop_value {
-                    // We only want to rewrite Refs if they point to instances within the
-                    // cloned subtree
-                    if let Some(new_ref) = ctx.ref_rewrites.get(original_ref) {
-                        *prop_value = Variant::Ref(*new_ref);
-                    }
-                }
-            }
-        }
-    }
 }
 
 #[cfg(test)]

From fc26bdc7b686c91b0d1be03e776d24f571110f19 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 23:14:52 -0700
Subject: [PATCH 14/25] Move CloneContext down below WeakDom

---
 rbx_dom_weak/src/dom.rs | 52 ++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index de265926..f5948379 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -18,32 +18,6 @@ pub struct WeakDom {
     unique_ids: HashSet<UniqueId>,
 }
 
-#[derive(Debug, Default)]
-struct CloneContext {
-    queue: VecDeque<(Ref, Ref)>,
-    ref_rewrites: HashMap<Ref, Ref>,
-}
-
-impl CloneContext {
-    fn rewrite_refs(self, dom: &mut WeakDom) {
-        for (_, new_ref) in self.ref_rewrites.iter() {
-            let instance = dom
-                .get_by_ref_mut(*new_ref)
-                .expect("Cannot rewrite refs on an instance that does not exist");
-
-            for prop_value in instance.properties.values_mut() {
-                if let Variant::Ref(original_ref) = prop_value {
-                    // We only want to rewrite Refs if they point to instances within the
-                    // cloned subtree
-                    if let Some(new_ref) = self.ref_rewrites.get(original_ref) {
-                        *prop_value = Variant::Ref(*new_ref);
-                    }
-                }
-            }
-        }
-    }
-}
-
 impl WeakDom {
     /// Construct a new `WeakDom` described by the given [`InstanceBuilder`].
     pub fn new(builder: InstanceBuilder) -> WeakDom {
@@ -369,6 +343,32 @@ impl WeakDom {
     }
 }
 
+#[derive(Debug, Default)]
+struct CloneContext {
+    queue: VecDeque<(Ref, Ref)>,
+    ref_rewrites: HashMap<Ref, Ref>,
+}
+
+impl CloneContext {
+    fn rewrite_refs(self, dom: &mut WeakDom) {
+        for (_, new_ref) in self.ref_rewrites.iter() {
+            let instance = dom
+                .get_by_ref_mut(*new_ref)
+                .expect("Cannot rewrite refs on an instance that does not exist");
+
+            for prop_value in instance.properties.values_mut() {
+                if let Variant::Ref(original_ref) = prop_value {
+                    // We only want to rewrite Refs if they point to instances within the
+                    // cloned subtree
+                    if let Some(new_ref) = self.ref_rewrites.get(original_ref) {
+                        *prop_value = Variant::Ref(*new_ref);
+                    }
+                }
+            }
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;

From 85f49a8cfb50f6004ade161eed153685216c0f89 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 23:34:34 -0700
Subject: [PATCH 15/25] Refactor: move WeakDom::clone_ref_as_builder into
 CloneContext

CloneContext definitely ain't no POD anymore

It was nice while it lasted...
---
 rbx_dom_weak/src/dom.rs | 56 ++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index f5948379..f7f0e90d 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -243,11 +243,11 @@ impl WeakDom {
     /// rewritten to point to the cloned instances.
     pub fn clone_within(&mut self, referent: Ref) -> Ref {
         let mut ctx = CloneContext::default();
-        let root_builder = self.clone_ref_as_builder(&mut ctx, referent);
+        let root_builder = ctx.clone_ref_as_builder(self, referent);
         let root_ref = self.insert(Ref::none(), root_builder);
 
         while let Some((cloned_parent, uncloned_child)) = ctx.queue.pop_front() {
-            let builder = self.clone_ref_as_builder(&mut ctx, uncloned_child);
+            let builder = ctx.clone_ref_as_builder(self, uncloned_child);
             self.insert(cloned_parent, builder);
         }
 
@@ -263,11 +263,11 @@ impl WeakDom {
     /// rewritten to point to the cloned instances.
     pub fn clone_into_external(&self, referent: Ref, dest: &mut WeakDom) -> Ref {
         let mut ctx = CloneContext::default();
-        let root_builder = self.clone_ref_as_builder(&mut ctx, referent);
+        let root_builder = ctx.clone_ref_as_builder(self, referent);
         let root_ref = dest.insert(Ref::none(), root_builder);
 
         while let Some((cloned_parent, uncloned_child)) = ctx.queue.pop_front() {
-            let builder = self.clone_ref_as_builder(&mut ctx, uncloned_child);
+            let builder = ctx.clone_ref_as_builder(self, uncloned_child);
             dest.insert(cloned_parent, builder);
         }
 
@@ -317,30 +317,6 @@ impl WeakDom {
 
         instance
     }
-
-    /// Clones the instance with the given referent and context into a new
-    /// InstanceBuilder.
-    ///
-    /// This method only clones the instance's class name, name, and properties; it
-    /// does not clone any children.
-    fn clone_ref_as_builder(&self, ctx: &mut CloneContext, original_ref: Ref) -> InstanceBuilder {
-        let instance = self
-            .get_by_ref(original_ref)
-            .expect("Cannot clone an instance that does not exist");
-
-        let builder = InstanceBuilder::new(instance.class.to_string())
-            .with_name(instance.name.to_string())
-            .with_properties(instance.properties.clone());
-
-        let new_ref = builder.referent;
-
-        for uncloned_child in instance.children.iter() {
-            ctx.queue.push_back((new_ref, *uncloned_child))
-        }
-
-        ctx.ref_rewrites.insert(original_ref, new_ref);
-        builder
-    }
 }
 
 #[derive(Debug, Default)]
@@ -367,6 +343,30 @@ impl CloneContext {
             }
         }
     }
+
+    /// Clones the instance with the given referent and context into a new
+    /// InstanceBuilder.
+    ///
+    /// This method only clones the instance's class name, name, and properties; it
+    /// does not clone any children.
+    fn clone_ref_as_builder(&mut self, source: &WeakDom, original_ref: Ref) -> InstanceBuilder {
+        let instance = source
+            .get_by_ref(original_ref)
+            .expect("Cannot clone an instance that does not exist");
+
+        let builder = InstanceBuilder::new(instance.class.to_string())
+            .with_name(instance.name.to_string())
+            .with_properties(instance.properties.clone());
+
+        let new_ref = builder.referent;
+
+        for uncloned_child in instance.children.iter() {
+            self.queue.push_back((new_ref, *uncloned_child))
+        }
+
+        self.ref_rewrites.insert(original_ref, new_ref);
+        builder
+    }
 }
 
 #[cfg(test)]

From 0c6a90e645c20f2dd644dc7809d226ee880b5a90 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 23:47:00 -0700
Subject: [PATCH 16/25] Clarify the docs

---
 rbx_dom_weak/src/dom.rs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index f7f0e90d..c747aa2d 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -235,7 +235,8 @@ impl WeakDom {
         dest_parent.children.push(referent);
     }
 
-    /// Clone an instance and all its descendants into the same WeakDom.
+    /// Clone the instance with the given `referent` and all its descendants
+    /// (i.e. the entire subtree) into the same WeakDom.
     ///
     /// After the operation, the root of the cloned subtree has no parent.
     ///
@@ -255,7 +256,8 @@ impl WeakDom {
         root_ref
     }
 
-    /// Clone an instance and all its descendants into a different WeakDom.
+    /// Clone the instance with the given `referent` and all its descendants (i.e. the
+    /// entire subtree) into the given WeakDom.
     ///
     /// After the operation, the root of the cloned subtree has no parent.
     ///

From 1ba6eabae79f80276d1d7af14a8f5e6645c5cca9 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Wed, 5 Jul 2023 23:52:29 -0700
Subject: [PATCH 17/25] Docs docs docs

---
 rbx_dom_weak/src/dom.rs | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index c747aa2d..fb3ca952 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -328,6 +328,8 @@ struct CloneContext {
 }
 
 impl CloneContext {
+    /// On any instances cloned during the operation, rewrite any Ref properties that
+    /// point to instances that were also cloned.
     fn rewrite_refs(self, dom: &mut WeakDom) {
         for (_, new_ref) in self.ref_rewrites.iter() {
             let instance = dom
@@ -346,8 +348,9 @@ impl CloneContext {
         }
     }
 
-    /// Clones the instance with the given referent and context into a new
-    /// InstanceBuilder.
+    /// Clone the instance with the given `referent` and `source` WeakDom into a new
+    /// InstanceBuilder, and record the mapping of the original referent to the new
+    /// referent.
     ///
     /// This method only clones the instance's class name, name, and properties; it
     /// does not clone any children.

From a253fefd32ecf2f1f3437dc50699ce9464f8bec9 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 7 Jul 2023 01:01:51 -0700
Subject: [PATCH 18/25] Set ref properties to none if they DNE in the
 destination WeakDom

---
 rbx_dom_weak/src/dom.rs | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index fb3ca952..81d3f937 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -330,18 +330,34 @@ struct CloneContext {
 impl CloneContext {
     /// On any instances cloned during the operation, rewrite any Ref properties that
     /// point to instances that were also cloned.
-    fn rewrite_refs(self, dom: &mut WeakDom) {
+    fn rewrite_refs(self, dest: &mut WeakDom) {
+        let original_dest_refs: HashSet<Ref> = self
+            .ref_rewrites
+            .iter()
+            .filter_map(|(original_ref, _)| {
+                if dest.instances.contains_key(original_ref) {
+                    Some(*original_ref)
+                } else {
+                    None
+                }
+            })
+            .collect();
+
         for (_, new_ref) in self.ref_rewrites.iter() {
-            let instance = dom
+            let instance = dest
                 .get_by_ref_mut(*new_ref)
                 .expect("Cannot rewrite refs on an instance that does not exist");
 
             for prop_value in instance.properties.values_mut() {
                 if let Variant::Ref(original_ref) = prop_value {
-                    // We only want to rewrite Refs if they point to instances within the
-                    // cloned subtree
                     if let Some(new_ref) = self.ref_rewrites.get(original_ref) {
+                        // If the ref points to an instances contained within the
+                        // cloned subtree, rewrite it as the corresponding new ref
                         *prop_value = Variant::Ref(*new_ref);
+                    } else if !original_dest_refs.contains(original_ref) {
+                        // If the ref points to an instance that does not exist
+                        // in the destination WeakDom, rewrite it as none
+                        *prop_value = Variant::Ref(Ref::none())
                     }
                 }
             }

From 7a88c91d4f2563c69ef055e849c69bae8d6b8a88 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 7 Jul 2023 01:24:42 -0700
Subject: [PATCH 19/25] Edit clone_into_external test to show refs that DNE are
 set to none

---
 rbx_dom_weak/src/dom.rs                       | 15 ++++++++------
 ...eak__dom__test__clone_into_external-2.snap | 20 ++++++++++++-------
 ..._weak__dom__test__clone_into_external.snap | 10 ++++++++--
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index 81d3f937..05d0e2ba 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -488,16 +488,18 @@ mod test {
     #[test]
     fn clone_into_external() {
         let dom = {
-            let mut child1 = InstanceBuilder::new("Part");
-            let mut child2 = InstanceBuilder::new("Part");
+            let mut child1 = InstanceBuilder::new("Part").with_name("Child1");
+            let mut child2 = InstanceBuilder::new("Part").with_name("Child2");
+            let mut child3 = InstanceBuilder::new("Part").with_name("Child3");
 
             child1 = child1.with_property("RefProp", child2.referent);
             child2 = child2.with_property("RefProp", child1.referent);
+            child3 = child3.with_property("RefProp", Ref::new());
 
             WeakDom::new(
                 InstanceBuilder::new("Folder")
                     .with_name("Root")
-                    .with_children([child1, child2]),
+                    .with_children([child1, child2, child3]),
             )
         };
 
@@ -514,12 +516,13 @@ mod test {
         let mut viewer = DomViewer::new();
 
         // This snapshot is here just to show that the ref props are rewritten after being
-        // cloned into the other dom. It should contain a Folder at the root with two
+        // cloned into the other dom. It should contain a Folder at the root with the three
         // Parts as children
         insta::assert_yaml_snapshot!(viewer.view(&dom));
 
-        // This snapshot should have a clone of the root Folder under the other dom's
-        // DataModel, with the ref properties pointing to the newly cloned parts.
+        // This snapshot should have a clone of the root Folder under the other
+        // dom's DataModel, with Child1's and Child2's ref properties rewritten to point
+        // to the newly cloned instances, and Child3's ref property rewritten to none.
         insta::assert_yaml_snapshot!(viewer.view(&other_dom));
     }
 
diff --git a/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external-2.snap b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external-2.snap
index 9697b823..0958c8f4 100644
--- a/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external-2.snap
+++ b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external-2.snap
@@ -2,26 +2,32 @@
 source: rbx_dom_weak/src/dom.rs
 expression: viewer.view(&other_dom)
 ---
-referent: referent-3
+referent: referent-4
 name: DataModel
 class: DataModel
 properties: {}
 children:
-  - referent: referent-4
+  - referent: referent-5
     name: Root
     class: Folder
     properties: {}
     children:
-      - referent: referent-5
-        name: Part
+      - referent: referent-6
+        name: Child1
+        class: Part
+        properties:
+          RefProp: referent-7
+        children: []
+      - referent: referent-7
+        name: Child2
         class: Part
         properties:
           RefProp: referent-6
         children: []
-      - referent: referent-6
-        name: Part
+      - referent: referent-8
+        name: Child3
         class: Part
         properties:
-          RefProp: referent-5
+          RefProp: "null"
         children: []
 
diff --git a/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external.snap b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external.snap
index 780ca1a1..3a35f191 100644
--- a/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external.snap
+++ b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_into_external.snap
@@ -8,15 +8,21 @@ class: Folder
 properties: {}
 children:
   - referent: referent-1
-    name: Part
+    name: Child1
     class: Part
     properties:
       RefProp: referent-2
     children: []
   - referent: referent-2
-    name: Part
+    name: Child2
     class: Part
     properties:
       RefProp: referent-1
     children: []
+  - referent: referent-3
+    name: Child3
+    class: Part
+    properties:
+      RefProp: "[unknown ID]"
+    children: []
 

From 040df5eb1d72afaf5b87bee6621676c169efb375 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 7 Jul 2023 04:23:19 -0700
Subject: [PATCH 20/25] Put (valid) ref pointing outside the subtree in
 clone_within test

Currently fails because the rewrite_refs implementation in
a253fefd32ecf2f1f3437dc50699ce9464f8bec9 rewrites all refs that point
outside the cloned subtree to none, which is not correct

Revert "Fix ref rewrites"

This reverts commit 41a2fe6f1418a48e86d622a336854f9276977c38.
---
 rbx_dom_weak/src/dom.rs                       | 26 +++++++--------
 ...rbx_dom_weak__dom__test__clone_within.snap | 33 ++++++++-----------
 2 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index 05d0e2ba..7cb55ce9 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -456,31 +456,31 @@ mod test {
 
     #[test]
     fn clone_within() {
+        let mut child1 = InstanceBuilder::new("Part").with_name("Child1");
+        let child1_ref = child1.referent;
+
         let mut dom = {
-            let mut child1 = InstanceBuilder::new("Part");
-            let mut child2 = InstanceBuilder::new("Part");
+            let root = InstanceBuilder::new("Folder").with_name("Root");
+            let mut child2 = InstanceBuilder::new("Part").with_name("Child2");
 
-            child1 = child1.with_property("RefProp", child2.referent);
+            child1 = child1.with_property("RefProp", root.referent);
             child2 = child2.with_property("RefProp", child1.referent);
 
-            WeakDom::new(
-                InstanceBuilder::new("Folder")
-                    .with_name("Root")
-                    .with_children([child1, child2]),
-            )
+            WeakDom::new(root.with_child(child1.with_child(child2)))
         };
 
-        let cloned_root = dom.clone_within(dom.root_ref);
+        let cloned_child1_ref = dom.clone_within(child1_ref);
 
         assert!(
-            dom.get_by_ref(cloned_root).unwrap().parent.is_none(),
+            dom.get_by_ref(cloned_child1_ref).unwrap().parent.is_none(),
             "parent of cloned subtree root should be none directly after a clone"
         );
 
-        dom.transfer_within(cloned_root, dom.root_ref);
+        dom.transfer_within(cloned_child1_ref, dom.root_ref);
 
-        // This snapshot should have a clone of the root Folder under itself, with the ref
-        // properties in the cloned subtree pointing to the cloned instances.
+        // This snapshot should have a clone of the Child1 subtree under the
+        // root Folder, with Child2's ref property pointing to the cloned
+        // Child1, and Child1's ref property pointing to the root Folder.
         let mut viewer = DomViewer::new();
         insta::assert_yaml_snapshot!(viewer.view(&dom));
     }
diff --git a/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_within.snap b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_within.snap
index b1cdf37a..c796968e 100644
--- a/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_within.snap
+++ b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_within.snap
@@ -8,32 +8,27 @@ class: Folder
 properties: {}
 children:
   - referent: referent-1
-    name: Part
+    name: Child1
     class: Part
     properties:
-      RefProp: referent-2
-    children: []
-  - referent: referent-2
-    name: Part
-    class: Part
-    properties:
-      RefProp: referent-1
-    children: []
-  - referent: referent-3
-    name: Root
-    class: Folder
-    properties: {}
+      RefProp: referent-0
     children:
-      - referent: referent-4
-        name: Part
+      - referent: referent-2
+        name: Child2
         class: Part
         properties:
-          RefProp: referent-5
+          RefProp: referent-1
         children: []
-      - referent: referent-5
-        name: Part
+  - referent: referent-3
+    name: Child1
+    class: Part
+    properties:
+      RefProp: "null"
+    children:
+      - referent: referent-4
+        name: Child2
         class: Part
         properties:
-          RefProp: referent-4
+          RefProp: referent-3
         children: []
 

From aafe32ce148b38d40dbdffc47fd00f75c71192e1 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 7 Jul 2023 03:59:53 -0700
Subject: [PATCH 21/25] Correctly rewrite refs that point outside subtree in
 rewrite_refs

---
 rbx_dom_weak/src/dom.rs                       | 26 +++++++++++--------
 ...rbx_dom_weak__dom__test__clone_within.snap |  2 +-
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index 7cb55ce9..26fa4116 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -331,17 +331,21 @@ impl CloneContext {
     /// On any instances cloned during the operation, rewrite any Ref properties that
     /// point to instances that were also cloned.
     fn rewrite_refs(self, dest: &mut WeakDom) {
-        let original_dest_refs: HashSet<Ref> = self
-            .ref_rewrites
-            .iter()
-            .filter_map(|(original_ref, _)| {
-                if dest.instances.contains_key(original_ref) {
-                    Some(*original_ref)
-                } else {
-                    None
+        let mut existing_dest_refs = HashSet::<Ref>::new();
+
+        for (_, new_ref) in self.ref_rewrites.iter() {
+            let instance = dest
+                .get_by_ref(*new_ref)
+                .expect("Cannot rewrite refs on an instance that does not exist");
+
+            for prop_value in instance.properties.values() {
+                if let Variant::Ref(value) = prop_value {
+                    if dest.instances.contains_key(value) {
+                        existing_dest_refs.insert(*value);
+                    }
                 }
-            })
-            .collect();
+            }
+        }
 
         for (_, new_ref) in self.ref_rewrites.iter() {
             let instance = dest
@@ -354,7 +358,7 @@ impl CloneContext {
                         // If the ref points to an instances contained within the
                         // cloned subtree, rewrite it as the corresponding new ref
                         *prop_value = Variant::Ref(*new_ref);
-                    } else if !original_dest_refs.contains(original_ref) {
+                    } else if !existing_dest_refs.contains(original_ref) {
                         // If the ref points to an instance that does not exist
                         // in the destination WeakDom, rewrite it as none
                         *prop_value = Variant::Ref(Ref::none())
diff --git a/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_within.snap b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_within.snap
index c796968e..660cc443 100644
--- a/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_within.snap
+++ b/rbx_dom_weak/src/snapshots/rbx_dom_weak__dom__test__clone_within.snap
@@ -23,7 +23,7 @@ children:
     name: Child1
     class: Part
     properties:
-      RefProp: "null"
+      RefProp: referent-0
     children:
       - referent: referent-4
         name: Child2

From e16aeaa6545d6e9652b24eb9b476a48745dd4661 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 7 Jul 2023 13:52:08 -0700
Subject: [PATCH 22/25] Don't need this type annotation

---
 rbx_dom_weak/src/dom.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index 26fa4116..4a095bd7 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -331,7 +331,7 @@ impl CloneContext {
     /// On any instances cloned during the operation, rewrite any Ref properties that
     /// point to instances that were also cloned.
     fn rewrite_refs(self, dest: &mut WeakDom) {
-        let mut existing_dest_refs = HashSet::<Ref>::new();
+        let mut existing_dest_refs = HashSet::new();
 
         for (_, new_ref) in self.ref_rewrites.iter() {
             let instance = dest

From e2bab9afd7ad796fc994d10c49419a49b2d20d1b Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 7 Jul 2023 15:14:13 -0700
Subject: [PATCH 23/25] Mention the out-of-subtree ref case in
 clone_into_external docs

Co-authored-by: Micah <dekkonot@rocketmail.com>
---
 rbx_dom_weak/src/dom.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index 4a095bd7..fe30c581 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -262,7 +262,8 @@ impl WeakDom {
     /// After the operation, the root of the cloned subtree has no parent.
     ///
     /// Any Ref properties that point to instances contained in the subtree are
-    /// rewritten to point to the cloned instances.
+    /// rewritten to point to the cloned instances. Any other Ref properties
+    /// would be invalid  in `dest` and are thus rewritten to be `Ref::none()`
     pub fn clone_into_external(&self, referent: Ref, dest: &mut WeakDom) -> Ref {
         let mut ctx = CloneContext::default();
         let root_builder = ctx.clone_ref_as_builder(self, referent);

From 2da1e4bd2059c8d860dbafaf9cc373cd88eb5945 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 7 Jul 2023 15:14:35 -0700
Subject: [PATCH 24/25] Add changelog entry

---
 rbx_dom_weak/CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/rbx_dom_weak/CHANGELOG.md b/rbx_dom_weak/CHANGELOG.md
index 6b0f6205..cbc85ff5 100644
--- a/rbx_dom_weak/CHANGELOG.md
+++ b/rbx_dom_weak/CHANGELOG.md
@@ -3,9 +3,11 @@
 ## Unreleased Changes
 * Fix potential stack overflow when creating or inserting into a `WeakDom`. ([#279])
 * Added `InstanceBuilder::has_property` for checking if an `InstanceBuilder` defines a given property. ([#283])
+* Added `WeakDom::clone_within` and `WeakDom::clone_into_external` for cloning instances into the same or a different `WeakDom`, respectively. ([#312])
 
 [#279]: https://github.com/rojo-rbx/rbx-dom/pull/279
 [#283]: https://github.com/rojo-rbx/rbx-dom/pull/283
+[#312]: https://github.com/rojo-rbx/rbx-dom/pull/312
 
 ## 2.4.0 (2022-06-05)
 * Added `WeakDom::into_raw` for enabling fast, non-tree-preserving transformations.

From 797954b1a128406b189f0d9f13390e7c8e29dc2e Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 7 Jul 2023 15:15:28 -0700
Subject: [PATCH 25/25] Fix typo in rewrite_refs comment

---
 rbx_dom_weak/src/dom.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rbx_dom_weak/src/dom.rs b/rbx_dom_weak/src/dom.rs
index fe30c581..0267dcc2 100644
--- a/rbx_dom_weak/src/dom.rs
+++ b/rbx_dom_weak/src/dom.rs
@@ -356,7 +356,7 @@ impl CloneContext {
             for prop_value in instance.properties.values_mut() {
                 if let Variant::Ref(original_ref) = prop_value {
                     if let Some(new_ref) = self.ref_rewrites.get(original_ref) {
-                        // If the ref points to an instances contained within the
+                        // If the ref points to an instance contained within the
                         // cloned subtree, rewrite it as the corresponding new ref
                         *prop_value = Variant::Ref(*new_ref);
                     } else if !existing_dest_refs.contains(original_ref) {