From 7aa3f2195429b4388e6b1b0548e7d99a61044a66 Mon Sep 17 00:00:00 2001 From: Ege Gungordu Date: Sun, 21 Jan 2024 18:26:48 +0300 Subject: [PATCH 1/3] fix attribute diff infinite loop (#1843) --- packages/core/src/diff.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index a116aee4d..2f5b84f25 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -160,6 +160,10 @@ impl<'b> VirtualDom { self.diff_attribute(left, right, mounted_id) } } + + // consume both + left_iter.next(); + right_iter.next(); } (Some(_), None) => { let left = left_iter.next().unwrap(); From 6111bdfdf0f90e1b8b9e2abaf4852532f7fcf4b3 Mon Sep 17 00:00:00 2001 From: Ege Gungordu Date: Sun, 21 Jan 2024 18:29:29 +0300 Subject: [PATCH 2/3] add attribute diff test --- packages/core/tests/diff_element.rs | 110 +++++++++++++++++++++++++++- 1 file changed, 109 insertions(+), 1 deletion(-) diff --git a/packages/core/tests/diff_element.rs b/packages/core/tests/diff_element.rs index a0b9bfa84..e201dbb38 100644 --- a/packages/core/tests/diff_element.rs +++ b/packages/core/tests/diff_element.rs @@ -1,6 +1,6 @@ use dioxus::core::Mutation::*; use dioxus::prelude::*; -use dioxus_core::ElementId; +use dioxus_core::{AttributeValue, ElementId}; #[test] fn text_diff() { @@ -82,3 +82,111 @@ fn element_swap() { ] ); } + +#[test] +fn attribute_diff() { + fn app(cx: Scope) -> Element { + let gen = cx.generation(); + + let attrs = cx.bump().alloc(match gen % 2 { + 0 => vec![Attribute::new( + "attr1", + AttributeValue::Text("hello"), + None, + false, + )], + 1 => vec![ + Attribute::new("attr1", AttributeValue::Text("hello"), None, false), + Attribute::new("attr2", AttributeValue::Float(1.0), None, false), + Attribute::new("attr3", AttributeValue::Int(1), None, false), + Attribute::new("attr4", AttributeValue::Bool(true), None, false), + ], + _ => unreachable!(), + }); + + cx.render(rsx!( + div { + ..*attrs, + "hello" + } + )) + } + + let mut vdom = VirtualDom::new(app); + _ = vdom.rebuild(); + + vdom.mark_dirty(ScopeId::ROOT); + assert_eq!( + vdom.render_immediate().santize().edits, + [ + SetAttribute { + name: "attr2", + value: (&AttributeValue::Float(1.0)).into(), + id: ElementId(1), + ns: None + }, + SetAttribute { + name: "attr3", + value: (&AttributeValue::Int(1)).into(), + id: ElementId(1), + ns: None + }, + SetAttribute { + name: "attr4", + value: (&AttributeValue::Bool(true)).into(), + id: ElementId(1), + ns: None + }, + ] + ); + + vdom.mark_dirty(ScopeId::ROOT); + assert_eq!( + vdom.render_immediate().santize().edits, + [ + SetAttribute { + name: "attr2", + value: (&AttributeValue::None).into(), + id: ElementId(1), + ns: None + }, + SetAttribute { + name: "attr3", + value: (&AttributeValue::None).into(), + id: ElementId(1), + ns: None + }, + SetAttribute { + name: "attr4", + value: (&AttributeValue::None).into(), + id: ElementId(1), + ns: None + }, + ] + ); + + vdom.mark_dirty(ScopeId::ROOT); + assert_eq!( + vdom.render_immediate().santize().edits, + [ + SetAttribute { + name: "attr2", + value: (&AttributeValue::Float(1.0)).into(), + id: ElementId(1), + ns: None + }, + SetAttribute { + name: "attr3", + value: (&AttributeValue::Int(1)).into(), + id: ElementId(1), + ns: None + }, + SetAttribute { + name: "attr4", + value: (&AttributeValue::Bool(true)).into(), + id: ElementId(1), + ns: None + }, + ] + ); +} From 43fe17d9c53ea9f71537c2c85cb72ab71d623890 Mon Sep 17 00:00:00 2001 From: Ege Gungordu Date: Mon, 22 Jan 2024 00:36:44 +0300 Subject: [PATCH 3/3] fix attribute diffs causing infinite loops with test --- packages/core/src/diff.rs | 36 +++++----- packages/core/tests/diff_element.rs | 105 +++++++++++++++------------- 2 files changed, 78 insertions(+), 63 deletions(-) diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index 2f5b84f25..b5f262b0c 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -145,25 +145,29 @@ impl<'b> VirtualDom { (Some(left), Some(right)) => { // check which name is greater match left.name.cmp(right.name) { - std::cmp::Ordering::Less => self.remove_attribute( - left.name, - left.namespace, - mounted_id, - ), - std::cmp::Ordering::Greater => self.write_attribute( - right_template, - right, - idx, - mounted_id, - ), + std::cmp::Ordering::Less => { + self.remove_attribute( + left.name, + left.namespace, + mounted_id, + ); + left_iter.next(); + } + std::cmp::Ordering::Greater => { + self.write_attribute( + right_template, + right, + idx, + mounted_id, + ); + right_iter.next(); + } std::cmp::Ordering::Equal => { - self.diff_attribute(left, right, mounted_id) + self.diff_attribute(left, right, mounted_id); + left_iter.next(); + right_iter.next(); } } - - // consume both - left_iter.next(); - right_iter.next(); } (Some(_), None) => { let left = left_iter.next().unwrap(); diff --git a/packages/core/tests/diff_element.rs b/packages/core/tests/diff_element.rs index e201dbb38..7169de609 100644 --- a/packages/core/tests/diff_element.rs +++ b/packages/core/tests/diff_element.rs @@ -88,21 +88,32 @@ fn attribute_diff() { fn app(cx: Scope) -> Element { let gen = cx.generation(); - let attrs = cx.bump().alloc(match gen % 2 { - 0 => vec![Attribute::new( - "attr1", + // attributes have to be sorted by name + let attrs = match gen % 5 { + 0 => cx.bump().alloc([Attribute::new( + "a", AttributeValue::Text("hello"), None, false, - )], - 1 => vec![ - Attribute::new("attr1", AttributeValue::Text("hello"), None, false), - Attribute::new("attr2", AttributeValue::Float(1.0), None, false), - Attribute::new("attr3", AttributeValue::Int(1), None, false), - Attribute::new("attr4", AttributeValue::Bool(true), None, false), - ], + )]) as &[Attribute], + 1 => cx.bump().alloc([ + Attribute::new("a", AttributeValue::Text("hello"), None, false), + Attribute::new("b", AttributeValue::Text("hello"), None, false), + Attribute::new("c", AttributeValue::Text("hello"), None, false), + ]) as &[Attribute], + 2 => cx.bump().alloc([ + Attribute::new("c", AttributeValue::Text("hello"), None, false), + Attribute::new("d", AttributeValue::Text("hello"), None, false), + Attribute::new("e", AttributeValue::Text("hello"), None, false), + ]) as &[Attribute], + 3 => cx.bump().alloc([Attribute::new( + "d", + AttributeValue::Text("world"), + None, + false, + )]) as &[Attribute], _ => unreachable!(), - }); + }; cx.render(rsx!( div { @@ -120,22 +131,16 @@ fn attribute_diff() { vdom.render_immediate().santize().edits, [ SetAttribute { - name: "attr2", - value: (&AttributeValue::Float(1.0)).into(), - id: ElementId(1), - ns: None + name: "b", + value: (&AttributeValue::Text("hello",)).into(), + id: ElementId(1,), + ns: None, }, SetAttribute { - name: "attr3", - value: (&AttributeValue::Int(1)).into(), - id: ElementId(1), - ns: None - }, - SetAttribute { - name: "attr4", - value: (&AttributeValue::Bool(true)).into(), - id: ElementId(1), - ns: None + name: "c", + value: (&AttributeValue::Text("hello",)).into(), + id: ElementId(1,), + ns: None, }, ] ); @@ -145,22 +150,28 @@ fn attribute_diff() { vdom.render_immediate().santize().edits, [ SetAttribute { - name: "attr2", + name: "a", value: (&AttributeValue::None).into(), - id: ElementId(1), - ns: None + id: ElementId(1,), + ns: None, }, SetAttribute { - name: "attr3", + name: "b", value: (&AttributeValue::None).into(), - id: ElementId(1), - ns: None + id: ElementId(1,), + ns: None, }, SetAttribute { - name: "attr4", - value: (&AttributeValue::None).into(), - id: ElementId(1), - ns: None + name: "d", + value: (&AttributeValue::Text("hello",)).into(), + id: ElementId(1,), + ns: None, + }, + SetAttribute { + name: "e", + value: (&AttributeValue::Text("hello",)).into(), + id: ElementId(1,), + ns: None, }, ] ); @@ -170,22 +181,22 @@ fn attribute_diff() { vdom.render_immediate().santize().edits, [ SetAttribute { - name: "attr2", - value: (&AttributeValue::Float(1.0)).into(), - id: ElementId(1), - ns: None + name: "c", + value: (&AttributeValue::None).into(), + id: ElementId(1,), + ns: None, }, SetAttribute { - name: "attr3", - value: (&AttributeValue::Int(1)).into(), - id: ElementId(1), - ns: None + name: "d", + value: (&AttributeValue::Text("world",)).into(), + id: ElementId(1,), + ns: None, }, SetAttribute { - name: "attr4", - value: (&AttributeValue::Bool(true)).into(), - id: ElementId(1), - ns: None + name: "e", + value: (&AttributeValue::None).into(), + id: ElementId(1,), + ns: None, }, ] );