Skip to content

Commit

Permalink
Implement method for cloning multiple instances into external dom (#364)
Browse files Browse the repository at this point in the history
  • Loading branch information
kennethloeffler authored Sep 27, 2023
1 parent 9716af3 commit c0c7d5a
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 0 deletions.
3 changes: 3 additions & 0 deletions rbx_dom_weak/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# rbx_dom_weak Changelog

## Unreleased Changes
* Added `WeakDom::clone_multiple_into_external` that allows cloning multiple subtrees all at once into a given `WeakDom`, useful for preserving `Ref` properties that point across cloned subtrees ([#364])

[#364]: https://github.com/rojo-rbx/rbx-dom/pull/364

## 2.5.0 (2023-08-09)
* Fix potential stack overflow when creating or inserting into a `WeakDom`. ([#279])
Expand Down
63 changes: 63 additions & 0 deletions rbx_dom_weak/src/dom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,10 @@ impl WeakDom {
/// Any Ref properties that point to instances contained in the subtree are
/// rewritten to point to the cloned instances. Any other Ref properties
/// would be invalid in `dest` and are thus rewritten to be `Ref::none()`
///
/// This means that if you call this method on multiple different instances, Ref
/// properties will not necessarily be preserved in the destination dom. If you're
/// cloning multiple instances, prefer `clone_multiple_into_external` instead!
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);
Expand All @@ -278,6 +282,26 @@ impl WeakDom {
root_ref
}

/// Similar to `clone_into_external`, but clones multiple subtrees all at once. This
/// method will preserve Ref properties that point across the cloned subtrees.
pub fn clone_multiple_into_external(&self, referents: &[Ref], dest: &mut WeakDom) -> Vec<Ref> {
let mut ctx = CloneContext::default();
let mut root_refs = Vec::with_capacity(referents.len());

for referent in referents {
let builder = ctx.clone_ref_as_builder(self, *referent);
root_refs.push(dest.insert(Ref::none(), builder));
}

while let Some((cloned_parent, uncloned_child)) = ctx.queue.pop_front() {
let builder = ctx.clone_ref_as_builder(self, uncloned_child);
dest.insert(cloned_parent, builder);
}

ctx.rewrite_refs(dest);
root_refs
}

fn inner_insert(&mut self, referent: Ref, instance: Instance) {
self.instances.insert(referent, instance);

Expand Down Expand Up @@ -531,6 +555,45 @@ mod test {
insta::assert_yaml_snapshot!(viewer.view(&other_dom));
}

#[test]
fn clone_multiple_into_external() {
let dom = {
let mut child1 = InstanceBuilder::new("Part").with_name("Child1");
let mut child2 = InstanceBuilder::new("Part").with_name("Child2");

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 = dom.clone_multiple_into_external(dom.root().children(), &mut other_dom);

assert!(
other_dom.get_by_ref(cloned[0]).unwrap().parent.is_none(),
"parent of cloned subtree root should be none directly after a clone"
);

assert!(
other_dom.get_by_ref(cloned[1]).unwrap().parent.is_none(),
"parent of cloned subtree root should be none directly after a clone"
);

other_dom.transfer_within(cloned[0], other_dom.root_ref);
other_dom.transfer_within(cloned[1], other_dom.root_ref);

let mut viewer = DomViewer::new();

// This snapshot should contain Child1 and Child2, with Child1's and Child2's ref
// properties rewritten to point to the newly cloned instances
insta::assert_yaml_snapshot!(viewer.view(&other_dom));
}

#[test]
fn large_depth_tree() {
// We've had issues with stack overflows when creating WeakDoms with
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
source: rbx_dom_weak/src/dom.rs
expression: viewer.view(&other_dom)
---
referent: referent-0
name: DataModel
class: DataModel
properties: {}
children:
- referent: referent-1
name: Child1
class: Part
properties:
RefProp: referent-2
children: []
- referent: referent-2
name: Child2
class: Part
properties:
RefProp: referent-1
children: []

0 comments on commit c0c7d5a

Please sign in to comment.