From 50afc9d36c9c74db3d08ded2e77969671f7bf38c Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Mon, 19 Dec 2022 19:24:01 +0900 Subject: [PATCH 01/14] Make VList's children Rc'ed. --- packages/yew/src/dom_bundle/blist.rs | 18 ++++++++-- packages/yew/src/virtual_dom/vlist.rs | 51 +++++++++++++++++++++------ 2 files changed, 55 insertions(+), 14 deletions(-) diff --git a/packages/yew/src/dom_bundle/blist.rs b/packages/yew/src/dom_bundle/blist.rs index 40908699571..4d125fd8d46 100644 --- a/packages/yew/src/dom_bundle/blist.rs +++ b/packages/yew/src/dom_bundle/blist.rs @@ -4,6 +4,7 @@ use std::cmp::Ordering; use std::collections::HashSet; use std::hash::Hash; use std::ops::Deref; +use std::rc::Rc; use web_sys::Element; @@ -22,6 +23,17 @@ pub(super) struct BList { key: Option, } +impl BList { + // Unwraps a Vec from the children of a VList. + fn unwrap_children(children: Option>>) -> Vec { + children + .map(Rc::try_unwrap) + .unwrap_or_else(|| Ok(Vec::new())) + // Rc::unwrap_or_clone is not stable yet. + .unwrap_or_else(|m| m.to_vec()) + } +} + impl Deref for BList { type Target = Vec; @@ -437,7 +449,7 @@ impl Reconcilable for VList { // (self.children). For the right ones, we will look at the bundle, // i.e. the current DOM list element that we want to replace with self. - if self.children.is_empty() { + if self.is_empty() { // Without a placeholder the next element becomes first // and corrupts the order of rendering // We use empty text element to stake out a place @@ -445,7 +457,7 @@ impl Reconcilable for VList { } let fully_keyed = self.fully_keyed(); - let lefts = self.children; + let lefts = BList::unwrap_children(self.children); let rights = &mut blist.rev_children; test_log!("lefts: {:?}", lefts); test_log!("rights: {:?}", rights); @@ -480,7 +492,7 @@ mod feat_hydration { ) -> (NodeRef, Self::Bundle) { let node_ref = NodeRef::default(); let fully_keyed = self.fully_keyed(); - let vchildren = self.children; + let vchildren = BList::unwrap_children(self.children); let mut children = Vec::with_capacity(vchildren.len()); for (index, child) in vchildren.into_iter().enumerate() { diff --git a/packages/yew/src/virtual_dom/vlist.rs b/packages/yew/src/virtual_dom/vlist.rs index 0dc2e25f00a..c205226a9ac 100644 --- a/packages/yew/src/virtual_dom/vlist.rs +++ b/packages/yew/src/virtual_dom/vlist.rs @@ -1,5 +1,6 @@ //! This module contains fragments implementation. use std::ops::{Deref, DerefMut}; +use std::rc::Rc; use super::{Key, VNode}; @@ -14,7 +15,7 @@ enum FullyKeyedState { #[derive(Clone, Debug)] pub struct VList { /// The list of child [VNode]s - pub(crate) children: Vec, + pub(crate) children: Option>>, /// All [VNode]s in the VList have keys fully_keyed: FullyKeyedState, @@ -24,7 +25,15 @@ pub struct VList { impl PartialEq for VList { fn eq(&self, other: &Self) -> bool { - self.children == other.children && self.key == other.key + self.key == other.key + && match (self.children.as_ref(), other.children.as_ref()) { + // We try to use ptr_eq if both are behind Rc, + // Somehow VNode is not Eq? + (Some(l), Some(r)) if Rc::ptr_eq(l, r) => true, + // We fallback to PartialEq if left and right didn't point to the same memory + // address. + (l, r) => l == r, + } } } @@ -38,14 +47,22 @@ impl Deref for VList { type Target = Vec; fn deref(&self) -> &Self::Target { - &self.children + match self.children { + Some(ref m) => m, + None => { + // This is mutable because the Vec is not Sync + static mut EMPTY: Vec = Vec::new(); + // SAFETY: The EMPTY value is always read-only + unsafe { &EMPTY } + } + } } } impl DerefMut for VList { fn deref_mut(&mut self) -> &mut Self::Target { self.fully_keyed = FullyKeyedState::Unknown; - &mut self.children + self.children_mut() } } @@ -53,7 +70,7 @@ impl VList { /// Creates a new empty [VList] instance. pub const fn new() -> Self { Self { - children: Vec::new(), + children: None, key: None, fully_keyed: FullyKeyedState::KnownFullyKeyed, } @@ -63,7 +80,7 @@ impl VList { pub fn with_children(children: Vec, key: Option) -> Self { let mut vlist = VList { fully_keyed: FullyKeyedState::Unknown, - children, + children: Some(Rc::new(children)), key, }; vlist.fully_keyed = if vlist.fully_keyed() { @@ -74,19 +91,31 @@ impl VList { vlist } + #[inline(never)] + pub fn children_mut(&mut self) -> &mut Vec { + loop { + match self.children { + Some(ref mut m) => return Rc::make_mut(m), + None => { + self.children = Some(Rc::new(Vec::new())); + } + } + } + } + /// Add [VNode] child. pub fn add_child(&mut self, child: VNode) { if self.fully_keyed == FullyKeyedState::KnownFullyKeyed && !child.has_key() { self.fully_keyed = FullyKeyedState::KnownMissingKeys; } - self.children.push(child); + self.children_mut().push(child); } /// Add multiple [VNode] children. pub fn add_children(&mut self, children: impl IntoIterator) { let it = children.into_iter(); let bound = it.size_hint(); - self.children.reserve(bound.1.unwrap_or(bound.0)); + self.children_mut().reserve(bound.1.unwrap_or(bound.0)); for ch in it { self.add_child(ch); } @@ -108,7 +137,7 @@ impl VList { match self.fully_keyed { FullyKeyedState::KnownFullyKeyed => true, FullyKeyedState::KnownMissingKeys => false, - FullyKeyedState::Unknown => self.children.iter().all(|c| c.has_key()), + FullyKeyedState::Unknown => self.iter().all(|c| c.has_key()), } } } @@ -173,7 +202,7 @@ mod feat_ssr { parent_scope: &AnyScope, hydratable: bool, ) { - match &self.children[..] { + match &self[..] { [] => {} [child] => { child.render_into_stream(w, parent_scope, hydratable).await; @@ -237,7 +266,7 @@ mod feat_ssr { } } - let children = self.children.iter(); + let children = self.iter(); render_child_iter(children, w, parent_scope, hydratable).await; } } From 71cf2a1d36f1a99463c68aae3d128aa3be93dc03 Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Mon, 19 Dec 2022 22:07:05 +0900 Subject: [PATCH 02/14] Slightly optimise for bundle size. --- packages/yew/src/dom_bundle/blist.rs | 56 ++++++++++++++++----------- packages/yew/src/virtual_dom/vlist.rs | 11 +++--- 2 files changed, 40 insertions(+), 27 deletions(-) diff --git a/packages/yew/src/dom_bundle/blist.rs b/packages/yew/src/dom_bundle/blist.rs index 4d125fd8d46..e15ebf4bf83 100644 --- a/packages/yew/src/dom_bundle/blist.rs +++ b/packages/yew/src/dom_bundle/blist.rs @@ -23,14 +23,34 @@ pub(super) struct BList { key: Option, } -impl BList { - // Unwraps a Vec from the children of a VList. - fn unwrap_children(children: Option>>) -> Vec { - children - .map(Rc::try_unwrap) - .unwrap_or_else(|| Ok(Vec::new())) - // Rc::unwrap_or_clone is not stable yet. - .unwrap_or_else(|m| m.to_vec()) +impl VList { + /// Splits current VList for BList + fn split_for_bundle(self) -> (Option, bool, Vec) { + let mut fully_keyed = self.fully_keyed(); + + let mut children = match self.children { + None => Vec::with_capacity(1), + Some(m) => match Rc::try_unwrap(m) { + Ok(m) => m, + Err(e) => { + let mut children = Vec::with_capacity(e.len()); + for i in e.as_ref() { + children.push(i.clone()); + } + children + } + }, + }; + + if children.is_empty() { + // Without a placeholder the next element becomes first + // and corrupts the order of rendering + // We use empty text element to stake out a place + children.push(VNode::VText(VText::new(""))); + fully_keyed = false; + } + + (self.key, fully_keyed, children) } } @@ -433,7 +453,7 @@ impl Reconcilable for VList { } fn reconcile( - mut self, + self, root: &BSubtree, parent_scope: &AnyScope, parent: &Element, @@ -448,16 +468,8 @@ impl Reconcilable for VList { // The left items are known since we want to insert them // (self.children). For the right ones, we will look at the bundle, // i.e. the current DOM list element that we want to replace with self. + let (key, fully_keyed, lefts) = self.split_for_bundle(); - if self.is_empty() { - // Without a placeholder the next element becomes first - // and corrupts the order of rendering - // We use empty text element to stake out a place - self.add_child(VText::new("").into()); - } - - let fully_keyed = self.fully_keyed(); - let lefts = BList::unwrap_children(self.children); let rights = &mut blist.rev_children; test_log!("lefts: {:?}", lefts); test_log!("rights: {:?}", rights); @@ -471,7 +483,7 @@ impl Reconcilable for VList { BList::apply_unkeyed(root, parent_scope, parent, next_sibling, lefts, rights) }; blist.fully_keyed = fully_keyed; - blist.key = self.key; + blist.key = key; test_log!("result: {:?}", rights); first } @@ -491,8 +503,8 @@ mod feat_hydration { fragment: &mut Fragment, ) -> (NodeRef, Self::Bundle) { let node_ref = NodeRef::default(); - let fully_keyed = self.fully_keyed(); - let vchildren = BList::unwrap_children(self.children); + + let (key, fully_keyed, vchildren) = self.split_for_bundle(); let mut children = Vec::with_capacity(vchildren.len()); for (index, child) in vchildren.into_iter().enumerate() { @@ -512,7 +524,7 @@ mod feat_hydration { BList { rev_children: children, fully_keyed, - key: self.key, + key, }, ) } diff --git a/packages/yew/src/virtual_dom/vlist.rs b/packages/yew/src/virtual_dom/vlist.rs index c205226a9ac..44e73c3ca02 100644 --- a/packages/yew/src/virtual_dom/vlist.rs +++ b/packages/yew/src/virtual_dom/vlist.rs @@ -21,6 +21,9 @@ pub struct VList { fully_keyed: FullyKeyedState, pub key: Option, + + /// A padding to reduce bundle size + _padding: u64, } impl PartialEq for VList { @@ -73,6 +76,7 @@ impl VList { children: None, key: None, fully_keyed: FullyKeyedState::KnownFullyKeyed, + _padding: 0, } } @@ -82,12 +86,9 @@ impl VList { fully_keyed: FullyKeyedState::Unknown, children: Some(Rc::new(children)), key, + _padding: 0, }; - vlist.fully_keyed = if vlist.fully_keyed() { - FullyKeyedState::KnownFullyKeyed - } else { - FullyKeyedState::KnownMissingKeys - }; + vlist.recheck_fully_keyed(); vlist } From 1f737f26f85a13e49186df71eb41425387f3aa02 Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Mon, 19 Dec 2022 22:29:10 +0900 Subject: [PATCH 03/14] Revert manual cloning. --- packages/yew/src/dom_bundle/blist.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/yew/src/dom_bundle/blist.rs b/packages/yew/src/dom_bundle/blist.rs index e15ebf4bf83..266eb824beb 100644 --- a/packages/yew/src/dom_bundle/blist.rs +++ b/packages/yew/src/dom_bundle/blist.rs @@ -29,16 +29,10 @@ impl VList { let mut fully_keyed = self.fully_keyed(); let mut children = match self.children { - None => Vec::with_capacity(1), + None => Vec::new(), Some(m) => match Rc::try_unwrap(m) { Ok(m) => m, - Err(e) => { - let mut children = Vec::with_capacity(e.len()); - for i in e.as_ref() { - children.push(i.clone()); - } - children - } + Err(e) => e.as_ref().clone(), }, }; From f372e00d1807e0483985fc35cd724f13668b6b6e Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Mon, 19 Dec 2022 22:57:07 +0900 Subject: [PATCH 04/14] Add a benchmark. --- tools/benchmark-ssr/src/main.rs | 62 ++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/tools/benchmark-ssr/src/main.rs b/tools/benchmark-ssr/src/main.rs index b0f5eb29bb7..b0f24e3ed56 100644 --- a/tools/benchmark-ssr/src/main.rs +++ b/tools/benchmark-ssr/src/main.rs @@ -82,6 +82,56 @@ async fn bench_router_app() -> Duration { start_time.elapsed() } +async fn bench_many_providers() -> Duration { + static TOTAL: usize = 250_000; + + #[derive(Properties, PartialEq, Clone)] + struct ProviderProps { + children: Children, + } + + #[function_component] + fn Provider(props: &ProviderProps) -> Html { + let ProviderProps { children } = props.clone(); + + html! {
{children}
} + } + + #[function_component] + fn App() -> Html { + // Let's make 10 providers. + html! { + + + + + + + + + + {"Hello, World!"} + + + + + + + + + + } + } + + let start_time = Instant::now(); + + for _ in 0..TOTAL { + yew::LocalServerRenderer::::new().render().await; + } + + start_time.elapsed() +} + async fn bench_concurrent_task() -> Duration { static TOTAL: usize = 100; @@ -215,12 +265,13 @@ async fn main() { let args = Args::parse(); // Tests in each round. - static TESTS: usize = 4; + static TESTS: usize = 5; let mut baseline_results = Vec::with_capacity(args.rounds); let mut hello_world_results = Vec::with_capacity(args.rounds); let mut function_router_results = Vec::with_capacity(args.rounds); let mut concurrent_tasks_results = Vec::with_capacity(args.rounds); + let mut many_provider_results = Vec::with_capacity(args.rounds); let bar = (!args.no_term).then(|| create_progress(TESTS, args.rounds)); @@ -267,6 +318,14 @@ async fn main() { bar.inc(1); } } + + let dur = bench_many_providers().await; + if i > 0 { + many_provider_results.push(dur); + if let Some(ref bar) = bar { + bar.inc(1); + } + } } }) .await; @@ -281,6 +340,7 @@ async fn main() { Statistics::from_results("Hello World", args.rounds, hello_world_results), Statistics::from_results("Function Router", args.rounds, function_router_results), Statistics::from_results("Concurrent Task", args.rounds, concurrent_tasks_results), + Statistics::from_results("Many Providers", args.rounds, many_provider_results), ]; println!("{}", output.as_ref().table().with(Style::rounded())); From e7783c89cddc3a9e7bc479f75bfc82f9b66aebe8 Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Mon, 19 Dec 2022 23:21:47 +0900 Subject: [PATCH 05/14] Revert "Revert manual cloning." This reverts commit 1f737f26f85a13e49186df71eb41425387f3aa02. --- packages/yew/src/dom_bundle/blist.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/yew/src/dom_bundle/blist.rs b/packages/yew/src/dom_bundle/blist.rs index 266eb824beb..e15ebf4bf83 100644 --- a/packages/yew/src/dom_bundle/blist.rs +++ b/packages/yew/src/dom_bundle/blist.rs @@ -29,10 +29,16 @@ impl VList { let mut fully_keyed = self.fully_keyed(); let mut children = match self.children { - None => Vec::new(), + None => Vec::with_capacity(1), Some(m) => match Rc::try_unwrap(m) { Ok(m) => m, - Err(e) => e.as_ref().clone(), + Err(e) => { + let mut children = Vec::with_capacity(e.len()); + for i in e.as_ref() { + children.push(i.clone()); + } + children + } }, }; From ac1d9fdac06ddc6b225e060605b2bb3c8a1b8847 Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Mon, 19 Dec 2022 23:21:55 +0900 Subject: [PATCH 06/14] Revert "Slightly optimise for bundle size." This reverts commit 71cf2a1d36f1a99463c68aae3d128aa3be93dc03. --- packages/yew/src/dom_bundle/blist.rs | 56 +++++++++++---------------- packages/yew/src/virtual_dom/vlist.rs | 11 +++--- 2 files changed, 27 insertions(+), 40 deletions(-) diff --git a/packages/yew/src/dom_bundle/blist.rs b/packages/yew/src/dom_bundle/blist.rs index e15ebf4bf83..4d125fd8d46 100644 --- a/packages/yew/src/dom_bundle/blist.rs +++ b/packages/yew/src/dom_bundle/blist.rs @@ -23,34 +23,14 @@ pub(super) struct BList { key: Option, } -impl VList { - /// Splits current VList for BList - fn split_for_bundle(self) -> (Option, bool, Vec) { - let mut fully_keyed = self.fully_keyed(); - - let mut children = match self.children { - None => Vec::with_capacity(1), - Some(m) => match Rc::try_unwrap(m) { - Ok(m) => m, - Err(e) => { - let mut children = Vec::with_capacity(e.len()); - for i in e.as_ref() { - children.push(i.clone()); - } - children - } - }, - }; - - if children.is_empty() { - // Without a placeholder the next element becomes first - // and corrupts the order of rendering - // We use empty text element to stake out a place - children.push(VNode::VText(VText::new(""))); - fully_keyed = false; - } - - (self.key, fully_keyed, children) +impl BList { + // Unwraps a Vec from the children of a VList. + fn unwrap_children(children: Option>>) -> Vec { + children + .map(Rc::try_unwrap) + .unwrap_or_else(|| Ok(Vec::new())) + // Rc::unwrap_or_clone is not stable yet. + .unwrap_or_else(|m| m.to_vec()) } } @@ -453,7 +433,7 @@ impl Reconcilable for VList { } fn reconcile( - self, + mut self, root: &BSubtree, parent_scope: &AnyScope, parent: &Element, @@ -468,8 +448,16 @@ impl Reconcilable for VList { // The left items are known since we want to insert them // (self.children). For the right ones, we will look at the bundle, // i.e. the current DOM list element that we want to replace with self. - let (key, fully_keyed, lefts) = self.split_for_bundle(); + if self.is_empty() { + // Without a placeholder the next element becomes first + // and corrupts the order of rendering + // We use empty text element to stake out a place + self.add_child(VText::new("").into()); + } + + let fully_keyed = self.fully_keyed(); + let lefts = BList::unwrap_children(self.children); let rights = &mut blist.rev_children; test_log!("lefts: {:?}", lefts); test_log!("rights: {:?}", rights); @@ -483,7 +471,7 @@ impl Reconcilable for VList { BList::apply_unkeyed(root, parent_scope, parent, next_sibling, lefts, rights) }; blist.fully_keyed = fully_keyed; - blist.key = key; + blist.key = self.key; test_log!("result: {:?}", rights); first } @@ -503,8 +491,8 @@ mod feat_hydration { fragment: &mut Fragment, ) -> (NodeRef, Self::Bundle) { let node_ref = NodeRef::default(); - - let (key, fully_keyed, vchildren) = self.split_for_bundle(); + let fully_keyed = self.fully_keyed(); + let vchildren = BList::unwrap_children(self.children); let mut children = Vec::with_capacity(vchildren.len()); for (index, child) in vchildren.into_iter().enumerate() { @@ -524,7 +512,7 @@ mod feat_hydration { BList { rev_children: children, fully_keyed, - key, + key: self.key, }, ) } diff --git a/packages/yew/src/virtual_dom/vlist.rs b/packages/yew/src/virtual_dom/vlist.rs index 44e73c3ca02..c205226a9ac 100644 --- a/packages/yew/src/virtual_dom/vlist.rs +++ b/packages/yew/src/virtual_dom/vlist.rs @@ -21,9 +21,6 @@ pub struct VList { fully_keyed: FullyKeyedState, pub key: Option, - - /// A padding to reduce bundle size - _padding: u64, } impl PartialEq for VList { @@ -76,7 +73,6 @@ impl VList { children: None, key: None, fully_keyed: FullyKeyedState::KnownFullyKeyed, - _padding: 0, } } @@ -86,9 +82,12 @@ impl VList { fully_keyed: FullyKeyedState::Unknown, children: Some(Rc::new(children)), key, - _padding: 0, }; - vlist.recheck_fully_keyed(); + vlist.fully_keyed = if vlist.fully_keyed() { + FullyKeyedState::KnownFullyKeyed + } else { + FullyKeyedState::KnownMissingKeys + }; vlist } From 3ca55be4ea309f516d91ecd8e2aa8256dd810d11 Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Mon, 19 Dec 2022 23:44:21 +0900 Subject: [PATCH 07/14] Add size marker. --- packages/yew/src/virtual_dom/vlist.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/yew/src/virtual_dom/vlist.rs b/packages/yew/src/virtual_dom/vlist.rs index c205226a9ac..1eac254e0a7 100644 --- a/packages/yew/src/virtual_dom/vlist.rs +++ b/packages/yew/src/virtual_dom/vlist.rs @@ -21,6 +21,9 @@ pub struct VList { fully_keyed: FullyKeyedState, pub key: Option, + + /// A size marker to reduce bundle size + _marker: u64, } impl PartialEq for VList { @@ -73,6 +76,7 @@ impl VList { children: None, key: None, fully_keyed: FullyKeyedState::KnownFullyKeyed, + _marker: 0, } } @@ -82,6 +86,7 @@ impl VList { fully_keyed: FullyKeyedState::Unknown, children: Some(Rc::new(children)), key, + _marker: 0, }; vlist.fully_keyed = if vlist.fully_keyed() { FullyKeyedState::KnownFullyKeyed From 128ed4277fabcc46bff387babd78795f3ad43c70 Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Tue, 20 Dec 2022 00:06:31 +0900 Subject: [PATCH 08/14] Revert "Add size marker." This reverts commit 3ca55be4ea309f516d91ecd8e2aa8256dd810d11. --- packages/yew/src/virtual_dom/vlist.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/yew/src/virtual_dom/vlist.rs b/packages/yew/src/virtual_dom/vlist.rs index 1eac254e0a7..c205226a9ac 100644 --- a/packages/yew/src/virtual_dom/vlist.rs +++ b/packages/yew/src/virtual_dom/vlist.rs @@ -21,9 +21,6 @@ pub struct VList { fully_keyed: FullyKeyedState, pub key: Option, - - /// A size marker to reduce bundle size - _marker: u64, } impl PartialEq for VList { @@ -76,7 +73,6 @@ impl VList { children: None, key: None, fully_keyed: FullyKeyedState::KnownFullyKeyed, - _marker: 0, } } @@ -86,7 +82,6 @@ impl VList { fully_keyed: FullyKeyedState::Unknown, children: Some(Rc::new(children)), key, - _marker: 0, }; vlist.fully_keyed = if vlist.fully_keyed() { FullyKeyedState::KnownFullyKeyed From d21b9d179c5a994f49ff04846b3e2b6f17dac19c Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Tue, 20 Dec 2022 01:39:15 +0900 Subject: [PATCH 09/14] Update benchmark. --- tools/benchmark-ssr/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/benchmark-ssr/src/main.rs b/tools/benchmark-ssr/src/main.rs index b0f24e3ed56..27734180f79 100644 --- a/tools/benchmark-ssr/src/main.rs +++ b/tools/benchmark-ssr/src/main.rs @@ -94,7 +94,7 @@ async fn bench_many_providers() -> Duration { fn Provider(props: &ProviderProps) -> Html { let ProviderProps { children } = props.clone(); - html! {
{children}
} + html! {<>{children}} } #[function_component] From f3d4c7fe81afb1d0128e3c178ef880ed310da85f Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Tue, 20 Dec 2022 20:54:50 +0900 Subject: [PATCH 10/14] Fix children_mut visibility. --- packages/yew/src/virtual_dom/vlist.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/yew/src/virtual_dom/vlist.rs b/packages/yew/src/virtual_dom/vlist.rs index c205226a9ac..8cfe04c603e 100644 --- a/packages/yew/src/virtual_dom/vlist.rs +++ b/packages/yew/src/virtual_dom/vlist.rs @@ -83,16 +83,11 @@ impl VList { children: Some(Rc::new(children)), key, }; - vlist.fully_keyed = if vlist.fully_keyed() { - FullyKeyedState::KnownFullyKeyed - } else { - FullyKeyedState::KnownMissingKeys - }; + vlist.recheck_fully_keyed(); vlist } - #[inline(never)] - pub fn children_mut(&mut self) -> &mut Vec { + pub(crate) fn children_mut(&mut self) -> &mut Vec { loop { match self.children { Some(ref mut m) => return Rc::make_mut(m), From 622650869e945b825819b0c426a5afdf822f64f6 Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Tue, 20 Dec 2022 22:29:25 +0900 Subject: [PATCH 11/14] Try to exclude add_child behaviour. --- packages/yew/src/dom_bundle/blist.rs | 43 ++++++++++++++++------------ 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/packages/yew/src/dom_bundle/blist.rs b/packages/yew/src/dom_bundle/blist.rs index 4d125fd8d46..e7fd873c1f2 100644 --- a/packages/yew/src/dom_bundle/blist.rs +++ b/packages/yew/src/dom_bundle/blist.rs @@ -23,14 +23,27 @@ pub(super) struct BList { key: Option, } -impl BList { - // Unwraps a Vec from the children of a VList. - fn unwrap_children(children: Option>>) -> Vec { - children +impl VList { + // Splits a VList for creating / reconcling to a BList. + fn split_for_blist(self) -> (Option, bool, Vec) { + let mut fully_keyed = self.fully_keyed(); + + let mut children = self + .children .map(Rc::try_unwrap) .unwrap_or_else(|| Ok(Vec::new())) // Rc::unwrap_or_clone is not stable yet. - .unwrap_or_else(|m| m.to_vec()) + .unwrap_or_else(|m| m.to_vec()); + + if children.is_empty() { + // Without a placeholder the next element becomes first + // and corrupts the order of rendering + // We use empty text element to stake out a place + children.push(VText::new("").into()); + fully_keyed = false; + } + + (self.key, fully_keyed, children) } } @@ -433,7 +446,7 @@ impl Reconcilable for VList { } fn reconcile( - mut self, + self, root: &BSubtree, parent_scope: &AnyScope, parent: &Element, @@ -448,16 +461,8 @@ impl Reconcilable for VList { // The left items are known since we want to insert them // (self.children). For the right ones, we will look at the bundle, // i.e. the current DOM list element that we want to replace with self. + let (key, fully_keyed, lefts) = self.split_for_blist(); - if self.is_empty() { - // Without a placeholder the next element becomes first - // and corrupts the order of rendering - // We use empty text element to stake out a place - self.add_child(VText::new("").into()); - } - - let fully_keyed = self.fully_keyed(); - let lefts = BList::unwrap_children(self.children); let rights = &mut blist.rev_children; test_log!("lefts: {:?}", lefts); test_log!("rights: {:?}", rights); @@ -471,7 +476,7 @@ impl Reconcilable for VList { BList::apply_unkeyed(root, parent_scope, parent, next_sibling, lefts, rights) }; blist.fully_keyed = fully_keyed; - blist.key = self.key; + blist.key = key; test_log!("result: {:?}", rights); first } @@ -491,8 +496,8 @@ mod feat_hydration { fragment: &mut Fragment, ) -> (NodeRef, Self::Bundle) { let node_ref = NodeRef::default(); - let fully_keyed = self.fully_keyed(); - let vchildren = BList::unwrap_children(self.children); + let (key, fully_keyed, vchildren) = self.split_for_blist(); + let mut children = Vec::with_capacity(vchildren.len()); for (index, child) in vchildren.into_iter().enumerate() { @@ -512,7 +517,7 @@ mod feat_hydration { BList { rev_children: children, fully_keyed, - key: self.key, + key, }, ) } From 98f99098817fe6367e1ae174c89d2aadf8ff65a3 Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Tue, 20 Dec 2022 23:24:39 +0900 Subject: [PATCH 12/14] Fix typo. --- packages/yew/src/dom_bundle/blist.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/yew/src/dom_bundle/blist.rs b/packages/yew/src/dom_bundle/blist.rs index e7fd873c1f2..25a1c3dadb9 100644 --- a/packages/yew/src/dom_bundle/blist.rs +++ b/packages/yew/src/dom_bundle/blist.rs @@ -24,7 +24,7 @@ pub(super) struct BList { } impl VList { - // Splits a VList for creating / reconcling to a BList. + // Splits a VList for creating / reconciling to a BList. fn split_for_blist(self) -> (Option, bool, Vec) { let mut fully_keyed = self.fully_keyed(); From 4555f151958c4c73eec414237e4dd522682859e2 Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Tue, 20 Dec 2022 23:30:20 +0900 Subject: [PATCH 13/14] Adjust visibility and docs. --- packages/yew/src/virtual_dom/vlist.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/yew/src/virtual_dom/vlist.rs b/packages/yew/src/virtual_dom/vlist.rs index 8cfe04c603e..fc211ce976c 100644 --- a/packages/yew/src/virtual_dom/vlist.rs +++ b/packages/yew/src/virtual_dom/vlist.rs @@ -87,7 +87,10 @@ impl VList { vlist } - pub(crate) fn children_mut(&mut self) -> &mut Vec { + // Returns a mutable reference to children, allocates the children if it hasn't been done. + // + // This method does not reassign key state. So it should only be used internally. + fn children_mut(&mut self) -> &mut Vec { loop { match self.children { Some(ref mut m) => return Rc::make_mut(m), From f36a4f163f341b47e97c4554c39c252e1414b723 Mon Sep 17 00:00:00 2001 From: Kaede Hoshikawa Date: Sun, 2 Apr 2023 17:49:11 +0900 Subject: [PATCH 14/14] Fix hydration with empty children. --- packages/yew/src/dom_bundle/blist.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/yew/src/dom_bundle/blist.rs b/packages/yew/src/dom_bundle/blist.rs index f78e6ac942a..93a9af585a8 100644 --- a/packages/yew/src/dom_bundle/blist.rs +++ b/packages/yew/src/dom_bundle/blist.rs @@ -26,23 +26,15 @@ pub(super) struct BList { impl VList { // Splits a VList for creating / reconciling to a BList. fn split_for_blist(self) -> (Option, bool, Vec) { - let mut fully_keyed = self.fully_keyed(); + let fully_keyed = self.fully_keyed(); - let mut children = self + let children = self .children .map(Rc::try_unwrap) .unwrap_or_else(|| Ok(Vec::new())) // Rc::unwrap_or_clone is not stable yet. .unwrap_or_else(|m| m.to_vec()); - if children.is_empty() { - // Without a placeholder the next element becomes first - // and corrupts the order of rendering - // We use empty text element to stake out a place - children.push(VText::new("").into()); - fully_keyed = false; - } - (self.key, fully_keyed, children) } } @@ -437,7 +429,15 @@ impl Reconcilable for VList { // The left items are known since we want to insert them // (self.children). For the right ones, we will look at the bundle, // i.e. the current DOM list element that we want to replace with self. - let (key, fully_keyed, lefts) = self.split_for_blist(); + let (key, mut fully_keyed, mut lefts) = self.split_for_blist(); + + if lefts.is_empty() { + // Without a placeholder the next element becomes first + // and corrupts the order of rendering + // We use empty text element to stake out a place + lefts.push(VText::new("").into()); + fully_keyed = false; + } let rights = &mut blist.rev_children; test_log!("lefts: {:?}", lefts);