Skip to content

Commit

Permalink
Attributes: Fix apply_diff_index_maps (#2653)
Browse files Browse the repository at this point in the history
* Attributes: Fix apply_diff_index_maps

The old algorithm simply stops when key name/order changes, which
is simply wrong.

Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>

* add test case for test for bug #2653
  • Loading branch information
maurerdietmar committed May 5, 2022
1 parent fd1906f commit e588013
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 38 deletions.
50 changes: 12 additions & 38 deletions packages/yew/src/dom_bundle/btag/attributes.rs
@@ -1,5 +1,4 @@
use std::collections::HashMap;
use std::iter;
use std::ops::Deref;

use indexmap::IndexMap;
Expand Down Expand Up @@ -91,45 +90,20 @@ impl Attributes {
new: &IndexMap<AttrValue, AttrValue>,
old: &IndexMap<AttrValue, AttrValue>,
) {
let mut old_iter = old.iter();
let mut new_iter = new.iter();
loop {
match (new_iter.next(), old_iter.next()) {
(Some((new_key, new_value)), Some((old_key, old_value))) => {
if new_key != old_key {
break;
for (key, value) in new.iter() {
match old.get(key) {
Some(old_value) => {
if value != old_value {
Self::set_attribute(el, key, value);
}
if new_value != old_value {
Self::set_attribute(el, new_key, new_value);
}
}
// new attributes
(Some(attr), None) => {
for (key, value) in iter::once(attr).chain(new_iter) {
match old.get(key) {
Some(old_value) => {
if value != old_value {
Self::set_attribute(el, key, value);
}
}
None => {
Self::set_attribute(el, key, value);
}
}
}
break;
}
// removed attributes
(None, Some(attr)) => {
for (key, _) in iter::once(attr).chain(old_iter) {
let key = key;
if !new.contains_key(key) {
Self::remove_attribute(el, key);
}
}
break;
}
(None, None) => break,
None => Self::set_attribute(el, key, value),
}
}

for (key, _value) in old.iter() {
if !new.contains_key(key) {
Self::remove_attribute(el, key);
}
}
}
Expand Down
42 changes: 42 additions & 0 deletions packages/yew/src/dom_bundle/btag/mod.rs
Expand Up @@ -931,6 +931,48 @@ mod tests {
"<div id=\"after\"></div>"
);
}

// test for bug: https://github.com/yewstack/yew/pull/2653
#[test]
fn test_index_map_attribute_diff() {
let (root, scope, parent) = setup_parent();

let test_ref = NodeRef::default();

// We want to test appy_diff with Attributes::IndexMap, so we
// need to create the VTag manually

// Create <div disabled="disabled" tabindex="0">
let mut vtag = VTag::new("div");
vtag.node_ref = test_ref.clone();
vtag.add_attribute("disabled", "disabled");
vtag.add_attribute("tabindex", "0");

let elem = VNode::VTag(Box::new(vtag));

let (_, mut elem) = elem.attach(&root, &scope, &parent, NodeRef::default());

// Create <div tabindex="0"> (removed first attribute "disabled")
let mut vtag = VTag::new("div");
vtag.node_ref = test_ref.clone();
vtag.add_attribute("tabindex", "0");
let next_elem = VNode::VTag(Box::new(vtag));
let elem_vtag = assert_vtag(next_elem);

// Sync happens here
// this should remove the the "disabled" attribute
elem_vtag.reconcile_node(&root, &scope, &parent, NodeRef::default(), &mut elem);

assert_eq!(
test_ref
.get()
.unwrap()
.dyn_ref::<web_sys::Element>()
.unwrap()
.outer_html(),
"<div tabindex=\"0\"></div>"
)
}
}

#[cfg(all(test, feature = "wasm_test"))]
Expand Down

1 comment on commit e588013

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yew master branch benchmarks (Lower is better)

Benchmark suite Current: e588013 Previous: fd1906f Ratio
yew-struct-keyed 01_run1k 174.8715 158.5575 1.10
yew-struct-keyed 02_replace1k 182.658 170.13049999999998 1.07
yew-struct-keyed 03_update10th1k_x16 432.025 278.2025 1.55
yew-struct-keyed 04_select1k 76.96600000000001 50.2155 1.53
yew-struct-keyed 05_swap1k 101.127 69.78200000000001 1.45
yew-struct-keyed 06_remove-one-1k 30.807 25.3115 1.22
yew-struct-keyed 07_create10k 3024.2375 2653.8785 1.14
yew-struct-keyed 08_create1k-after1k_x2 422.8645 365.9105 1.16
yew-struct-keyed 09_clear1k_x8 190.7245 147.3945 1.29
yew-struct-keyed 21_ready-memory 1.459808349609375 1.457233428955078 1.00
yew-struct-keyed 22_run-memory 1.6642074584960938 1.6938209533691406 0.98
yew-struct-keyed 23_update5-memory 1.6686172485351562 1.6972122192382812 0.98
yew-struct-keyed 24_run5-memory 1.713031768798828 1.7057380676269531 1.00
yew-struct-keyed 25_run-clear-memory 1.3325920104980469 1.3268890380859375 1.00
yew-struct-keyed 31_startup-ci 1730.42 1729.632 1.00
yew-struct-keyed 32_startup-bt 28.792 24.028 1.20
yew-struct-keyed 33_startup-mainthreadcost 216.704 190.556 1.14
yew-struct-keyed 34_startup-totalbytes 328.7412109375 328.7412109375 1

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.