Skip to content

Commit

Permalink
Fix Keyed iteration again (#75)
Browse files Browse the repository at this point in the history 
* Fix

* Remove fmt::Debug bound for debugging

* Add nested reactivity test
  • Loading branch information
@lukechu10
lukechu10 authored Apr 1, 2021
1 parent ffab9c3 commit 5b99fb5
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 65 deletions.
149 changes: 84 additions & 65 deletions maple-core/src/flow.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ pub fn Keyed<T, F: 'static, K: 'static, Key: 'static>(
where
F: Fn(T) -> TemplateResult,
K: Fn(&T) -> Key,
Key: Clone + Hash + Eq + std::fmt::Debug,
T: Clone + PartialEq + std::fmt::Debug,
Key: Clone + Hash + Eq,
T: Clone + PartialEq,
{
let KeyedProps {
iterable,
Expand Down Expand Up @@ -92,11 +92,24 @@ where
let templates = Rc::clone(&templates);
let marker = marker.clone();
move || {
let previous_values: HashMap<_, _> = {
struct PreviousData<T> {
value: T,
index: usize,
}

let previous_values: HashMap<_, PreviousData<T>> = {
let templates = templates.borrow();
templates
.iter()
.map(|x| ((*x.0).clone(), (x.1 .1.clone(), x.1 .3)))
.map(|x| {
(
(*x.0).clone(),
PreviousData {
value: x.1 .1.clone(),
index: x.1 .3,
},
)
})
.collect()
};

Expand All @@ -106,87 +119,93 @@ where

let previous_value = previous_values.get(&key);

if previous_value.is_none()
|| previous_value.unwrap().1 != i
|| &previous_value.unwrap().0 != item
{
// value changed, re-render item

templates
.borrow_mut()
.get_mut(&key)
.and_then(|(owner, _, _, _)| {
// destroy old owner
let old_owner =
mem::replace(owner, Owner::new() /* placeholder */);
drop(old_owner);
None::<()>
});
if previous_value.is_none() {
// Create new DOM node.

let mut new_template = None;
let owner = create_root(|| new_template = Some(template(item.clone())));

if templates.borrow().get(&key).is_some() && &previous_value.unwrap().0 != item
{
// Value changed. Re-render node (with same previous key).
let (_, _, old_node, _) = mem::replace(
templates.borrow_mut().get_mut(&key).unwrap(),
(owner, item.clone(), new_template.clone().unwrap(), i),
);
templates.borrow_mut().insert(
key.clone(),
(owner, item.clone(), new_template.clone().unwrap(), i),
);

let parent = old_node.node.parent_node().unwrap();
parent
.replace_child(&new_template.unwrap().node, &old_node.node)
.unwrap();
} else if templates.borrow().get(&key).is_some()
&& templates.borrow().get(&key).unwrap().3 != i
{
// Node was moved in the DOM. Move node to new index.
let node = templates.borrow().get(&key).unwrap().2.node.clone();

if let Some(next_item) = iterable.get().get(i + 1) {
let templates = templates.borrow();
let next_node = templates.get(&key_fn(next_item)).unwrap();
if let Some(next_item) = iterable.get().get(i + 1) {
let templates = templates.borrow();
if let Some(next_node) = templates.get(&key_fn(next_item)) {
next_node
.2
.node
.unchecked_ref::<HtmlElement>()
.before_with_node_1(&node)
.before_with_node_1(&new_template.unwrap().node)
.unwrap();
} else {
marker.before_with_node_1(&node).unwrap(); // Move to end.
}
} else {
// Create new DOM node.
templates.borrow_mut().insert(
key.clone(),
(owner, item.clone(), new_template.clone().unwrap(), i),
);

if let Some(next_item) = iterable.get().get(i + 1) {
let templates = templates.borrow();
if let Some(next_node) = templates.get(&key_fn(next_item)) {
next_node
.2
.node
.unchecked_ref::<HtmlElement>()
.before_with_node_1(&new_template.unwrap().node)
.unwrap();
} else {
marker
.before_with_node_1(&new_template.unwrap().node)
.unwrap();
}
} else {
marker
.before_with_node_1(&new_template.unwrap().node)
.unwrap();
}
} else {
marker
.before_with_node_1(&new_template.unwrap().node)
.unwrap();
}
} else if match previous_value {
Some(prev) => prev.index,
_ => unreachable!(),
} != i
{
// Location changed, move from old location to new location
// Node was moved in the DOM. Move node to new index.

let node = templates.borrow().get(&key).unwrap().2.node.clone();

if let Some(next_item) = iterable.get().get(i + 1) {
let templates = templates.borrow();
let next_node = templates.get(&key_fn(next_item)).unwrap();
next_node
.2
.node
.unchecked_ref::<HtmlElement>()
.before_with_node_1(&node)
.unwrap(); // Move to before next node
} else {
marker.before_with_node_1(&node).unwrap(); // Move to end.
}

templates.borrow_mut().get_mut(&key).unwrap().3 = i;
} else if match previous_value {
Some(prev) => &prev.value,
_ => unreachable!(),
} != item
{
// Value changed. Re-render node (with same previous key and index).

// Destroy old template owner.
let mut templates = templates.borrow_mut();
let (old_owner, _, _, _) = templates
.get_mut(&key)
.expect("previous value is different but must be valid");
let old_owner = mem::replace(old_owner, Owner::new() /* placeholder */);
drop(old_owner);

let mut new_template = None;
let owner = create_root(|| new_template = Some(template(item.clone())));

let (_, _, old_node, _) = mem::replace(
templates.get_mut(&key).unwrap(),
(owner, item.clone(), new_template.clone().unwrap(), i),
);

let parent = old_node.node.parent_node().unwrap();
parent
.replace_child(&new_template.unwrap().node, &old_node.node)
.unwrap();
}
}

if templates.borrow().len() > iterable.get().len() {
// remove extra templates

let mut templates = templates.borrow_mut();
let new_keys: HashSet<Key> =
iterable.get().iter().map(|item| key_fn(item)).collect();
Expand Down
39 changes: 39 additions & 0 deletions maple-core/tests/integration/keyed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ fn swap_rows() {
tmp
});
assert_eq!(p.text_content().unwrap(), "321");

count.set({
let mut tmp = (*count.get()).clone();
tmp.swap(0, 2);
tmp
});
assert_eq!(p.text_content().unwrap(), "123");
}

#[wasm_bindgen_test]
Expand Down Expand Up @@ -90,3 +97,35 @@ fn insert_front() {
});
assert_eq!(p.text_content().unwrap(), "4123");
}

#[wasm_bindgen_test]
fn nested_reactivity() {
let count = Signal::new(vec![1, 2, 3].into_iter().map(Signal::new).collect());

let node = cloned!((count) => template! {
ul {
Keyed(KeyedProps {
iterable: count.handle(),
template: |item| template! {
li { (item.get()) }
},
key: |item| *item.get(),
})
}
});

render_to(|| node, &test_div());

let p = document().query_selector("ul").unwrap().unwrap();
assert_eq!(p.text_content().unwrap(), "123");

count.get()[0].set(4);
assert_eq!(p.text_content().unwrap(), "423");

count.set({
let mut tmp = (*count.get()).clone();
tmp.push(Signal::new(5));
tmp
});
assert_eq!(p.text_content().unwrap(), "4235");
}
38 changes: 38 additions & 0 deletions maple-core/tests/integration/non_keyed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ fn swap_rows() {
tmp
});
assert_eq!(p.text_content().unwrap(), "321");

count.set({
let mut tmp = (*count.get()).clone();
tmp.swap(0, 2);
tmp
});
assert_eq!(p.text_content().unwrap(), "123");
}

#[wasm_bindgen_test]
Expand Down Expand Up @@ -87,3 +94,34 @@ fn insert_front() {
});
assert_eq!(p.text_content().unwrap(), "4123");
}

#[wasm_bindgen_test]
fn nested_reactivity() {
let count = Signal::new(vec![1, 2, 3].into_iter().map(Signal::new).collect());

let node = cloned!((count) => template! {
ul {
Indexed(IndexedProps {
iterable: count.handle(),
template: |item| template! {
li { (item.get()) }
},
})
}
});

render_to(|| node, &test_div());

let p = document().query_selector("ul").unwrap().unwrap();
assert_eq!(p.text_content().unwrap(), "123");

count.get()[0].set(4);
assert_eq!(p.text_content().unwrap(), "423");

count.set({
let mut tmp = (*count.get()).clone();
tmp.push(Signal::new(5));
tmp
});
assert_eq!(p.text_content().unwrap(), "4235");
}

0 comments on commit 5b99fb5

Please sign in to comment.