From 8a1c96a68cb5c46d366ac50d77afb9777482cbf4 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Fri, 9 Dec 2022 16:18:37 -0600 Subject: [PATCH 01/25] fix custom attribute value support --- packages/autofmt/src/buffer.rs | 4 +- packages/autofmt/src/element.rs | 2 +- packages/core/src/create.rs | 37 +++---- packages/core/src/diff.rs | 22 ++-- packages/core/src/lib.rs | 14 +-- packages/core/src/mutations.rs | 21 +--- packages/core/src/nodes.rs | 77 ++++++++++++-- packages/core/src/scopes.rs | 4 +- packages/core/src/virtual_dom.rs | 10 +- packages/core/tests/attr_cleanup.rs | 21 +++- packages/core/tests/boolattrs.rs | 11 +- packages/core/tests/borrowedstate.rs | 2 +- packages/core/tests/bubble_error.rs | 4 +- packages/core/tests/cycle.rs | 18 ++-- packages/core/tests/diff_component.rs | 28 ++--- packages/core/tests/diff_keyed_list.rs | 38 +++---- packages/core/tests/diff_unkeyed_list.rs | 126 ++++++++++++----------- packages/core/tests/kitchen_sink.rs | 18 ++-- packages/core/tests/suspense.rs | 50 ++++----- packages/dioxus/benches/jsframework.rs | 2 +- packages/native-core-macro/Cargo.toml | 3 +- packages/native-core-macro/src/lib.rs | 3 +- packages/native-core/src/node.rs | 41 ++++++-- packages/native-core/src/real_dom.rs | 54 ++-------- packages/rsx/src/element.rs | 6 +- packages/tui/src/focus.rs | 2 +- packages/web/src/dom.rs | 19 +++- packages/web/src/lib.rs | 8 +- 28 files changed, 369 insertions(+), 276 deletions(-) diff --git a/packages/autofmt/src/buffer.rs b/packages/autofmt/src/buffer.rs index 45799ca62..79ad7dfb7 100644 --- a/packages/autofmt/src/buffer.rs +++ b/packages/autofmt/src/buffer.rs @@ -144,8 +144,8 @@ impl Buffer { let mut total = 0; for attr in attributes { - if self.current_span_is_primary(attr.attr.flart()) { - 'line: for line in self.src[..attr.attr.flart().start().line - 1].iter().rev() { + if self.current_span_is_primary(attr.attr.start()) { + 'line: for line in self.src[..attr.attr.start().start().line - 1].iter().rev() { match (line.trim().starts_with("//"), line.is_empty()) { (true, _) => return 100000, (_, true) => continue 'line, diff --git a/packages/autofmt/src/element.rs b/packages/autofmt/src/element.rs index fd4529766..b5f1c2f84 100644 --- a/packages/autofmt/src/element.rs +++ b/packages/autofmt/src/element.rs @@ -157,7 +157,7 @@ impl Buffer { while let Some(attr) = attr_iter.next() { self.indent += 1; if !sameline { - self.write_comments(attr.attr.flart())?; + self.write_comments(attr.attr.start())?; } self.indent -= 1; diff --git a/packages/core/src/create.rs b/packages/core/src/create.rs index cc63a0e8b..4804897a2 100644 --- a/packages/core/src/create.rs +++ b/packages/core/src/create.rs @@ -105,27 +105,10 @@ impl<'b> VirtualDom { attribute.mounted_element.set(id); // Safety: we promise not to re-alias this text later on after committing it to the mutation - let unbounded_name = unsafe { std::mem::transmute(attribute.name) }; + let unbounded_name: &str = + unsafe { std::mem::transmute(attribute.name) }; match &attribute.value { - AttributeValue::Text(value) => { - // Safety: we promise not to re-alias this text later on after committing it to the mutation - let unbounded_value = unsafe { std::mem::transmute(*value) }; - - self.mutations.push(SetAttribute { - name: unbounded_name, - value: unbounded_value, - ns: attribute.namespace, - id, - }) - } - AttributeValue::Bool(value) => { - self.mutations.push(SetBoolAttribute { - name: unbounded_name, - value: *value, - id, - }) - } AttributeValue::Listener(_) => { self.mutations.push(NewEventListener { // all listeners start with "on" @@ -134,10 +117,18 @@ impl<'b> VirtualDom { id, }) } - AttributeValue::Float(_) => todo!(), - AttributeValue::Int(_) => todo!(), - AttributeValue::Any(_) => todo!(), - AttributeValue::None => todo!(), + _ => { + // Safety: we promise not to re-alias this text later on after committing it to the mutation + let unbounded_value = + unsafe { std::mem::transmute(attribute.value.clone()) }; + + self.mutations.push(SetAttribute { + name: unbounded_name, + value: unbounded_value, + ns: attribute.namespace, + id, + }) + } } // Only push the dynamic attributes forward if they match the current path (same element) diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index 0384192e2..b2d90a2ca 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -77,20 +77,14 @@ impl<'b> VirtualDom { if left_attr.value != right_attr.value || left_attr.volatile { // todo: add more types of attribute values - match right_attr.value { - AttributeValue::Text(text) => { - let name = unsafe { std::mem::transmute(left_attr.name) }; - let value = unsafe { std::mem::transmute(text) }; - self.mutations.push(Mutation::SetAttribute { - id: left_attr.mounted_element.get(), - ns: right_attr.namespace, - name, - value, - }); - } - // todo: more types of attribute values - _ => todo!("other attribute types"), - } + let name = unsafe { std::mem::transmute(left_attr.name) }; + let value = unsafe { std::mem::transmute(right_attr.value.clone()) }; + self.mutations.push(Mutation::SetAttribute { + id: left_attr.mounted_element.get(), + ns: right_attr.namespace, + name, + value, + }); } } diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index 613a4a115..0de74c911 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -70,10 +70,10 @@ pub(crate) mod innerlude { } pub use crate::innerlude::{ - fc_to_builder, Attribute, AttributeValue, Component, DynamicNode, Element, ElementId, Event, - Fragment, IntoDynNode, LazyNodes, Mutation, Mutations, Properties, RenderReturn, Scope, - ScopeId, ScopeState, Scoped, SuspenseContext, TaskId, Template, TemplateAttribute, - TemplateNode, VComponent, VNode, VText, VirtualDom, + fc_to_builder, AnyValueBox, Attribute, AttributeValue, Component, DynamicNode, Element, + ElementId, Event, Fragment, IntoAttributeValue, IntoDynNode, LazyNodes, Mutation, Mutations, + Properties, RenderReturn, Scope, ScopeId, ScopeState, Scoped, SuspenseContext, TaskId, + Template, TemplateAttribute, TemplateNode, VComponent, VNode, VText, VirtualDom, }; /// The purpose of this module is to alleviate imports of many common types @@ -81,9 +81,9 @@ pub use crate::innerlude::{ /// This includes types like [`Scope`], [`Element`], and [`Component`]. pub mod prelude { pub use crate::innerlude::{ - fc_to_builder, Element, Event, EventHandler, Fragment, LazyNodes, Properties, Scope, - ScopeId, ScopeState, Scoped, TaskId, Template, TemplateAttribute, TemplateNode, VNode, - VirtualDom, + fc_to_builder, Element, Event, EventHandler, Fragment, IntoAttributeValue, LazyNodes, + Properties, Scope, ScopeId, ScopeState, Scoped, TaskId, Template, TemplateAttribute, + TemplateNode, VNode, VirtualDom, }; } diff --git a/packages/core/src/mutations.rs b/packages/core/src/mutations.rs index 2512ae9fa..938458a3d 100644 --- a/packages/core/src/mutations.rs +++ b/packages/core/src/mutations.rs @@ -1,6 +1,6 @@ use fxhash::FxHashSet; -use crate::{arena::ElementId, ScopeId, Template}; +use crate::{arena::ElementId, AttributeValue, ScopeId, Template}; /// A container for all the relevant steps to modify the Real DOM /// @@ -48,7 +48,7 @@ impl<'a> Mutations<'a> { /// Push a new mutation into the dom_edits list pub(crate) fn push(&mut self, mutation: Mutation<'static>) { - self.edits.push(mutation) + unsafe { self.edits.push(std::mem::transmute(mutation)) } } } @@ -61,7 +61,7 @@ impl<'a> Mutations<'a> { derive(serde::Serialize, serde::Deserialize), serde(tag = "type") )] -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq)] pub enum Mutation<'a> { /// Add these m children to the target element AppendChildren { @@ -193,8 +193,9 @@ pub enum Mutation<'a> { SetAttribute { /// The name of the attribute to set. name: &'a str, + /// The value of the attribute. - value: &'a str, + value: AttributeValue<'a>, /// The ID of the node to set the attribute of. id: ElementId, @@ -204,18 +205,6 @@ pub enum Mutation<'a> { ns: Option<&'a str>, }, - /// Set the value of a node's attribute. - SetBoolAttribute { - /// The name of the attribute to set. - name: &'a str, - - /// The value of the attribute. - value: bool, - - /// The ID of the node to set the attribute of. - id: ElementId, - }, - /// Set the textcontent of a node. SetText { /// The textcontent of the node diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 81ed5da06..c776da967 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -8,6 +8,7 @@ use std::{ cell::{Cell, RefCell}, fmt::Arguments, future::Future, + rc::Rc, }; pub type TemplateId = &'static str; @@ -88,7 +89,7 @@ impl<'a> VNode<'a> { pub(crate) fn clear_listeners(&self) { for attr in self.dynamic_attrs { if let AttributeValue::Listener(l) = &attr.value { - l.borrow_mut().take(); + l.0.borrow_mut().take(); } } } @@ -317,6 +318,9 @@ pub struct Attribute<'a> { /// /// These are built-in to be faster during the diffing process. To use a custom value, use the [`AttributeValue::Any`] /// variant. +#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "serialize", serde(untagged))] +#[derive(Clone)] pub enum AttributeValue<'a> { /// Text attribute Text(&'a str), @@ -331,16 +335,67 @@ pub enum AttributeValue<'a> { Bool(bool), /// A listener, like "onclick" - Listener(RefCell>>), + Listener(ListenerCb<'a>), /// An arbitrary value that implements PartialEq and is static - Any(BumpBox<'a, dyn AnyValue>), + Any(AnyValueBox), /// A "none" value, resulting in the removal of an attribute from the dom None, } -type ListenerCb<'a> = BumpBox<'a, dyn FnMut(Event) + 'a>; +pub type ListenerCbInner<'a> = RefCell) + 'a>>>; +pub struct ListenerCb<'a>(pub ListenerCbInner<'a>); + +impl Clone for ListenerCb<'_> { + fn clone(&self) -> Self { + panic!("ListenerCb cannot be cloned") + } +} + +#[cfg(feature = "serialize")] +impl<'a> serde::Serialize for ListenerCb<'a> { + fn serialize(&self, _: S) -> Result + where + S: serde::Serializer, + { + panic!("ListenerCb cannot be serialized") + } +} + +#[cfg(feature = "serialize")] +impl<'de, 'a> serde::Deserialize<'de> for ListenerCb<'a> { + fn deserialize(_: D) -> Result + where + D: serde::Deserializer<'de>, + { + panic!("ListenerCb cannot be deserialized") + } +} + +/// A boxed value that implements PartialEq and Any +#[derive(Clone)] +pub struct AnyValueBox(pub Rc); + +#[cfg(feature = "serialize")] +impl serde::Serialize for AnyValueBox { + fn serialize(&self, _: S) -> Result + where + S: serde::Serializer, + { + panic!("AnyValueBox cannot be serialized") + } +} + +#[cfg(feature = "serialize")] +impl<'de> serde::Deserialize<'de> for AnyValueBox { + fn deserialize(_: D) -> Result + where + D: serde::Deserializer<'de>, + { + panic!("AnyValueBox cannot be deserialized") + } +} impl<'a> std::fmt::Debug for AttributeValue<'a> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -364,8 +419,8 @@ impl<'a> PartialEq for AttributeValue<'a> { (Self::Int(l0), Self::Int(r0)) => l0 == r0, (Self::Bool(l0), Self::Bool(r0)) => l0 == r0, (Self::Listener(_), Self::Listener(_)) => true, - (Self::Any(l0), Self::Any(r0)) => l0.any_cmp(r0.as_ref()), - _ => core::mem::discriminant(self) == core::mem::discriminant(other), + (Self::Any(l0), Self::Any(r0)) => l0.0.any_cmp(r0.0.as_ref()), + _ => false, } } } @@ -559,21 +614,25 @@ impl<'a> IntoAttributeValue<'a> for &'a str { AttributeValue::Text(self) } } + impl<'a> IntoAttributeValue<'a> for f64 { fn into_value(self, _: &'a Bump) -> AttributeValue<'a> { AttributeValue::Float(self) } } + impl<'a> IntoAttributeValue<'a> for i64 { fn into_value(self, _: &'a Bump) -> AttributeValue<'a> { AttributeValue::Int(self) } } + impl<'a> IntoAttributeValue<'a> for bool { fn into_value(self, _: &'a Bump) -> AttributeValue<'a> { AttributeValue::Bool(self) } } + impl<'a> IntoAttributeValue<'a> for Arguments<'_> { fn into_value(self, bump: &'a Bump) -> AttributeValue<'a> { use bumpalo::core_alloc::fmt::Write; @@ -582,3 +641,9 @@ impl<'a> IntoAttributeValue<'a> for Arguments<'_> { AttributeValue::Text(str_buf.into_bump_str()) } } + +impl<'a> IntoAttributeValue<'a> for AnyValueBox { + fn into_value(self, _: &'a Bump) -> AttributeValue<'a> { + AttributeValue::Any(self) + } +} diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index b1bbb42fa..31ac50440 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -4,7 +4,7 @@ use crate::{ arena::ElementId, bump_frame::BumpFrame, innerlude::{DynamicNode, EventHandler, VComponent, VText}, - innerlude::{Scheduler, SchedulerMsg}, + innerlude::{ListenerCb, Scheduler, SchedulerMsg}, lazynodes::LazyNodes, nodes::{ComponentReturn, IntoAttributeValue, IntoDynNode, RenderReturn}, Attribute, AttributeValue, Element, Event, Properties, TaskId, @@ -483,7 +483,7 @@ impl<'src> ScopeState { })) }; - AttributeValue::Listener(RefCell::new(Some(boxed))) + AttributeValue::Listener(ListenerCb(RefCell::new(Some(boxed)))) } /// Store a value between renders. The foundational hook for all other hooks. diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index ced92acb0..4c01512e2 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -386,7 +386,7 @@ impl VirtualDom { // We check the bubble state between each call to see if the event has been stopped from bubbling for listener in listeners.drain(..).rev() { if let AttributeValue::Listener(listener) = listener { - if let Some(cb) = listener.borrow_mut().as_deref_mut() { + if let Some(cb) = listener.0.borrow_mut().as_deref_mut() { cb(uievent.clone()); } @@ -493,7 +493,7 @@ impl VirtualDom { RenderReturn::Async(_) => unreachable!("Root scope cannot be an async component"), } - self.finalize() + unsafe { std::mem::transmute(self.finalize()) } } /// Render whatever the VirtualDom has ready as fast as possible without requiring an executor to progress @@ -591,7 +591,7 @@ impl VirtualDom { // If there's no pending suspense, then we have no reason to wait for anything if self.scheduler.leaves.borrow().is_empty() { - return self.finalize(); + return unsafe { std::mem::transmute(self.finalize()) }; } // Poll the suspense leaves in the meantime @@ -605,13 +605,13 @@ impl VirtualDom { if let Either::Left((_, _)) = select(&mut deadline, pinned).await { // release the borrowed drop(work); - return self.finalize(); + return unsafe { std::mem::transmute(self.finalize()) }; } } } /// Swap the current mutations with a new - fn finalize(&mut self) -> Mutations { + fn finalize(&mut self) -> Mutations<'static> { // todo: make this a routine let mut out = Mutations::default(); std::mem::swap(&mut self.mutations, &mut out); diff --git a/packages/core/tests/attr_cleanup.rs b/packages/core/tests/attr_cleanup.rs index c0aa9a1da..c8e1baa1f 100644 --- a/packages/core/tests/attr_cleanup.rs +++ b/packages/core/tests/attr_cleanup.rs @@ -2,6 +2,7 @@ //! //! This tests to ensure we clean it up +use bumpalo::Bump; use dioxus::core::{ElementId, Mutation::*}; use dioxus::prelude::*; @@ -22,6 +23,8 @@ fn attrs_cycle() { } }); + let bump = Bump::new(); + assert_eq!( dom.rebuild().santize().edits, [ @@ -36,8 +39,13 @@ fn attrs_cycle() { [ LoadTemplate { name: "template", index: 0, id: ElementId(2,) }, AssignId { path: &[0,], id: ElementId(3,) }, - SetAttribute { name: "class", value: "1", id: ElementId(3,), ns: None }, - SetAttribute { name: "id", value: "1", id: ElementId(3,), ns: None }, + SetAttribute { + name: "class", + value: "1".into_value(&bump), + id: ElementId(3,), + ns: None + }, + SetAttribute { name: "id", value: "1".into_value(&bump), id: ElementId(3,), ns: None }, ReplaceWith { id: ElementId(1,), m: 1 }, ] ); @@ -57,8 +65,13 @@ fn attrs_cycle() { [ LoadTemplate { name: "template", index: 0, id: ElementId(2) }, AssignId { path: &[0], id: ElementId(1) }, - SetAttribute { name: "class", value: "3", id: ElementId(1), ns: None }, - SetAttribute { name: "id", value: "3", id: ElementId(1), ns: None }, + SetAttribute { + name: "class", + value: "3".into_value(&bump), + id: ElementId(1), + ns: None + }, + SetAttribute { name: "id", value: "3".into_value(&bump), id: ElementId(1), ns: None }, ReplaceWith { id: ElementId(3), m: 1 } ] ); diff --git a/packages/core/tests/boolattrs.rs b/packages/core/tests/boolattrs.rs index 5cd2ad1ad..c2ad70333 100644 --- a/packages/core/tests/boolattrs.rs +++ b/packages/core/tests/boolattrs.rs @@ -1,15 +1,22 @@ +use bumpalo::Bump; use dioxus::core::{ElementId, Mutation::*}; use dioxus::prelude::*; #[test] fn bool_test() { let mut app = VirtualDom::new(|cx| cx.render(rsx!(div { hidden: false }))); + let bump = Bump::new(); assert_eq!( app.rebuild().santize().edits, [ LoadTemplate { name: "template", index: 0, id: ElementId(1) }, - SetBoolAttribute { name: "hidden", value: false, id: ElementId(1,) }, + SetAttribute { + name: "hidden", + value: false.into_value(&bump), + id: ElementId(1,), + ns: None + }, AppendChildren { m: 1, id: ElementId(0) }, ] - ) + ); } diff --git a/packages/core/tests/borrowedstate.rs b/packages/core/tests/borrowedstate.rs index 0aace9c97..1b83789ec 100644 --- a/packages/core/tests/borrowedstate.rs +++ b/packages/core/tests/borrowedstate.rs @@ -18,7 +18,7 @@ fn test_borrowed_state() { ReplacePlaceholder { path: &[0,], m: 1 }, AppendChildren { m: 1, id: ElementId(0) }, ] - ) + ); } fn Parent(cx: Scope) -> Element { diff --git a/packages/core/tests/bubble_error.rs b/packages/core/tests/bubble_error.rs index 3dbfe80a9..ebbbc613c 100644 --- a/packages/core/tests/bubble_error.rs +++ b/packages/core/tests/bubble_error.rs @@ -20,7 +20,9 @@ fn app(cx: Scope) -> Element { fn bubbles_error() { let mut dom = VirtualDom::new(app); - let _edits = dom.rebuild().santize(); + { + let _edits = dom.rebuild().santize(); + } dom.mark_dirty(ScopeId(0)); diff --git a/packages/core/tests/cycle.rs b/packages/core/tests/cycle.rs index 133b04658..10498f5cc 100644 --- a/packages/core/tests/cycle.rs +++ b/packages/core/tests/cycle.rs @@ -12,14 +12,16 @@ fn cycling_elements() { }) }); - let edits = dom.rebuild().santize(); - assert_eq!( - edits.edits, - [ - LoadTemplate { name: "template", index: 0, id: ElementId(1,) }, - AppendChildren { m: 1, id: ElementId(0) }, - ] - ); + { + let edits = dom.rebuild().santize(); + assert_eq!( + edits.edits, + [ + LoadTemplate { name: "template", index: 0, id: ElementId(1,) }, + AppendChildren { m: 1, id: ElementId(0) }, + ] + ); + } dom.mark_dirty(ScopeId(0)); assert_eq!( diff --git a/packages/core/tests/diff_component.rs b/packages/core/tests/diff_component.rs index 2b74f8302..69ab6de6d 100644 --- a/packages/core/tests/diff_component.rs +++ b/packages/core/tests/diff_component.rs @@ -59,19 +59,21 @@ fn component_swap() { } let mut dom = VirtualDom::new(app); - let edits = dom.rebuild().santize(); - assert_eq!( - edits.edits, - [ - LoadTemplate { name: "template", index: 0, id: ElementId(1) }, - LoadTemplate { name: "template", index: 0, id: ElementId(2) }, - LoadTemplate { name: "template", index: 0, id: ElementId(3) }, - LoadTemplate { name: "template", index: 0, id: ElementId(4) }, - ReplacePlaceholder { path: &[1], m: 3 }, - LoadTemplate { name: "template", index: 0, id: ElementId(5) }, - AppendChildren { m: 2, id: ElementId(0) } - ] - ); + { + let edits = dom.rebuild().santize(); + assert_eq!( + edits.edits, + [ + LoadTemplate { name: "template", index: 0, id: ElementId(1) }, + LoadTemplate { name: "template", index: 0, id: ElementId(2) }, + LoadTemplate { name: "template", index: 0, id: ElementId(3) }, + LoadTemplate { name: "template", index: 0, id: ElementId(4) }, + ReplacePlaceholder { path: &[1], m: 3 }, + LoadTemplate { name: "template", index: 0, id: ElementId(5) }, + AppendChildren { m: 2, id: ElementId(0) } + ] + ); + } dom.mark_dirty(ScopeId(0)); assert_eq!( diff --git a/packages/core/tests/diff_keyed_list.rs b/packages/core/tests/diff_keyed_list.rs index 81013849d..be26e00c0 100644 --- a/packages/core/tests/diff_keyed_list.rs +++ b/packages/core/tests/diff_keyed_list.rs @@ -20,22 +20,24 @@ fn keyed_diffing_out_of_order() { cx.render(rsx!(order.iter().map(|i| rsx!(div { key: "{i}" })))) }); - assert_eq!( - dom.rebuild().santize().edits, - [ - LoadTemplate { name: "template", index: 0, id: ElementId(1,) }, - LoadTemplate { name: "template", index: 0, id: ElementId(2,) }, - LoadTemplate { name: "template", index: 0, id: ElementId(3,) }, - LoadTemplate { name: "template", index: 0, id: ElementId(4,) }, - LoadTemplate { name: "template", index: 0, id: ElementId(5,) }, - LoadTemplate { name: "template", index: 0, id: ElementId(6,) }, - LoadTemplate { name: "template", index: 0, id: ElementId(7,) }, - LoadTemplate { name: "template", index: 0, id: ElementId(8,) }, - LoadTemplate { name: "template", index: 0, id: ElementId(9,) }, - LoadTemplate { name: "template", index: 0, id: ElementId(10,) }, - AppendChildren { m: 10, id: ElementId(0) }, - ] - ); + { + assert_eq!( + dom.rebuild().santize().edits, + [ + LoadTemplate { name: "template", index: 0, id: ElementId(1,) }, + LoadTemplate { name: "template", index: 0, id: ElementId(2,) }, + LoadTemplate { name: "template", index: 0, id: ElementId(3,) }, + LoadTemplate { name: "template", index: 0, id: ElementId(4,) }, + LoadTemplate { name: "template", index: 0, id: ElementId(5,) }, + LoadTemplate { name: "template", index: 0, id: ElementId(6,) }, + LoadTemplate { name: "template", index: 0, id: ElementId(7,) }, + LoadTemplate { name: "template", index: 0, id: ElementId(8,) }, + LoadTemplate { name: "template", index: 0, id: ElementId(9,) }, + LoadTemplate { name: "template", index: 0, id: ElementId(10,) }, + AppendChildren { m: 10, id: ElementId(0) }, + ] + ); + } dom.mark_dirty(ScopeId(0)); assert_eq!( @@ -44,7 +46,7 @@ fn keyed_diffing_out_of_order() { PushRoot { id: ElementId(7,) }, InsertBefore { id: ElementId(5,), m: 1 }, ] - ) + ); } /// Should result in moves only @@ -70,7 +72,7 @@ fn keyed_diffing_out_of_order_adds() { PushRoot { id: ElementId(4,) }, InsertBefore { id: ElementId(1,), m: 2 }, ] - ) + ); } /// Should result in moves only diff --git a/packages/core/tests/diff_unkeyed_list.rs b/packages/core/tests/diff_unkeyed_list.rs index 415aa79c0..84fd65604 100644 --- a/packages/core/tests/diff_unkeyed_list.rs +++ b/packages/core/tests/diff_unkeyed_list.rs @@ -314,66 +314,76 @@ fn remove_many() { }) }); - let edits = dom.rebuild().santize(); - assert!(edits.templates.is_empty()); - assert_eq!( - edits.edits, - [ - CreatePlaceholder { id: ElementId(1,) }, - AppendChildren { id: ElementId(0), m: 1 }, - ] - ); + { + let edits = dom.rebuild().santize(); + assert!(edits.templates.is_empty()); + assert_eq!( + edits.edits, + [ + CreatePlaceholder { id: ElementId(1,) }, + AppendChildren { id: ElementId(0), m: 1 }, + ] + ); + } - dom.mark_dirty(ScopeId(0)); - let edits = dom.render_immediate().santize(); - assert_eq!( - edits.edits, - [ - LoadTemplate { name: "template", index: 0, id: ElementId(2,) }, - HydrateText { path: &[0,], value: "hello 0", id: ElementId(3,) }, - ReplaceWith { id: ElementId(1,), m: 1 }, - ] - ); + { + dom.mark_dirty(ScopeId(0)); + let edits = dom.render_immediate().santize(); + assert_eq!( + edits.edits, + [ + LoadTemplate { name: "template", index: 0, id: ElementId(2,) }, + HydrateText { path: &[0,], value: "hello 0", id: ElementId(3,) }, + ReplaceWith { id: ElementId(1,), m: 1 }, + ] + ); + } - dom.mark_dirty(ScopeId(0)); - let edits = dom.render_immediate().santize(); - assert_eq!( - edits.edits, - [ - LoadTemplate { name: "template", index: 0, id: ElementId(1,) }, - HydrateText { path: &[0,], value: "hello 1", id: ElementId(4,) }, - LoadTemplate { name: "template", index: 0, id: ElementId(5,) }, - HydrateText { path: &[0,], value: "hello 2", id: ElementId(6,) }, - LoadTemplate { name: "template", index: 0, id: ElementId(7,) }, - HydrateText { path: &[0,], value: "hello 3", id: ElementId(8,) }, - LoadTemplate { name: "template", index: 0, id: ElementId(9,) }, - HydrateText { path: &[0,], value: "hello 4", id: ElementId(10,) }, - InsertAfter { id: ElementId(2,), m: 4 }, - ] - ); + { + dom.mark_dirty(ScopeId(0)); + let edits = dom.render_immediate().santize(); + assert_eq!( + edits.edits, + [ + LoadTemplate { name: "template", index: 0, id: ElementId(1,) }, + HydrateText { path: &[0,], value: "hello 1", id: ElementId(4,) }, + LoadTemplate { name: "template", index: 0, id: ElementId(5,) }, + HydrateText { path: &[0,], value: "hello 2", id: ElementId(6,) }, + LoadTemplate { name: "template", index: 0, id: ElementId(7,) }, + HydrateText { path: &[0,], value: "hello 3", id: ElementId(8,) }, + LoadTemplate { name: "template", index: 0, id: ElementId(9,) }, + HydrateText { path: &[0,], value: "hello 4", id: ElementId(10,) }, + InsertAfter { id: ElementId(2,), m: 4 }, + ] + ); + } - dom.mark_dirty(ScopeId(0)); - let edits = dom.render_immediate().santize(); - assert_eq!( - edits.edits, - [ - Remove { id: ElementId(9,) }, - Remove { id: ElementId(7,) }, - Remove { id: ElementId(5,) }, - Remove { id: ElementId(1,) }, - CreatePlaceholder { id: ElementId(3,) }, - ReplaceWith { id: ElementId(2,), m: 1 }, - ] - ); + { + dom.mark_dirty(ScopeId(0)); + let edits = dom.render_immediate().santize(); + assert_eq!( + edits.edits, + [ + Remove { id: ElementId(9,) }, + Remove { id: ElementId(7,) }, + Remove { id: ElementId(5,) }, + Remove { id: ElementId(1,) }, + CreatePlaceholder { id: ElementId(3,) }, + ReplaceWith { id: ElementId(2,), m: 1 }, + ] + ); + } - dom.mark_dirty(ScopeId(0)); - let edits = dom.render_immediate().santize(); - assert_eq!( - edits.edits, - [ - LoadTemplate { name: "template", index: 0, id: ElementId(2,) }, - HydrateText { path: &[0,], value: "hello 0", id: ElementId(1,) }, - ReplaceWith { id: ElementId(3,), m: 1 }, - ] - ) + { + dom.mark_dirty(ScopeId(0)); + let edits = dom.render_immediate().santize(); + assert_eq!( + edits.edits, + [ + LoadTemplate { name: "template", index: 0, id: ElementId(2,) }, + HydrateText { path: &[0,], value: "hello 0", id: ElementId(1,) }, + ReplaceWith { id: ElementId(3,), m: 1 }, + ] + ) + } } diff --git a/packages/core/tests/kitchen_sink.rs b/packages/core/tests/kitchen_sink.rs index 78ef608cc..1201f4e91 100644 --- a/packages/core/tests/kitchen_sink.rs +++ b/packages/core/tests/kitchen_sink.rs @@ -1,3 +1,4 @@ +use bumpalo::Bump; use dioxus::core::{ElementId, Mutation}; use dioxus::prelude::*; @@ -26,17 +27,22 @@ fn basic_syntax_is_a_template(cx: Scope) -> Element { #[test] fn dual_stream() { let mut dom = VirtualDom::new(basic_syntax_is_a_template); + let bump = Bump::new(); let edits = dom.rebuild().santize(); use Mutation::*; - assert_eq!( - edits.edits, + assert_eq!(edits.edits, { [ LoadTemplate { name: "template", index: 0, id: ElementId(1) }, - SetAttribute { name: "class", value: "123", id: ElementId(1), ns: None }, + SetAttribute { + name: "class", + value: "123".into_value(&bump), + id: ElementId(1), + ns: None, + }, NewEventListener { name: "click", scope: ScopeId(0), id: ElementId(1) }, HydrateText { path: &[0, 0], value: "123", id: ElementId(2) }, - AppendChildren { id: ElementId(0), m: 1 } - ], - ); + AppendChildren { id: ElementId(0), m: 1 }, + ] + }); } diff --git a/packages/core/tests/suspense.rs b/packages/core/tests/suspense.rs index 9fac13937..775ca3d71 100644 --- a/packages/core/tests/suspense.rs +++ b/packages/core/tests/suspense.rs @@ -9,32 +9,34 @@ use std::time::Duration; async fn it_works() { let mut dom = VirtualDom::new(app); - let mutations = dom.rebuild().santize(); + { + let mutations = dom.rebuild().santize(); - // We should at least get the top-level template in before pausing for the children - // note: we dont test template edits anymore - // assert_eq!( - // mutations.templates, - // [ - // CreateElement { name: "div" }, - // CreateStaticText { value: "Waiting for child..." }, - // CreateStaticPlaceholder, - // AppendChildren { m: 2 }, - // SaveTemplate { name: "template", m: 1 } - // ] - // ); + // We should at least get the top-level template in before pausing for the children + // note: we dont test template edits anymore + // assert_eq!( + // mutations.templates, + // [ + // CreateElement { name: "div" }, + // CreateStaticText { value: "Waiting for child..." }, + // CreateStaticPlaceholder, + // AppendChildren { m: 2 }, + // SaveTemplate { name: "template", m: 1 } + // ] + // ); - // And we should load it in and assign the placeholder properly - assert_eq!( - mutations.edits, - [ - LoadTemplate { name: "template", index: 0, id: ElementId(1) }, - // hmmmmmmmmm.... with suspense how do we guarantee that IDs increase linearly? - // can we even? - AssignId { path: &[1], id: ElementId(3) }, - AppendChildren { m: 1, id: ElementId(0) }, - ] - ); + // And we should load it in and assign the placeholder properly + assert_eq!( + mutations.edits, + [ + LoadTemplate { name: "template", index: 0, id: ElementId(1) }, + // hmmmmmmmmm.... with suspense how do we guarantee that IDs increase linearly? + // can we even? + AssignId { path: &[1], id: ElementId(3) }, + AppendChildren { m: 1, id: ElementId(0) }, + ] + ); + } // wait just a moment, not enough time for the boundary to resolve diff --git a/packages/dioxus/benches/jsframework.rs b/packages/dioxus/benches/jsframework.rs index 24ca2fe69..b3405b2bd 100644 --- a/packages/dioxus/benches/jsframework.rs +++ b/packages/dioxus/benches/jsframework.rs @@ -42,7 +42,7 @@ fn create_rows(c: &mut Criterion) { c.bench_function("create rows", |b| { let mut dom = VirtualDom::new(app); - dom.rebuild(); + let _ = dom.rebuild(); b.iter(|| { let g = dom.rebuild(); diff --git a/packages/native-core-macro/Cargo.toml b/packages/native-core-macro/Cargo.toml index bff83ec42..759d8ff00 100644 --- a/packages/native-core-macro/Cargo.toml +++ b/packages/native-core-macro/Cargo.toml @@ -18,10 +18,9 @@ proc-macro = true syn = { version = "1.0.11", features = ["extra-traits"] } quote = "1.0" dioxus-native-core = { path = "../native-core", version = "^0.2.0" } - -[dev-dependencies] dioxus = { path = "../dioxus" } +[dev-dependencies] smallvec = "1.6" rustc-hash = "1.1.0" anymap = "0.12.1" diff --git a/packages/native-core-macro/src/lib.rs b/packages/native-core-macro/src/lib.rs index dda96b512..5bf50afcc 100644 --- a/packages/native-core-macro/src/lib.rs +++ b/packages/native-core-macro/src/lib.rs @@ -306,8 +306,7 @@ impl Member { + field.ty.to_token_stream().to_string().as_str()) .as_str(), Span::call_site(), - ) - .into(), + ), ident: field.ident.as_ref()?.clone(), }) } diff --git a/packages/native-core/src/node.rs b/packages/native-core/src/node.rs index 4530fd350..7220e0fab 100644 --- a/packages/native-core/src/node.rs +++ b/packages/native-core/src/node.rs @@ -1,6 +1,7 @@ use crate::{state::State, tree::NodeId}; -use dioxus_core::ElementId; +use dioxus_core::{AnyValueBox, AttributeValue, ElementId}; use rustc_hash::{FxHashMap, FxHashSet}; +use std::fmt::Debug; /// The node is stored client side and stores only basic data about the node. #[derive(Debug, Clone)] @@ -72,15 +73,43 @@ pub struct OwnedAttributeView<'a> { pub value: &'a OwnedAttributeValue, } -#[derive(Clone, Debug)] +#[derive(Clone)] pub enum OwnedAttributeValue { Text(String), - Float(f32), - Int(i32), + Float(f64), + Int(i64), Bool(bool), + Any(AnyValueBox), None, } +impl Debug for OwnedAttributeValue { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Text(arg0) => f.debug_tuple("Text").field(arg0).finish(), + Self::Float(arg0) => f.debug_tuple("Float").field(arg0).finish(), + Self::Int(arg0) => f.debug_tuple("Int").field(arg0).finish(), + Self::Bool(arg0) => f.debug_tuple("Bool").field(arg0).finish(), + Self::Any(_) => f.debug_tuple("Any").finish(), + Self::None => write!(f, "None"), + } + } +} + +impl From> for OwnedAttributeValue { + fn from(value: AttributeValue<'_>) -> Self { + match value { + AttributeValue::Text(text) => Self::Text(text.to_string()), + AttributeValue::Float(float) => Self::Float(float), + AttributeValue::Int(int) => Self::Int(int), + AttributeValue::Bool(bool) => Self::Bool(bool), + AttributeValue::Any(any) => Self::Any(any), + AttributeValue::None => Self::None, + _ => Self::None, + } + } +} + impl OwnedAttributeValue { pub fn as_text(&self) -> Option<&str> { match self { @@ -89,14 +118,14 @@ impl OwnedAttributeValue { } } - pub fn as_float(&self) -> Option { + pub fn as_float(&self) -> Option { match self { OwnedAttributeValue::Float(float) => Some(*float), _ => None, } } - pub fn as_int(&self) -> Option { + pub fn as_int(&self) -> Option { match self { OwnedAttributeValue::Int(int) => Some(*int), _ => None, diff --git a/packages/native-core/src/real_dom.rs b/packages/native-core/src/real_dom.rs index 7d56b0dca..637a8d7ce 100644 --- a/packages/native-core/src/real_dom.rs +++ b/packages/native-core/src/real_dom.rs @@ -102,11 +102,7 @@ impl RealDom { self.tree.add_child(node_id, child_id); } - fn create_template_node( - &mut self, - node: &TemplateNode, - mutations_vec: &mut FxHashMap, - ) -> RealNodeId { + fn create_template_node(&mut self, node: &TemplateNode) -> RealNodeId { match node { TemplateNode::Element { tag, @@ -139,27 +135,18 @@ impl RealDom { }); let node_id = self.create_node(node); for child in *children { - let child_id = self.create_template_node(child, mutations_vec); + let child_id = self.create_template_node(child); self.add_child(node_id, child_id); } node_id } - TemplateNode::Text { text } => { - let node_id = self.create_node(Node::new(NodeType::Text { - text: text.to_string(), - })); - node_id - } - TemplateNode::Dynamic { .. } => { - let node_id = self.create_node(Node::new(NodeType::Placeholder)); - node_id - } - TemplateNode::DynamicText { .. } => { - let node_id = self.create_node(Node::new(NodeType::Text { - text: String::new(), - })); - node_id - } + TemplateNode::Text { text } => self.create_node(Node::new(NodeType::Text { + text: text.to_string(), + })), + TemplateNode::Dynamic { .. } => self.create_node(Node::new(NodeType::Placeholder)), + TemplateNode::DynamicText { .. } => self.create_node(Node::new(NodeType::Text { + text: String::new(), + })), } } @@ -172,7 +159,7 @@ impl RealDom { for template in mutations.templates { let mut template_root_ids = Vec::new(); for root in template.roots { - let id = self.create_template_node(root, &mut nodes_updated); + let id = self.create_template_node(root); template_root_ids.push(id); } self.templates @@ -283,26 +270,7 @@ impl RealDom { namespace: ns.map(|s| s.to_string()), volatile: false, }, - crate::node::OwnedAttributeValue::Text(value.to_string()), - ); - mark_dirty( - node_id, - NodeMask::new_with_attrs(AttributeMask::single(name)), - &mut nodes_updated, - ); - } - } - SetBoolAttribute { name, value, id } => { - let node_id = self.element_to_node_id(id); - let node = self.tree.get_mut(node_id).unwrap(); - if let NodeType::Element { attributes, .. } = &mut node.node_data.node_type { - attributes.insert( - OwnedAttributeDiscription { - name: name.to_string(), - namespace: None, - volatile: false, - }, - crate::node::OwnedAttributeValue::Bool(value), + OwnedAttributeValue::from(value), ); mark_dirty( node_id, diff --git a/packages/rsx/src/element.rs b/packages/rsx/src/element.rs index 307059f9d..88699b79e 100644 --- a/packages/rsx/src/element.rs +++ b/packages/rsx/src/element.rs @@ -211,7 +211,7 @@ pub enum ElementAttr { } impl ElementAttr { - pub fn flart(&self) -> Span { + pub fn start(&self) -> Span { match self { ElementAttr::AttrText { name, .. } => name.span(), ElementAttr::AttrExpression { name, .. } => name.span(), @@ -265,7 +265,7 @@ impl ToTokens for ElementAttrNamed { ElementAttr::CustomAttrText { name, value } => { quote! { __cx.attr( - dioxus_elements::#el_name::#name.0, + #name, #value, None, false @@ -275,7 +275,7 @@ impl ToTokens for ElementAttrNamed { ElementAttr::CustomAttrExpression { name, value } => { quote! { __cx.attr( - dioxus_elements::#el_name::#name.0, + #name, #value, None, false diff --git a/packages/tui/src/focus.rs b/packages/tui/src/focus.rs index 90d9e9da0..57f1c9e1f 100644 --- a/packages/tui/src/focus.rs +++ b/packages/tui/src/focus.rs @@ -79,7 +79,7 @@ impl NodeDepState for Focus { if let Some(index) = a .value .as_int() - .or_else(|| a.value.as_text().and_then(|v| v.parse::().ok())) + .or_else(|| a.value.as_text().and_then(|v| v.parse::().ok())) { match index.cmp(&0) { Ordering::Less => FocusLevel::Unfocusable, diff --git a/packages/web/src/dom.rs b/packages/web/src/dom.rs index 0cdf46b3e..96d95bc89 100644 --- a/packages/web/src/dom.rs +++ b/packages/web/src/dom.rs @@ -75,10 +75,21 @@ impl WebsysDom { value, id, ns, - } => i.SetAttribute(id.0 as u32, name, value.into(), ns), - SetBoolAttribute { name, value, id } => { - i.SetBoolAttribute(id.0 as u32, name, value) - } + } => match value { + dioxus_core::AttributeValue::Text(txt) => { + i.SetAttribute(id.0 as u32, name, txt.into(), ns) + } + dioxus_core::AttributeValue::Float(f) => { + i.SetAttribute(id.0 as u32, name, f.into(), ns) + } + dioxus_core::AttributeValue::Int(n) => { + i.SetAttribute(id.0 as u32, name, n.into(), ns) + } + dioxus_core::AttributeValue::Bool(b) => { + i.SetBoolAttribute(id.0 as u32, name, b) + } + _ => unreachable!(), + }, SetText { value, id } => i.SetText(id.0 as u32, value.into()), NewEventListener { name, id, .. } => { self.interpreter.NewEventListener( diff --git a/packages/web/src/lib.rs b/packages/web/src/lib.rs index 62a2bf4dd..cf206b698 100644 --- a/packages/web/src/lib.rs +++ b/packages/web/src/lib.rs @@ -187,10 +187,12 @@ pub async fn run_with_props(root: fn(Scope) -> Element, root_prop // if should_hydrate { // } else { - let edits = dom.rebuild(); + { + let edits = dom.rebuild(); - websys_dom.load_templates(&edits.templates); - websys_dom.apply_edits(edits.edits); + websys_dom.load_templates(&edits.templates); + websys_dom.apply_edits(edits.edits); + } // the mutations come back with nothing - we need to actually mount them websys_dom.mount(); From 0813d531f157f87e479bb9a1416d57a7fa4feb3b Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Sat, 10 Dec 2022 12:50:15 -0600 Subject: [PATCH 02/25] add some utilities around AnyValueRc --- packages/core/src/lib.rs | 2 +- packages/core/src/nodes.rs | 49 ++++++++++++++++++++++++++++---- packages/native-core/src/node.rs | 4 +-- packages/tui/src/lib.rs | 4 +-- packages/tui/src/query.rs | 2 +- 5 files changed, 49 insertions(+), 12 deletions(-) diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index 0de74c911..fd8a934f6 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -70,7 +70,7 @@ pub(crate) mod innerlude { } pub use crate::innerlude::{ - fc_to_builder, AnyValueBox, Attribute, AttributeValue, Component, DynamicNode, Element, + fc_to_builder, AnyValueRc, Attribute, AttributeValue, Component, DynamicNode, Element, ElementId, Event, Fragment, IntoAttributeValue, IntoDynNode, LazyNodes, Mutation, Mutations, Properties, RenderReturn, Scope, ScopeId, ScopeState, Scoped, SuspenseContext, TaskId, Template, TemplateAttribute, TemplateNode, VComponent, VNode, VText, VirtualDom, diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index c776da967..62b88ac83 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -338,7 +338,7 @@ pub enum AttributeValue<'a> { Listener(ListenerCb<'a>), /// An arbitrary value that implements PartialEq and is static - Any(AnyValueBox), + Any(AnyValueRc), /// A "none" value, resulting in the removal of an attribute from the dom None, @@ -375,10 +375,47 @@ impl<'de, 'a> serde::Deserialize<'de> for ListenerCb<'a> { /// A boxed value that implements PartialEq and Any #[derive(Clone)] -pub struct AnyValueBox(pub Rc); +pub struct AnyValueRc(pub Rc); + +impl PartialEq for AnyValueRc { + fn eq(&self, other: &Self) -> bool { + self.0.any_cmp(other.0.as_ref()) + } +} + +impl AnyValueRc { + /// Returns a reference to the inner value without checking the type. + pub fn downcast_ref_unchecked(&self) -> &T { + unsafe { &*(self.0.as_ref() as *const dyn AnyValue as *const T) } + } + + /// Returns a reference to the inner value. + pub fn downcast_ref(&self) -> Option<&T> { + if self.0.our_typeid() == TypeId::of::() { + Some(self.downcast_ref_unchecked()) + } else { + None + } + } + + /// Checks if the inner value is of type `T`. + pub fn is(&self) -> bool { + self.0.our_typeid() == TypeId::of::() + } +} + +#[test] +fn test_any_value_rc() { + let a = AnyValueRc(Rc::new(1i32)); + assert!(a.is::()); + assert!(!a.is::()); + assert_eq!(a.downcast_ref::(), Some(&1i32)); + assert_eq!(a.downcast_ref::(), None); + assert_eq!(a.downcast_ref_unchecked::(), &1i32); +} #[cfg(feature = "serialize")] -impl serde::Serialize for AnyValueBox { +impl serde::Serialize for AnyValueRc { fn serialize(&self, _: S) -> Result where S: serde::Serializer, @@ -388,7 +425,7 @@ impl serde::Serialize for AnyValueBox { } #[cfg(feature = "serialize")] -impl<'de> serde::Deserialize<'de> for AnyValueBox { +impl<'de> serde::Deserialize<'de> for AnyValueRc { fn deserialize(_: D) -> Result where D: serde::Deserializer<'de>, @@ -419,7 +456,7 @@ impl<'a> PartialEq for AttributeValue<'a> { (Self::Int(l0), Self::Int(r0)) => l0 == r0, (Self::Bool(l0), Self::Bool(r0)) => l0 == r0, (Self::Listener(_), Self::Listener(_)) => true, - (Self::Any(l0), Self::Any(r0)) => l0.0.any_cmp(r0.0.as_ref()), + (Self::Any(l0), Self::Any(r0)) => l0 == r0, _ => false, } } @@ -642,7 +679,7 @@ impl<'a> IntoAttributeValue<'a> for Arguments<'_> { } } -impl<'a> IntoAttributeValue<'a> for AnyValueBox { +impl<'a> IntoAttributeValue<'a> for AnyValueRc { fn into_value(self, _: &'a Bump) -> AttributeValue<'a> { AttributeValue::Any(self) } diff --git a/packages/native-core/src/node.rs b/packages/native-core/src/node.rs index 7220e0fab..6ee7d7184 100644 --- a/packages/native-core/src/node.rs +++ b/packages/native-core/src/node.rs @@ -1,5 +1,5 @@ use crate::{state::State, tree::NodeId}; -use dioxus_core::{AnyValueBox, AttributeValue, ElementId}; +use dioxus_core::{AnyValueRc, AttributeValue, ElementId}; use rustc_hash::{FxHashMap, FxHashSet}; use std::fmt::Debug; @@ -79,7 +79,7 @@ pub enum OwnedAttributeValue { Float(f64), Int(i64), Bool(bool), - Any(AnyValueBox), + Any(AnyValueRc), None, } diff --git a/packages/tui/src/lib.rs b/packages/tui/src/lib.rs index 38638f7c5..e60988a24 100644 --- a/packages/tui/src/lib.rs +++ b/packages/tui/src/lib.rs @@ -185,9 +185,9 @@ fn render_vdom( let rdom = rdom.borrow(); let mut taffy = taffy.lock().expect("taffy lock poisoned"); // size is guaranteed to not change when rendering - resize(frame.size(), &mut *taffy, &rdom); + resize(frame.size(), &mut taffy, &rdom); let root = &rdom[NodeId(0)]; - render::render_vnode(frame, &*taffy, &rdom, root, cfg, Point::ZERO); + render::render_vnode(frame, &taffy, &rdom, root, cfg, Point::ZERO); })?; } else { let rdom = rdom.borrow(); diff --git a/packages/tui/src/query.rs b/packages/tui/src/query.rs index 64da87650..c052ad9ea 100644 --- a/packages/tui/src/query.rs +++ b/packages/tui/src/query.rs @@ -89,7 +89,7 @@ impl<'a> ElementRef<'a> { .ok(); layout.map(|layout| Layout { order: layout.order, - size: layout.size.map(|v| layout_to_screen_space(v)), + size: layout.size.map(layout_to_screen_space), location: Point { x: layout_to_screen_space(layout.location.x), y: layout_to_screen_space(layout.location.y), From e4a7e4582bc29f5ac038f24b43724f3fffef0fba Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Sat, 10 Dec 2022 12:59:12 -0600 Subject: [PATCH 03/25] make downcast_ref_unchecked unsafe --- packages/core/src/nodes.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 62b88ac83..d00825c34 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -385,14 +385,17 @@ impl PartialEq for AnyValueRc { impl AnyValueRc { /// Returns a reference to the inner value without checking the type. - pub fn downcast_ref_unchecked(&self) -> &T { + /// + /// # Safety + /// The caller must ensure that the type of the inner value is `T`. + pub unsafe fn downcast_ref_unchecked(&self) -> &T { unsafe { &*(self.0.as_ref() as *const dyn AnyValue as *const T) } } /// Returns a reference to the inner value. pub fn downcast_ref(&self) -> Option<&T> { if self.0.our_typeid() == TypeId::of::() { - Some(self.downcast_ref_unchecked()) + Some(unsafe { self.downcast_ref_unchecked() }) } else { None } @@ -411,7 +414,9 @@ fn test_any_value_rc() { assert!(!a.is::()); assert_eq!(a.downcast_ref::(), Some(&1i32)); assert_eq!(a.downcast_ref::(), None); - assert_eq!(a.downcast_ref_unchecked::(), &1i32); + unsafe { + assert_eq!(a.downcast_ref_unchecked::(), &1i32); + } } #[cfg(feature = "serialize")] From 0ac02cc89391f6067ab5e95f82dee67d0dd9ac7f Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Sat, 10 Dec 2022 13:24:39 -0600 Subject: [PATCH 04/25] move any bound to trait instead of implementation --- packages/core/src/nodes.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index d00825c34..3e8638a84 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -394,7 +394,7 @@ impl AnyValueRc { /// Returns a reference to the inner value. pub fn downcast_ref(&self) -> Option<&T> { - if self.0.our_typeid() == TypeId::of::() { + if self.0.type_id() == TypeId::of::() { Some(unsafe { self.downcast_ref_unchecked() }) } else { None @@ -403,7 +403,7 @@ impl AnyValueRc { /// Checks if the inner value is of type `T`. pub fn is(&self) -> bool { - self.0.our_typeid() == TypeId::of::() + self.0.type_id() == TypeId::of::() } } @@ -468,23 +468,18 @@ impl<'a> PartialEq for AttributeValue<'a> { } #[doc(hidden)] -pub trait AnyValue { +pub trait AnyValue: Any { fn any_cmp(&self, other: &dyn AnyValue) -> bool; - fn our_typeid(&self) -> TypeId; } impl AnyValue for T { fn any_cmp(&self, other: &dyn AnyValue) -> bool { - if self.type_id() != other.our_typeid() { + if self.type_id() != other.type_id() { return false; } self == unsafe { &*(other as *const _ as *const T) } } - - fn our_typeid(&self) -> TypeId { - self.type_id() - } } #[doc(hidden)] From 74e9d5977aee111d00ef37967f0c66f9f6068e42 Mon Sep 17 00:00:00 2001 From: = Date: Sun, 11 Dec 2022 17:31:15 -0600 Subject: [PATCH 05/25] add sync behind a feature flag --- packages/core/Cargo.toml | 1 + packages/core/src/lib.rs | 8 ++-- packages/core/src/nodes.rs | 68 ++++++++++++++++++++-------- packages/native-core/Cargo.toml | 6 ++- packages/native-core/src/node.rs | 4 +- packages/native-core/src/passes.rs | 46 ++++++++++++++++++- packages/native-core/src/real_dom.rs | 19 +++++++- packages/native-core/src/state.rs | 14 +++++- packages/native-core/src/tree.rs | 4 +- 9 files changed, 137 insertions(+), 33 deletions(-) diff --git a/packages/core/Cargo.toml b/packages/core/Cargo.toml index 827968655..6d4ed51ef 100644 --- a/packages/core/Cargo.toml +++ b/packages/core/Cargo.toml @@ -44,3 +44,4 @@ dioxus = { path = "../dioxus" } [features] default = [] serialize = ["serde"] +sync_attributes = [] \ No newline at end of file diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index fd8a934f6..d15d7a032 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -70,10 +70,10 @@ pub(crate) mod innerlude { } pub use crate::innerlude::{ - fc_to_builder, AnyValueRc, Attribute, AttributeValue, Component, DynamicNode, Element, - ElementId, Event, Fragment, IntoAttributeValue, IntoDynNode, LazyNodes, Mutation, Mutations, - Properties, RenderReturn, Scope, ScopeId, ScopeState, Scoped, SuspenseContext, TaskId, - Template, TemplateAttribute, TemplateNode, VComponent, VNode, VText, VirtualDom, + fc_to_builder, AnyValue, AnyValueContainer, Attribute, AttributeValue, Component, DynamicNode, + Element, ElementId, Event, Fragment, IntoAttributeValue, IntoDynNode, LazyNodes, Mutation, + Mutations, Properties, RenderReturn, Scope, ScopeId, ScopeState, Scoped, SuspenseContext, + TaskId, Template, TemplateAttribute, TemplateNode, VComponent, VNode, VText, VirtualDom, }; /// The purpose of this module is to alleviate imports of many common types diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 3e8638a84..909ced495 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -338,7 +338,7 @@ pub enum AttributeValue<'a> { Listener(ListenerCb<'a>), /// An arbitrary value that implements PartialEq and is static - Any(AnyValueRc), + Any(AnyValueContainer), /// A "none" value, resulting in the removal of an attribute from the dom None, @@ -375,26 +375,38 @@ impl<'de, 'a> serde::Deserialize<'de> for ListenerCb<'a> { /// A boxed value that implements PartialEq and Any #[derive(Clone)] -pub struct AnyValueRc(pub Rc); +#[cfg(not(feature = "sync_attributes"))] +pub struct AnyValueContainer(pub Rc); -impl PartialEq for AnyValueRc { +#[derive(Clone)] +#[cfg(feature = "sync_attributes")] +pub struct AnyValueContainer(pub std::sync::Arc); + +impl PartialEq for AnyValueContainer { fn eq(&self, other: &Self) -> bool { self.0.any_cmp(other.0.as_ref()) } } -impl AnyValueRc { +impl AnyValueContainer { + pub fn new(value: T) -> Self { + #[cfg(feature = "sync_attributes")] + return Self(std::sync::Arc::new(value)); + #[cfg(not(feature = "sync_attributes"))] + return Self(Rc::new(value)); + } + /// Returns a reference to the inner value without checking the type. /// /// # Safety /// The caller must ensure that the type of the inner value is `T`. - pub unsafe fn downcast_ref_unchecked(&self) -> &T { - unsafe { &*(self.0.as_ref() as *const dyn AnyValue as *const T) } + pub unsafe fn downcast_ref_unchecked(&self) -> &T { + unsafe { &*(self.0.as_ref() as *const _ as *const T) } } /// Returns a reference to the inner value. - pub fn downcast_ref(&self) -> Option<&T> { - if self.0.type_id() == TypeId::of::() { + pub fn downcast_ref(&self) -> Option<&T> { + if self.0.our_typeid() == TypeId::of::() { Some(unsafe { self.downcast_ref_unchecked() }) } else { None @@ -402,25 +414,25 @@ impl AnyValueRc { } /// Checks if the inner value is of type `T`. - pub fn is(&self) -> bool { - self.0.type_id() == TypeId::of::() + pub fn is(&self) -> bool { + self.0.our_typeid() == TypeId::of::() } } #[test] fn test_any_value_rc() { - let a = AnyValueRc(Rc::new(1i32)); - assert!(a.is::()); - assert!(!a.is::()); + let a = AnyValueContainer::new(1i32); assert_eq!(a.downcast_ref::(), Some(&1i32)); assert_eq!(a.downcast_ref::(), None); + assert!(a.is::()); + assert!(!a.is::()); unsafe { assert_eq!(a.downcast_ref_unchecked::(), &1i32); } } #[cfg(feature = "serialize")] -impl serde::Serialize for AnyValueRc { +impl serde::Serialize for AnyValueContainer { fn serialize(&self, _: S) -> Result where S: serde::Serializer, @@ -430,7 +442,7 @@ impl serde::Serialize for AnyValueRc { } #[cfg(feature = "serialize")] -impl<'de> serde::Deserialize<'de> for AnyValueRc { +impl<'de> serde::Deserialize<'de> for AnyValueContainer { fn deserialize(_: D) -> Result where D: serde::Deserializer<'de>, @@ -467,19 +479,37 @@ impl<'a> PartialEq for AttributeValue<'a> { } } +#[cfg(feature = "sync_attributes")] +pub trait AnyValueBounds: Any + PartialEq + Sync {} +#[cfg(not(feature = "sync_attributes"))] +pub trait AnyValueBounds: Any + PartialEq {} + #[doc(hidden)] -pub trait AnyValue: Any { +#[cfg(feature = "sync_attributes")] +pub trait AnyValue: Sync { fn any_cmp(&self, other: &dyn AnyValue) -> bool; + fn our_typeid(&self) -> TypeId; } -impl AnyValue for T { +impl AnyValue for T { fn any_cmp(&self, other: &dyn AnyValue) -> bool { - if self.type_id() != other.type_id() { + if self.our_typeid() != other.our_typeid() { return false; } self == unsafe { &*(other as *const _ as *const T) } } + + fn our_typeid(&self) -> TypeId { + self.type_id() + } +} + +#[doc(hidden)] +#[cfg(not(feature = "sync_attributes"))] +pub trait AnyValue { + fn any_cmp(&self, other: &dyn AnyValue) -> bool; + fn our_typeid(&self) -> TypeId; } #[doc(hidden)] @@ -679,7 +709,7 @@ impl<'a> IntoAttributeValue<'a> for Arguments<'_> { } } -impl<'a> IntoAttributeValue<'a> for AnyValueRc { +impl<'a> IntoAttributeValue<'a> for AnyValueContainer { fn into_value(self, _: &'a Bump) -> AttributeValue<'a> { AttributeValue::Any(self) } diff --git a/packages/native-core/Cargo.toml b/packages/native-core/Cargo.toml index 309fba781..057317779 100644 --- a/packages/native-core/Cargo.toml +++ b/packages/native-core/Cargo.toml @@ -26,4 +26,8 @@ dashmap = "5.4.0" [dev-dependencies] rand = "0.8.5" -dioxus = { path = "../dioxus", version = "^0.2.1" } \ No newline at end of file +dioxus = { path = "../dioxus", version = "^0.2.1" } + +[features] +default = ["parallel"] +parallel = ["dioxus-core/sync_attributes"] \ No newline at end of file diff --git a/packages/native-core/src/node.rs b/packages/native-core/src/node.rs index 6ee7d7184..c471dba11 100644 --- a/packages/native-core/src/node.rs +++ b/packages/native-core/src/node.rs @@ -1,5 +1,5 @@ use crate::{state::State, tree::NodeId}; -use dioxus_core::{AnyValueRc, AttributeValue, ElementId}; +use dioxus_core::{AnyValueContainer, AttributeValue, ElementId}; use rustc_hash::{FxHashMap, FxHashSet}; use std::fmt::Debug; @@ -79,7 +79,7 @@ pub enum OwnedAttributeValue { Float(f64), Int(i64), Bool(bool), - Any(AnyValueRc), + Any(AnyValueContainer), None, } diff --git a/packages/native-core/src/passes.rs b/packages/native-core/src/passes.rs index bdc7c7e5b..fd15e2ae6 100644 --- a/packages/native-core/src/passes.rs +++ b/packages/native-core/src/passes.rs @@ -314,7 +314,7 @@ struct RawPointer(*mut T); unsafe impl Send for RawPointer {} unsafe impl Sync for RawPointer {} -pub fn resolve_passes>( +pub fn resolve_passes + Sync>( tree: &mut Tr, dirty_nodes: DirtyNodeStates, mut passes: Vec<&AnyPass>, @@ -368,6 +368,50 @@ pub fn resolve_passes>( std::sync::Arc::try_unwrap(nodes_updated).unwrap() } +pub fn resolve_passes_single_threaded>( + tree: &mut Tr, + dirty_nodes: DirtyNodeStates, + mut passes: Vec<&AnyPass>, + ctx: SendAnyMap, +) -> FxDashSet { + let dirty_states = Arc::new(dirty_nodes); + let mut resolved_passes: FxHashSet = FxHashSet::default(); + let mut resolving = Vec::new(); + let nodes_updated = Arc::new(FxDashSet::default()); + let ctx = Arc::new(ctx); + while !passes.is_empty() { + let mut currently_borrowed = MemberMask::default(); + let mut i = 0; + while i < passes.len() { + let pass = &passes[i]; + let pass_id = pass.pass_id(); + let pass_mask = pass.mask(); + if pass + .dependancies() + .iter() + .all(|d| resolved_passes.contains(d) || *d == pass_id) + && !pass_mask.overlaps(currently_borrowed) + { + let pass = passes.remove(i); + resolving.push(pass_id); + currently_borrowed |= pass_mask; + let dirty_states = dirty_states.clone(); + let nodes_updated = nodes_updated.clone(); + let ctx = ctx.clone(); + // this is safe because the member_mask acts as a per-member mutex and we have verified that the pass does not overlap with any other pass + let mut dirty = DirtyNodes::default(); + dirty_states.all_dirty(pass_id, &mut dirty, tree); + pass.resolve(tree, dirty, &dirty_states, &nodes_updated, &ctx); + } else { + i += 1; + } + } + resolved_passes.extend(resolving.iter().copied()); + resolving.clear() + } + std::sync::Arc::try_unwrap(nodes_updated).unwrap() +} + #[test] fn node_pass() { use crate::tree::{Tree, TreeLike}; diff --git a/packages/native-core/src/real_dom.rs b/packages/native-core/src/real_dom.rs index 637a8d7ce..5fe3113ce 100644 --- a/packages/native-core/src/real_dom.rs +++ b/packages/native-core/src/real_dom.rs @@ -339,12 +339,12 @@ impl RealDom { } /// Update the state of the dom, after appling some mutations. This will keep the nodes in the dom up to date with their VNode counterparts. - pub fn update_state( + pub fn update_state_single_threaded( &mut self, nodes_updated: DirtyNodeStates, ctx: SendAnyMap, ) -> FxDashSet { - S::update(nodes_updated, &mut self.tree, ctx) + S::update_single_threaded(nodes_updated, &mut self.tree, ctx) } /// Find all nodes that are listening for an event, sorted by there height in the dom progressing starting at the bottom and progressing up. @@ -396,6 +396,21 @@ impl RealDom { } } +impl RealDom +where + Tree>: Sync, +{ + /// Update the state of the dom, after appling some mutations. This will keep the nodes in the dom up to date with their VNode counterparts. + /// This will resolve the state in parallel + pub fn update_state( + &mut self, + nodes_updated: DirtyNodeStates, + ctx: SendAnyMap, + ) -> FxDashSet { + S::update(nodes_updated, &mut self.tree, ctx) + } +} + impl Deref for RealDom { type Target = Tree>; diff --git a/packages/native-core/src/state.rs b/packages/native-core/src/state.rs index 7e6c5266a..54dad4997 100644 --- a/packages/native-core/src/state.rs +++ b/packages/native-core/src/state.rs @@ -2,7 +2,7 @@ use std::cmp::Ordering; use crate::node::Node; use crate::node_ref::{NodeMask, NodeView}; -use crate::passes::{resolve_passes, AnyPass, DirtyNodeStates}; +use crate::passes::{resolve_passes, resolve_passes_single_threaded, AnyPass, DirtyNodeStates}; use crate::tree::TreeView; use crate::{FxDashSet, RealNodeId, SendAnyMap}; @@ -217,7 +217,7 @@ pub trait State: Default + Clone + 'static { const MASKS: &'static [NodeMask]; #[doc(hidden)] - fn update>>( + fn update> + Sync>( dirty: DirtyNodeStates, tree: &mut T, ctx: SendAnyMap, @@ -225,6 +225,16 @@ pub trait State: Default + Clone + 'static { let passes = Self::PASSES.iter().collect(); resolve_passes(tree, dirty, passes, ctx) } + + #[doc(hidden)] + fn update_single_threaded>>( + dirty: DirtyNodeStates, + tree: &mut T, + ctx: SendAnyMap, + ) -> FxDashSet { + let passes = Self::PASSES.iter().collect(); + resolve_passes_single_threaded(tree, dirty, passes, ctx) + } } impl ChildDepState for () { diff --git a/packages/native-core/src/tree.rs b/packages/native-core/src/tree.rs index 0547fc3a9..c173824cb 100644 --- a/packages/native-core/src/tree.rs +++ b/packages/native-core/src/tree.rs @@ -564,8 +564,8 @@ impl<'a, T, Tr: TreeView> SharedView<'a, T, Tr> { } } -unsafe impl<'a, T, Tr: TreeView> Send for SharedView<'a, T, Tr> {} -unsafe impl<'a, T, Tr: TreeView> Sync for SharedView<'a, T, Tr> {} +unsafe impl<'a, T, Tr: TreeView + Send> Send for SharedView<'a, T, Tr> {} +unsafe impl<'a, T, Tr: TreeView + Sync> Sync for SharedView<'a, T, Tr> {} impl<'a, T, Tr: TreeView> Clone for SharedView<'a, T, Tr> { fn clone(&self) -> Self { Self { From 2dee361b6069abc095764eb9859dead9fde95a85 Mon Sep 17 00:00:00 2001 From: = Date: Sun, 11 Dec 2022 21:25:55 -0600 Subject: [PATCH 06/25] clean up features flags and AnyValueContainer --- packages/core/src/nodes.rs | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 909ced495..cc1c5f907 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -8,7 +8,6 @@ use std::{ cell::{Cell, RefCell}, fmt::Arguments, future::Future, - rc::Rc, }; pub type TemplateId = &'static str; @@ -376,10 +375,11 @@ impl<'de, 'a> serde::Deserialize<'de> for ListenerCb<'a> { /// A boxed value that implements PartialEq and Any #[derive(Clone)] #[cfg(not(feature = "sync_attributes"))] -pub struct AnyValueContainer(pub Rc); +pub struct AnyValueContainer(pub std::rc::Rc); #[derive(Clone)] #[cfg(feature = "sync_attributes")] +/// A boxed value that implements PartialEq, Any, Sync, and Send pub struct AnyValueContainer(pub std::sync::Arc); impl PartialEq for AnyValueContainer { @@ -389,11 +389,12 @@ impl PartialEq for AnyValueContainer { } impl AnyValueContainer { + /// Create a new AnyValueContainer containing the specified data. pub fn new(value: T) -> Self { #[cfg(feature = "sync_attributes")] return Self(std::sync::Arc::new(value)); #[cfg(not(feature = "sync_attributes"))] - return Self(Rc::new(value)); + return Self(std::rc::Rc::new(value)); } /// Returns a reference to the inner value without checking the type. @@ -480,13 +481,24 @@ impl<'a> PartialEq for AttributeValue<'a> { } #[cfg(feature = "sync_attributes")] -pub trait AnyValueBounds: Any + PartialEq + Sync {} +pub trait AnyValueBounds: Any + PartialEq + Sync + Send {} +#[cfg(feature = "sync_attributes")] +impl AnyValueBounds for T {} #[cfg(not(feature = "sync_attributes"))] pub trait AnyValueBounds: Any + PartialEq {} +#[cfg(not(feature = "sync_attributes"))] +impl AnyValueBounds for T {} #[doc(hidden)] #[cfg(feature = "sync_attributes")] -pub trait AnyValue: Sync { +pub trait AnyValue: Sync + Send { + fn any_cmp(&self, other: &dyn AnyValue) -> bool; + fn our_typeid(&self) -> TypeId; +} + +#[doc(hidden)] +#[cfg(not(feature = "sync_attributes"))] +pub trait AnyValue { fn any_cmp(&self, other: &dyn AnyValue) -> bool; fn our_typeid(&self) -> TypeId; } @@ -505,13 +517,6 @@ impl AnyValue for T { } } -#[doc(hidden)] -#[cfg(not(feature = "sync_attributes"))] -pub trait AnyValue { - fn any_cmp(&self, other: &dyn AnyValue) -> bool; - fn our_typeid(&self) -> TypeId; -} - #[doc(hidden)] pub trait ComponentReturn<'a, A = ()> { fn into_return(self, cx: &'a ScopeState) -> RenderReturn<'a>; From 53b436c684325567650edda2ac9f971a9d4e286e Mon Sep 17 00:00:00 2001 From: = Date: Sun, 11 Dec 2022 21:40:40 -0600 Subject: [PATCH 07/25] remove AnyValueBounds and add some docs --- packages/core/Cargo.toml | 1 + packages/core/src/nodes.rs | 37 ++++++++++++++++++++------------- packages/native-core/Cargo.toml | 1 + 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/packages/core/Cargo.toml b/packages/core/Cargo.toml index 367cdf400..b4ce6010e 100644 --- a/packages/core/Cargo.toml +++ b/packages/core/Cargo.toml @@ -44,4 +44,5 @@ dioxus = { path = "../dioxus" } [features] default = [] serialize = ["serde"] +# Adds a Sync + Send bound on any values in attributes sync_attributes = [] \ No newline at end of file diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index cc1c5f907..07f9739f6 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -390,7 +390,7 @@ impl PartialEq for AnyValueContainer { impl AnyValueContainer { /// Create a new AnyValueContainer containing the specified data. - pub fn new(value: T) -> Self { + pub fn new(value: T) -> Self { #[cfg(feature = "sync_attributes")] return Self(std::sync::Arc::new(value)); #[cfg(not(feature = "sync_attributes"))] @@ -401,12 +401,12 @@ impl AnyValueContainer { /// /// # Safety /// The caller must ensure that the type of the inner value is `T`. - pub unsafe fn downcast_ref_unchecked(&self) -> &T { + pub unsafe fn downcast_ref_unchecked(&self) -> &T { unsafe { &*(self.0.as_ref() as *const _ as *const T) } } /// Returns a reference to the inner value. - pub fn downcast_ref(&self) -> Option<&T> { + pub fn downcast_ref(&self) -> Option<&T> { if self.0.our_typeid() == TypeId::of::() { Some(unsafe { self.downcast_ref_unchecked() }) } else { @@ -415,7 +415,7 @@ impl AnyValueContainer { } /// Checks if the inner value is of type `T`. - pub fn is(&self) -> bool { + pub fn is(&self) -> bool { self.0.our_typeid() == TypeId::of::() } } @@ -480,15 +480,8 @@ impl<'a> PartialEq for AttributeValue<'a> { } } -#[cfg(feature = "sync_attributes")] -pub trait AnyValueBounds: Any + PartialEq + Sync + Send {} -#[cfg(feature = "sync_attributes")] -impl AnyValueBounds for T {} -#[cfg(not(feature = "sync_attributes"))] -pub trait AnyValueBounds: Any + PartialEq {} -#[cfg(not(feature = "sync_attributes"))] -impl AnyValueBounds for T {} - +// Some renderers need attributes to be sync and send. The rest of the attributes are already sync and send, but there is no way to force Any values to be sync and send on the renderer side. +// The sync_attributes flag restricts any valuse to be sync and send. #[doc(hidden)] #[cfg(feature = "sync_attributes")] pub trait AnyValue: Sync + Send { @@ -496,6 +489,21 @@ pub trait AnyValue: Sync + Send { fn our_typeid(&self) -> TypeId; } +#[cfg(feature = "sync_attributes")] +impl AnyValue for T { + fn any_cmp(&self, other: &dyn AnyValue) -> bool { + if self.our_typeid() != other.our_typeid() { + return false; + } + + self == unsafe { &*(other as *const _ as *const T) } + } + + fn our_typeid(&self) -> TypeId { + self.type_id() + } +} + #[doc(hidden)] #[cfg(not(feature = "sync_attributes"))] pub trait AnyValue { @@ -503,7 +511,8 @@ pub trait AnyValue { fn our_typeid(&self) -> TypeId; } -impl AnyValue for T { +#[cfg(not(feature = "sync_attributes"))] +impl AnyValue for T { fn any_cmp(&self, other: &dyn AnyValue) -> bool { if self.our_typeid() != other.our_typeid() { return false; diff --git a/packages/native-core/Cargo.toml b/packages/native-core/Cargo.toml index 057317779..ce56fd5da 100644 --- a/packages/native-core/Cargo.toml +++ b/packages/native-core/Cargo.toml @@ -30,4 +30,5 @@ dioxus = { path = "../dioxus", version = "^0.2.1" } [features] default = ["parallel"] +# This makes core only accept sync and send any values for attributes to allow for parallel resolution of passes. parallel = ["dioxus-core/sync_attributes"] \ No newline at end of file From c409fecaaf5ec5e0c082900fa03631528f143dbd Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sat, 17 Dec 2022 01:52:17 -0800 Subject: [PATCH 08/25] fix: sync with master --- packages/core/src/arena.rs | 2 +- packages/web/src/dom.rs | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/core/src/arena.rs b/packages/core/src/arena.rs index e876f09e1..380cfd90c 100644 --- a/packages/core/src/arena.rs +++ b/packages/core/src/arena.rs @@ -152,7 +152,7 @@ impl VirtualDom { listeners.drain(..).for_each(|listener| { let listener = unsafe { &*listener }; if let AttributeValue::Listener(l) = &listener.value { - _ = l.take(); + _ = l.0.take(); } }); } diff --git a/packages/web/src/dom.rs b/packages/web/src/dom.rs index 29a73b386..8fe1d7eb1 100644 --- a/packages/web/src/dom.rs +++ b/packages/web/src/dom.rs @@ -197,11 +197,9 @@ impl WebsysDom { }, SetText { value, id } => i.set_text(id.0 as u32, value), NewEventListener { name, id, .. } => { - i.new_event_listener(name, id.0 as u32, event_bubbles(&name[2..]) as u8); - } - RemoveEventListener { name, id } => { - i.remove_event_listener(name, id.0 as u32, event_bubbles(&name[2..]) as u8) + i.new_event_listener(name, id.0 as u32); } + RemoveEventListener { name, id } => i.remove_event_listener(name, id.0 as u32), Remove { id } => i.remove(id.0 as u32), PushRoot { id } => i.push_root(id.0 as u32), } From 3faef30075f9ca93dbea42e2a3db5c3146ede184 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sat, 17 Dec 2022 01:55:18 -0800 Subject: [PATCH 09/25] fix: dont clone callbacks --- packages/core/src/create.rs | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/packages/core/src/create.rs b/packages/core/src/create.rs index 9fe33a52e..0b8eff3d2 100644 --- a/packages/core/src/create.rs +++ b/packages/core/src/create.rs @@ -210,14 +210,27 @@ impl<'b> VirtualDom { // } // Safety: we promise not to re-alias this text later on after committing it to the mutation - let unbounded_value = unsafe { std::mem::transmute(attribute.value.clone()) }; - self.mutations.push(SetAttribute { - name: unbounded_name, - value: unbounded_value, - ns: attribute.namespace, - id, - }); + match &attribute.value { + AttributeValue::Listener(_) => { + self.mutations.push(NewEventListener { + // all listeners start with "on" + name: &unbounded_name[2..], + id, + }) + } + _ => { + // Safety: we promise not to re-alias this text later on after committing it to the mutation + let unbounded_value = unsafe { std::mem::transmute(attribute.value.clone()) }; + + self.mutations.push(SetAttribute { + name: unbounded_name, + value: unbounded_value, + ns: attribute.namespace, + id, + }) + } + } } fn load_template_root(&mut self, template: &VNode, root_idx: usize) -> ElementId { From 4caf95d639c72d5faa574ec779d8af50fd0ee0b1 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Fri, 23 Dec 2022 16:04:10 -0600 Subject: [PATCH 10/25] add bubbling data to event listeners in the web renderer --- packages/web/src/dom.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/web/src/dom.rs b/packages/web/src/dom.rs index ae449a176..cd600ce7d 100644 --- a/packages/web/src/dom.rs +++ b/packages/web/src/dom.rs @@ -197,9 +197,11 @@ impl WebsysDom { }, SetText { value, id } => i.set_text(id.0 as u32, value), NewEventListener { name, id, .. } => { - i.new_event_listener(name, id.0 as u32); + i.new_event_listener(name, id.0 as u32, event_bubbles(name) as u8); + } + RemoveEventListener { name, id } => { + i.remove_event_listener(name, id.0 as u32, event_bubbles(name) as u8) } - RemoveEventListener { name, id } => i.remove_event_listener(name, id.0 as u32), Remove { id } => i.remove(id.0 as u32), PushRoot { id } => i.push_root(id.0 as u32), } From 08c2440bcaa8c1d5261a11ac24b9eeaaf8e066d4 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Fri, 23 Dec 2022 16:10:05 -0600 Subject: [PATCH 11/25] update core tests --- packages/core/tests/attr_cleanup.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/core/tests/attr_cleanup.rs b/packages/core/tests/attr_cleanup.rs index 89f79abfb..fb67d0527 100644 --- a/packages/core/tests/attr_cleanup.rs +++ b/packages/core/tests/attr_cleanup.rs @@ -5,6 +5,7 @@ use bumpalo::Bump; use dioxus::core::{ElementId, Mutation::*}; use dioxus::prelude::*; +use dioxus_core::AttributeValue; #[test] fn attrs_cycle() { @@ -65,8 +66,18 @@ fn attrs_cycle() { [ LoadTemplate { name: "template", index: 0, id: ElementId(2) }, AssignId { path: &[0], id: ElementId(3) }, - SetAttribute { name: "class", value: "3", id: ElementId(3), ns: None }, - SetAttribute { name: "id", value: "3", id: ElementId(3), ns: None }, + SetAttribute { + name: "class", + value: AttributeValue::Text("3"), + id: ElementId(3), + ns: None + }, + SetAttribute { + name: "id", + value: AttributeValue::Text("3"), + id: ElementId(3), + ns: None + }, ReplaceWith { id: ElementId(1), m: 1 } ] ); From 170a3669f76e1b41d9459e3e975750db4d569346 Mon Sep 17 00:00:00 2001 From: = Date: Mon, 26 Dec 2022 08:37:50 -0600 Subject: [PATCH 12/25] reduce usage of unsafe --- packages/core/src/mutations.rs | 21 ++++++++++++- packages/core/src/nodes.rs | 52 ++++++++++++++------------------ packages/core/src/virtual_dom.rs | 13 +++----- 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/packages/core/src/mutations.rs b/packages/core/src/mutations.rs index 1d27769a2..06116f5dc 100644 --- a/packages/core/src/mutations.rs +++ b/packages/core/src/mutations.rs @@ -48,7 +48,20 @@ impl<'a> Mutations<'a> { /// Push a new mutation into the dom_edits list pub(crate) fn push(&mut self, mutation: Mutation<'static>) { - unsafe { self.edits.push(std::mem::transmute(mutation)) } + self.edits.push(mutation.downcast_lifetime()) + } +} + +impl Mutations<'static> { + pub(crate) fn take<'a>(&mut self) -> Mutations<'a> { + Mutations::<'a> { + subtree: self.subtree, + dirty_scopes: self.dirty_scopes.drain().collect(), + templates: self.templates.split_off(0), + // Safety: this downcasts the lifetime from 'static to 'a + // This is safe because 'static outlives 'a + edits: unsafe { std::mem::transmute(self.edits.split_off(0)) }, + } } } @@ -244,3 +257,9 @@ pub enum Mutation<'a> { id: ElementId, }, } + +impl Mutation<'static> { + pub(crate) fn downcast_lifetime<'a>(self) -> Mutation<'a> { + unsafe { std::mem::transmute(self) } + } +} diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 8fa33e526..cc9ac2829 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -8,6 +8,7 @@ use std::{ cell::{Cell, RefCell}, fmt::Arguments, future::Future, + ops::Deref, }; pub type TemplateId = &'static str; @@ -416,26 +417,14 @@ impl AnyValueContainer { return Self(std::rc::Rc::new(value)); } - /// Returns a reference to the inner value without checking the type. - /// - /// # Safety - /// The caller must ensure that the type of the inner value is `T`. - pub unsafe fn downcast_ref_unchecked(&self) -> &T { - unsafe { &*(self.0.as_ref() as *const _ as *const T) } - } - /// Returns a reference to the inner value. pub fn downcast_ref(&self) -> Option<&T> { - if self.0.our_typeid() == TypeId::of::() { - Some(unsafe { self.downcast_ref_unchecked() }) - } else { - None - } + self.0.deref().as_any().downcast_ref() } - /// Checks if the inner value is of type `T`. + /// Checks if the type of the inner value is 'T'. pub fn is(&self) -> bool { - self.0.our_typeid() == TypeId::of::() + self.0.deref().as_any().is::() } } @@ -446,9 +435,6 @@ fn test_any_value_rc() { assert_eq!(a.downcast_ref::(), None); assert!(a.is::()); assert!(!a.is::()); - unsafe { - assert_eq!(a.downcast_ref_unchecked::(), &1i32); - } } #[cfg(feature = "serialize")] @@ -503,45 +489,51 @@ impl<'a> PartialEq for AttributeValue<'a> { // The sync_attributes flag restricts any valuse to be sync and send. #[doc(hidden)] #[cfg(feature = "sync_attributes")] -pub trait AnyValue: Sync + Send { +pub trait AnyValue: Sync + Send + 'static { fn any_cmp(&self, other: &dyn AnyValue) -> bool; - fn our_typeid(&self) -> TypeId; + fn as_any(&self) -> &dyn Any; + fn type_id(&self) -> TypeId { + self.as_any().type_id() + } } #[cfg(feature = "sync_attributes")] -impl AnyValue for T { +impl AnyValue for T { fn any_cmp(&self, other: &dyn AnyValue) -> bool { - if self.our_typeid() != other.our_typeid() { + if self.type_id() != other.type_id() { return false; } self == unsafe { &*(other as *const _ as *const T) } } - fn our_typeid(&self) -> TypeId { - self.type_id() + fn as_any(&self) -> &dyn Any { + self } } #[doc(hidden)] #[cfg(not(feature = "sync_attributes"))] -pub trait AnyValue { +pub trait AnyValue: 'static { fn any_cmp(&self, other: &dyn AnyValue) -> bool; - fn our_typeid(&self) -> TypeId; + fn as_any(&self) -> &dyn Any; + fn type_id(&self) -> TypeId { + self.as_any().type_id() + } } #[cfg(not(feature = "sync_attributes"))] -impl AnyValue for T { +impl AnyValue for T { fn any_cmp(&self, other: &dyn AnyValue) -> bool { - if self.our_typeid() != other.our_typeid() { + if self.type_id() != other.type_id() { return false; } self == unsafe { &*(other as *const _ as *const T) } } - fn our_typeid(&self) -> TypeId { - self.type_id() + fn as_any(&self) -> &dyn Any { + self } } diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index c862a3e39..9afbe2ce9 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -489,7 +489,7 @@ impl VirtualDom { RenderReturn::Async(_) => unreachable!("Root scope cannot be an async component"), } - unsafe { std::mem::transmute(self.finalize()) } + self.finalize() } /// Render whatever the VirtualDom has ready as fast as possible without requiring an executor to progress @@ -594,7 +594,7 @@ impl VirtualDom { // If there's no pending suspense, then we have no reason to wait for anything if self.scheduler.leaves.borrow().is_empty() { - return unsafe { std::mem::transmute(self.finalize()) }; + return self.finalize(); } // Poll the suspense leaves in the meantime @@ -608,17 +608,14 @@ impl VirtualDom { if let Either::Left((_, _)) = select(&mut deadline, pinned).await { // release the borrowed drop(work); - return unsafe { std::mem::transmute(self.finalize()) }; + return self.finalize(); } } } /// Swap the current mutations with a new - fn finalize(&mut self) -> Mutations<'static> { - // todo: make this a routine - let mut out = Mutations::default(); - std::mem::swap(&mut self.mutations, &mut out); - out + fn finalize<'a>(&'a mut self) -> Mutations<'a> { + self.mutations.take() } } From c245f04c8cd6c3c6fe0910868039215dc8190b59 Mon Sep 17 00:00:00 2001 From: = Date: Mon, 26 Dec 2022 08:41:37 -0600 Subject: [PATCH 13/25] make any_cmp safe --- packages/core/src/nodes.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index cc9ac2829..7432057fe 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -500,11 +500,11 @@ pub trait AnyValue: Sync + Send + 'static { #[cfg(feature = "sync_attributes")] impl AnyValue for T { fn any_cmp(&self, other: &dyn AnyValue) -> bool { - if self.type_id() != other.type_id() { + if let Some(other) = other.as_any().downcast_ref() { + self == other + } else { return false; } - - self == unsafe { &*(other as *const _ as *const T) } } fn as_any(&self) -> &dyn Any { @@ -525,11 +525,11 @@ pub trait AnyValue: 'static { #[cfg(not(feature = "sync_attributes"))] impl AnyValue for T { fn any_cmp(&self, other: &dyn AnyValue) -> bool { - if self.type_id() != other.type_id() { + if let Some(other) = other.as_any().downcast_ref() { + self == other + } else { return false; } - - self == unsafe { &*(other as *const _ as *const T) } } fn as_any(&self) -> &dyn Any { From 1555500fbd0f8022aa8025b22df3d4b974a1712b Mon Sep 17 00:00:00 2001 From: = Date: Mon, 26 Dec 2022 10:53:25 -0600 Subject: [PATCH 14/25] drop any values correctly --- packages/core/src/arena.rs | 12 +++++++++--- packages/core/src/nodes.rs | 6 +++--- packages/core/src/scope_arena.rs | 2 +- packages/core/src/scopes.rs | 14 +++++++++----- packages/native-core/src/node.rs | 2 +- 5 files changed, 23 insertions(+), 13 deletions(-) diff --git a/packages/core/src/arena.rs b/packages/core/src/arena.rs index 4cb5d3964..a7d030162 100644 --- a/packages/core/src/arena.rs +++ b/packages/core/src/arena.rs @@ -155,11 +155,17 @@ impl VirtualDom { }); // Now that all the references are gone, we can safely drop our own references in our listeners. - let mut listeners = scope.listeners.borrow_mut(); + let mut listeners = scope.attributes_to_drop.borrow_mut(); listeners.drain(..).for_each(|listener| { let listener = unsafe { &*listener }; - if let AttributeValue::Listener(l) = &listener.value { - _ = l.0.take(); + match &listener.value { + AttributeValue::Listener(l) => { + _ = l.0.take(); + } + AttributeValue::Any(a) => { + _ = a.take(); + } + _ => (), } }); } diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 7432057fe..c0ac4efd4 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -357,7 +357,7 @@ pub enum AttributeValue<'a> { Listener(ListenerCb<'a>), /// An arbitrary value that implements PartialEq and is static - Any(AnyValueContainer), + Any(RefCell>), /// A "none" value, resulting in the removal of an attribute from the dom None, @@ -392,9 +392,9 @@ impl<'de, 'a> serde::Deserialize<'de> for ListenerCb<'a> { } } -/// A boxed value that implements PartialEq and Any #[derive(Clone)] #[cfg(not(feature = "sync_attributes"))] +/// A boxed value that implements PartialEq, and Any pub struct AnyValueContainer(pub std::rc::Rc); #[derive(Clone)] @@ -732,6 +732,6 @@ impl<'a> IntoAttributeValue<'a> for Arguments<'_> { impl<'a> IntoAttributeValue<'a> for AnyValueContainer { fn into_value(self, _: &'a Bump) -> AttributeValue<'a> { - AttributeValue::Any(self) + AttributeValue::Any(RefCell::new(Some(self))) } } diff --git a/packages/core/src/scope_arena.rs b/packages/core/src/scope_arena.rs index 9c0f96db3..6dd5316d3 100644 --- a/packages/core/src/scope_arena.rs +++ b/packages/core/src/scope_arena.rs @@ -45,7 +45,7 @@ impl VirtualDom { hook_idx: Default::default(), shared_contexts: Default::default(), borrowed_props: Default::default(), - listeners: Default::default(), + attributes_to_drop: Default::default(), })) } diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index 83428fd7d..7520df942 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -88,7 +88,7 @@ pub struct ScopeState { pub(crate) spawned_tasks: FxHashSet, pub(crate) borrowed_props: RefCell>>, - pub(crate) listeners: RefCell>>, + pub(crate) attributes_to_drop: RefCell>>, pub(crate) props: Option>>, pub(crate) placeholder: Cell>, @@ -374,11 +374,15 @@ impl<'src> ScopeState { pub fn render(&'src self, rsx: LazyNodes<'src, '_>) -> Element<'src> { let element = rsx.call(self); - let mut listeners = self.listeners.borrow_mut(); + let mut listeners = self.attributes_to_drop.borrow_mut(); for attr in element.dynamic_attrs { - if let AttributeValue::Listener(_) = attr.value { - let unbounded = unsafe { std::mem::transmute(attr as *const Attribute) }; - listeners.push(unbounded); + match attr.value { + AttributeValue::Any(_) | AttributeValue::Listener(_) => { + let unbounded = unsafe { std::mem::transmute(attr as *const Attribute) }; + listeners.push(unbounded); + } + + _ => (), } } diff --git a/packages/native-core/src/node.rs b/packages/native-core/src/node.rs index c471dba11..99777dc27 100644 --- a/packages/native-core/src/node.rs +++ b/packages/native-core/src/node.rs @@ -103,7 +103,7 @@ impl From> for OwnedAttributeValue { AttributeValue::Float(float) => Self::Float(float), AttributeValue::Int(int) => Self::Int(int), AttributeValue::Bool(bool) => Self::Bool(bool), - AttributeValue::Any(any) => Self::Any(any), + AttributeValue::Any(any) => Self::Any(any.borrow().as_ref().unwrap().clone()), AttributeValue::None => Self::None, _ => Self::None, } From cb17252a21607ce294d56232983da80ef06c38e3 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Wed, 28 Dec 2022 20:42:47 -0600 Subject: [PATCH 15/25] clean up warnings and update prelude --- packages/core/src/lib.rs | 14 +++++++------- packages/core/src/nodes.rs | 4 ++-- packages/core/src/virtual_dom.rs | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index f13c20b43..b6fe456bc 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -70,10 +70,10 @@ pub(crate) mod innerlude { } pub use crate::innerlude::{ - fc_to_builder, Attribute, AttributeValue, CapturedError, Component, DynamicNode, Element, - ElementId, Event, Fragment, IntoDynNode, LazyNodes, Mutation, Mutations, Properties, - RenderReturn, Scope, ScopeId, ScopeState, Scoped, SuspenseContext, TaskId, Template, - TemplateAttribute, TemplateNode, VComponent, VNode, VText, VirtualDom, + fc_to_builder, AnyValueContainer, Attribute, AttributeValue, CapturedError, Component, + DynamicNode, Element, ElementId, Event, Fragment, IntoDynNode, LazyNodes, Mutation, Mutations, + Properties, RenderReturn, Scope, ScopeId, ScopeState, Scoped, SuspenseContext, TaskId, + Template, TemplateAttribute, TemplateNode, VComponent, VNode, VText, VirtualDom, }; /// The purpose of this module is to alleviate imports of many common types @@ -81,9 +81,9 @@ pub use crate::innerlude::{ /// This includes types like [`Scope`], [`Element`], and [`Component`]. pub mod prelude { pub use crate::innerlude::{ - fc_to_builder, Component, Element, Event, EventHandler, Fragment, LazyNodes, Properties, - Scope, ScopeId, ScopeState, Scoped, TaskId, Template, TemplateAttribute, TemplateNode, - Throw, VNode, VirtualDom, + fc_to_builder, Component, Element, Event, EventHandler, Fragment, IntoAttributeValue, + LazyNodes, Properties, Scope, ScopeId, ScopeState, Scoped, TaskId, Template, + TemplateAttribute, TemplateNode, Throw, VNode, VirtualDom, }; } diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 030e77e8d..0c797460a 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -673,7 +673,7 @@ impl AnyValue for T { if let Some(other) = other.as_any().downcast_ref() { self == other } else { - return false; + false } } @@ -698,7 +698,7 @@ impl AnyValue for T { if let Some(other) = other.as_any().downcast_ref() { self == other } else { - return false; + false } } diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index 890416cd9..56363788f 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -640,7 +640,7 @@ impl VirtualDom { } /// Swap the current mutations with a new - fn finalize<'a>(&'a mut self) -> Mutations<'a> { + fn finalize(&mut self) -> Mutations { self.mutations.take() } } From 845f2fb7a9417dade313d4afe075a016125bd003 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Fri, 30 Dec 2022 14:19:56 -0600 Subject: [PATCH 16/25] use get2_mut instead of unsafe --- packages/native-core/src/tree.rs | 37 ++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/packages/native-core/src/tree.rs b/packages/native-core/src/tree.rs index c173824cb..dfd71743b 100644 --- a/packages/native-core/src/tree.rs +++ b/packages/native-core/src/tree.rs @@ -111,15 +111,7 @@ pub trait TreeView: Sized { fn parent_mut(&mut self, id: NodeId) -> Option<&mut T>; - fn node_parent_mut(&mut self, id: NodeId) -> Option<(&mut T, Option<&mut T>)> { - let mut_ptr: *mut Self = self; - unsafe { - // Safety: No node has itself as a parent. - (*mut_ptr) - .get_mut(id) - .map(|node| (node, (*mut_ptr).parent_mut(id))) - } - } + fn node_parent_mut(&mut self, id: NodeId) -> Option<(&mut T, Option<&mut T>)>; fn parent_id(&self, id: NodeId) -> Option; @@ -340,6 +332,16 @@ impl TreeView for Tree { fn size(&self) -> usize { self.nodes.len() } + + fn node_parent_mut(&mut self, id: NodeId) -> Option<(&mut T, Option<&mut T>)> { + if let Some(parent_id) = self.parent_id(id) { + self.nodes + .get2_mut(id.0, parent_id.0) + .map(|(node, parent)| (&mut node.value, Some(&mut parent.value))) + } else { + self.nodes.get_mut(id.0).map(|node| (&mut node.value, None)) + } + } } impl TreeLike for Tree { @@ -547,6 +549,15 @@ where fn size(&self) -> usize { self.tree.size() } + + fn node_parent_mut(&mut self, id: NodeId) -> Option<(&mut T2, Option<&mut T2>)> { + self.tree.node_parent_mut(id).map(|(node, parent)| { + ( + (self.map_mut)(node), + parent.map(|parent| (self.map_mut)(parent)), + ) + }) + } } /// A view into a tree that can be shared between multiple threads. Nodes are locked invividually. @@ -675,6 +686,14 @@ impl<'a, T, Tr: TreeView> TreeView for SharedView<'a, T, Tr> { fn size(&self) -> usize { unsafe { (*self.tree.get()).size() } } + + fn node_parent_mut(&mut self, id: NodeId) -> Option<(&mut T, Option<&mut T>)> { + let parent_id = self.parent_id(id)?; + self.lock_node(parent_id); + let return_value = self.with_node(id, |t| t.node_parent_mut(id)); + self.unlock_node(parent_id); + return_value + } } #[test] From c1fc00ed53cb2975500f8cc72cee5281791a2076 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Fri, 30 Dec 2022 15:23:04 -0600 Subject: [PATCH 17/25] remove unsafe sync impl --- packages/native-core/src/tree.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/native-core/src/tree.rs b/packages/native-core/src/tree.rs index dfd71743b..796105a1b 100644 --- a/packages/native-core/src/tree.rs +++ b/packages/native-core/src/tree.rs @@ -225,8 +225,6 @@ pub struct ChildNodeIteratorMut<'a, T, Tr: TreeView + 'a> { node_type: PhantomData, } -unsafe impl<'a, T, Tr: TreeView + 'a> Sync for ChildNodeIteratorMut<'a, T, Tr> {} - impl<'a, T, Tr: TreeView> ChildNodeIteratorMut<'a, T, Tr> { fn tree_mut(&mut self) -> &'a mut Tr { unsafe { &mut *self.tree } From 2d67dabfe5eb9b4df376980ef766172d3065befd Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Sun, 1 Jan 2023 15:48:26 -0600 Subject: [PATCH 18/25] require Send for S for resolve_passes multithreaded --- packages/native-core/src/passes.rs | 24 +++++++++++------------- packages/native-core/src/real_dom.rs | 2 +- packages/native-core/src/state.rs | 2 +- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/native-core/src/passes.rs b/packages/native-core/src/passes.rs index fd15e2ae6..c393cf119 100644 --- a/packages/native-core/src/passes.rs +++ b/packages/native-core/src/passes.rs @@ -310,11 +310,7 @@ impl AnyPass { } } -struct RawPointer(*mut T); -unsafe impl Send for RawPointer {} -unsafe impl Sync for RawPointer {} - -pub fn resolve_passes + Sync>( +pub fn resolve_passes + Sync + Send>( tree: &mut Tr, dirty_nodes: DirtyNodeStates, mut passes: Vec<&AnyPass>, @@ -342,19 +338,21 @@ pub fn resolve_passes + Sync>( let pass = passes.remove(i); resolving.push(pass_id); currently_borrowed |= pass_mask; - let tree_mut = tree as *mut _; - let raw_ptr = RawPointer(tree_mut); + let tree_mut_unbounded = unsafe { &mut *(tree as *mut _ as *mut Tr) }; let dirty_states = dirty_states.clone(); let nodes_updated = nodes_updated.clone(); let ctx = ctx.clone(); - s.spawn(move || unsafe { - // let tree_mut: &mut Tr = &mut *raw_ptr.0; - let raw = raw_ptr; + s.spawn(move || { // this is safe because the member_mask acts as a per-member mutex and we have verified that the pass does not overlap with any other pass - let tree_mut: &mut Tr = &mut *raw.0; let mut dirty = DirtyNodes::default(); - dirty_states.all_dirty(pass_id, &mut dirty, tree_mut); - pass.resolve(tree_mut, dirty, &dirty_states, &nodes_updated, &ctx); + dirty_states.all_dirty(pass_id, &mut dirty, tree_mut_unbounded); + pass.resolve( + tree_mut_unbounded, + dirty, + &dirty_states, + &nodes_updated, + &ctx, + ); }); } else { i += 1; diff --git a/packages/native-core/src/real_dom.rs b/packages/native-core/src/real_dom.rs index 2258f524b..4ef4d89c0 100644 --- a/packages/native-core/src/real_dom.rs +++ b/packages/native-core/src/real_dom.rs @@ -398,7 +398,7 @@ impl RealDom { impl RealDom where - Tree>: Sync, + Tree>: Sync + Send, { /// Update the state of the dom, after appling some mutations. This will keep the nodes in the dom up to date with their VNode counterparts. /// This will resolve the state in parallel diff --git a/packages/native-core/src/state.rs b/packages/native-core/src/state.rs index 54dad4997..9bedd19be 100644 --- a/packages/native-core/src/state.rs +++ b/packages/native-core/src/state.rs @@ -217,7 +217,7 @@ pub trait State: Default + Clone + 'static { const MASKS: &'static [NodeMask]; #[doc(hidden)] - fn update> + Sync>( + fn update> + Sync + Send>( dirty: DirtyNodeStates, tree: &mut T, ctx: SendAnyMap, From 359b4e514b477b7a800a67fbfe64eab1097bd262 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Sun, 1 Jan 2023 18:44:54 -0600 Subject: [PATCH 19/25] remove some more unnessisary usages of unsafe --- packages/native-core-macro/src/lib.rs | 3 +- packages/native-core/src/state.rs | 12 +-- packages/native-core/src/tree.rs | 136 -------------------------- 3 files changed, 8 insertions(+), 143 deletions(-) diff --git a/packages/native-core-macro/src/lib.rs b/packages/native-core-macro/src/lib.rs index 5bf50afcc..75b814347 100644 --- a/packages/native-core-macro/src/lib.rs +++ b/packages/native-core-macro/src/lib.rs @@ -393,7 +393,8 @@ impl<'a> StateMember<'a> { let ty = &self.mem.ty; let unit_type = &self.mem.unit_type; - let node_view = quote!(dioxus_native_core::node_ref::NodeView::new(unsafe{&*{&node.node_data as *const _}}, #ty::NODE_MASK)); + let node_view = + quote!(dioxus_native_core::node_ref::NodeView::new(&node.node_data, #ty::NODE_MASK)); let dep_idents = self.dep_mems.iter().map(|m| &m.ident); let impl_specific = match self.dep_kind { DependencyKind::Node => { diff --git a/packages/native-core/src/state.rs b/packages/native-core/src/state.rs index 9bedd19be..acdffa483 100644 --- a/packages/native-core/src/state.rs +++ b/packages/native-core/src/state.rs @@ -87,7 +87,7 @@ pub trait ChildDepState { /// Resolve the state current node's state from the state of the children, the state of the node, and some external context. fn reduce<'a>( &mut self, - node: NodeView, + node: NodeView<'a>, children: impl Iterator::ElementBorrowed<'a>>, ctx: &Self::Ctx, ) -> bool @@ -149,7 +149,7 @@ pub trait ParentDepState { /// Resolve the state current node's state from the state of the parent node, the state of the node, and some external context. fn reduce<'a>( &mut self, - node: NodeView, + node: NodeView<'a>, parent: Option<::ElementBorrowed<'a>>, ctx: &Self::Ctx, ) -> bool; @@ -203,7 +203,7 @@ pub trait NodeDepState { /// Resolve the state current node's state from the state of the sibling states, the state of the node, and some external context. fn reduce<'a>( &mut self, - node: NodeView, + node: NodeView<'a>, node_state: ::ElementBorrowed<'a>, ctx: &Self::Ctx, ) -> bool; @@ -240,7 +240,7 @@ pub trait State: Default + Clone + 'static { impl ChildDepState for () { type Ctx = (); type DepState = (); - fn reduce<'a>(&mut self, _: NodeView, _: impl Iterator, _: &Self::Ctx) -> bool + fn reduce<'a>(&mut self, _: NodeView<'a>, _: impl Iterator, _: &Self::Ctx) -> bool where Self::DepState: 'a, { @@ -251,7 +251,7 @@ impl ChildDepState for () { impl ParentDepState for () { type Ctx = (); type DepState = (); - fn reduce<'a>(&mut self, _: NodeView, _: Option<()>, _: &Self::Ctx) -> bool { + fn reduce<'a>(&mut self, _: NodeView<'a>, _: Option<()>, _: &Self::Ctx) -> bool { false } } @@ -259,7 +259,7 @@ impl ParentDepState for () { impl NodeDepState for () { type DepState = (); type Ctx = (); - fn reduce(&mut self, _: NodeView, _sibling: (), _: &Self::Ctx) -> bool { + fn reduce<'a>(&mut self, _: NodeView<'a>, _sibling: (), _: &Self::Ctx) -> bool { false } } diff --git a/packages/native-core/src/tree.rs b/packages/native-core/src/tree.rs index 796105a1b..b81c6a5c2 100644 --- a/packages/native-core/src/tree.rs +++ b/packages/native-core/src/tree.rs @@ -558,142 +558,6 @@ where } } -/// A view into a tree that can be shared between multiple threads. Nodes are locked invividually. -pub struct SharedView<'a, T, Tr: TreeView> { - tree: Arc>, - node_locks: Arc>>, - node_type: PhantomData, -} - -impl<'a, T, Tr: TreeView> SharedView<'a, T, Tr> { - /// Checks if a node is currently locked. Returns None if the node does not exist. - pub fn check_lock(&self, id: NodeId) -> Option { - let locks = self.node_locks.read(); - locks.get(id.0).map(|lock| lock.is_locked()) - } -} - -unsafe impl<'a, T, Tr: TreeView + Send> Send for SharedView<'a, T, Tr> {} -unsafe impl<'a, T, Tr: TreeView + Sync> Sync for SharedView<'a, T, Tr> {} -impl<'a, T, Tr: TreeView> Clone for SharedView<'a, T, Tr> { - fn clone(&self) -> Self { - Self { - tree: self.tree.clone(), - node_locks: self.node_locks.clone(), - node_type: PhantomData, - } - } -} - -impl<'a, T, Tr: TreeView> SharedView<'a, T, Tr> { - pub fn new(tree: &'a mut Tr) -> Self { - let tree = Arc::new(UnsafeCell::new(tree)); - let mut node_locks = Vec::new(); - for _ in 0..unsafe { (*tree.get()).size() } { - node_locks.push(RawMutex::INIT); - } - Self { - tree, - node_locks: Arc::new(RwLock::new(node_locks)), - node_type: PhantomData, - } - } - - fn lock_node(&self, node: NodeId) { - let read = self.node_locks.read(); - let lock = read.get(node.0); - match lock { - Some(lock) => lock.lock(), - None => { - drop(read); - let mut write = self.node_locks.write(); - write.resize_with(node.0 + 1, || RawMutex::INIT); - unsafe { write.get_unchecked(node.0).lock() } - } - } - } - - fn unlock_node(&self, node: NodeId) { - let read = self.node_locks.read(); - let lock = read.get(node.0); - match lock { - Some(lock) => unsafe { lock.unlock() }, - None => { - panic!("unlocking node that was not locked") - } - } - } - - fn with_node(&self, node_id: NodeId, f: impl FnOnce(&'a mut Tr) -> R) -> R { - self.lock_node(node_id); - let tree = unsafe { &mut *self.tree.get() }; - let r = f(tree); - self.unlock_node(node_id); - r - } -} - -impl<'a, T, Tr: TreeView> TreeView for SharedView<'a, T, Tr> { - type Iterator<'b> = Tr::Iterator<'b> where T: 'b, Self: 'b; - - type IteratorMut<'b>=Tr::IteratorMut<'b> - where - T: 'b, - Self: 'b; - - fn root(&self) -> NodeId { - unsafe { (*self.tree.get()).root() } - } - - fn get(&self, id: NodeId) -> Option<&T> { - self.with_node(id, |t| t.get(id)) - } - - fn get_mut(&mut self, id: NodeId) -> Option<&mut T> { - self.with_node(id, |t| t.get_mut(id)) - } - - fn children(&self, id: NodeId) -> Option> { - self.with_node(id, |t| t.children(id)) - } - - fn children_mut(&mut self, id: NodeId) -> Option> { - self.with_node(id, |t| t.children_mut(id)) - } - - fn children_ids(&self, id: NodeId) -> Option<&[NodeId]> { - self.with_node(id, |t| t.children_ids(id)) - } - - fn parent(&self, id: NodeId) -> Option<&T> { - self.with_node(id, |t| t.get(id)) - } - - fn parent_mut(&mut self, id: NodeId) -> Option<&mut T> { - self.with_node(id, |t| t.parent_mut(id)) - } - - fn parent_id(&self, id: NodeId) -> Option { - self.with_node(id, |t| t.parent_id(id)) - } - - fn height(&self, id: NodeId) -> Option { - unsafe { (*self.tree.get()).height(id) } - } - - fn size(&self) -> usize { - unsafe { (*self.tree.get()).size() } - } - - fn node_parent_mut(&mut self, id: NodeId) -> Option<(&mut T, Option<&mut T>)> { - let parent_id = self.parent_id(id)?; - self.lock_node(parent_id); - let return_value = self.with_node(id, |t| t.node_parent_mut(id)); - self.unlock_node(parent_id); - return_value - } -} - #[test] fn creation() { let mut tree = Tree::new(1); From 6d60c483f8debf835340cbff40872693674a85e3 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 2 Jan 2023 11:01:17 -0600 Subject: [PATCH 20/25] make it pass miri --- packages/native-core/Cargo.toml | 2 +- packages/native-core/src/lib.rs | 3 +- packages/native-core/src/passes.rs | 99 +++---- packages/native-core/src/tree.rs | 407 ++++++++++++----------------- packages/tui/Cargo.toml | 2 +- 5 files changed, 225 insertions(+), 288 deletions(-) diff --git a/packages/native-core/Cargo.toml b/packages/native-core/Cargo.toml index ce56fd5da..5cc25b61c 100644 --- a/packages/native-core/Cargo.toml +++ b/packages/native-core/Cargo.toml @@ -18,7 +18,7 @@ dioxus-core-macro = { path = "../core-macro", version = "^0.2.1" } taffy = "0.2.1" smallvec = "1.6" rustc-hash = "1.1.0" -anymap = "0.12.1" +anymap = "1.0.0-beta.2" slab = "0.4" parking_lot = "0.12.1" crossbeam-deque = "0.8.2" diff --git a/packages/native-core/src/lib.rs b/packages/native-core/src/lib.rs index ae739af86..1a8bb0ce0 100644 --- a/packages/native-core/src/lib.rs +++ b/packages/native-core/src/lib.rs @@ -1,3 +1,4 @@ +use std::any::Any; use std::hash::BuildHasherDefault; pub use node_ref::NodeMask; @@ -20,7 +21,7 @@ pub mod utils; pub type RealNodeId = NodeId; pub type FxDashMap = dashmap::DashMap>; pub type FxDashSet = dashmap::DashSet>; -pub type SendAnyMap = anymap::Map; +pub type SendAnyMap = anymap::Map; /// Used in derived state macros #[derive(Eq, PartialEq)] diff --git a/packages/native-core/src/passes.rs b/packages/native-core/src/passes.rs index c393cf119..c438828b0 100644 --- a/packages/native-core/src/passes.rs +++ b/packages/native-core/src/passes.rs @@ -313,57 +313,58 @@ impl AnyPass { pub fn resolve_passes + Sync + Send>( tree: &mut Tr, dirty_nodes: DirtyNodeStates, - mut passes: Vec<&AnyPass>, + passes: Vec<&AnyPass>, ctx: SendAnyMap, ) -> FxDashSet { - let dirty_states = Arc::new(dirty_nodes); - let mut resolved_passes: FxHashSet = FxHashSet::default(); - let mut resolving = Vec::new(); - let nodes_updated = Arc::new(FxDashSet::default()); - let ctx = Arc::new(ctx); - while !passes.is_empty() { - let mut currently_borrowed = MemberMask::default(); - std::thread::scope(|s| { - let mut i = 0; - while i < passes.len() { - let pass = &passes[i]; - let pass_id = pass.pass_id(); - let pass_mask = pass.mask(); - if pass - .dependancies() - .iter() - .all(|d| resolved_passes.contains(d) || *d == pass_id) - && !pass_mask.overlaps(currently_borrowed) - { - let pass = passes.remove(i); - resolving.push(pass_id); - currently_borrowed |= pass_mask; - let tree_mut_unbounded = unsafe { &mut *(tree as *mut _ as *mut Tr) }; - let dirty_states = dirty_states.clone(); - let nodes_updated = nodes_updated.clone(); - let ctx = ctx.clone(); - s.spawn(move || { - // this is safe because the member_mask acts as a per-member mutex and we have verified that the pass does not overlap with any other pass - let mut dirty = DirtyNodes::default(); - dirty_states.all_dirty(pass_id, &mut dirty, tree_mut_unbounded); - pass.resolve( - tree_mut_unbounded, - dirty, - &dirty_states, - &nodes_updated, - &ctx, - ); - }); - } else { - i += 1; - } - } - // all passes are resolved at the end of the scope - }); - resolved_passes.extend(resolving.iter().copied()); - resolving.clear() - } - std::sync::Arc::try_unwrap(nodes_updated).unwrap() + resolve_passes_single_threaded(tree, dirty_nodes, passes, ctx) + // let dirty_states = Arc::new(dirty_nodes); + // let mut resolved_passes: FxHashSet = FxHashSet::default(); + // let mut resolving = Vec::new(); + // let nodes_updated = Arc::new(FxDashSet::default()); + // let ctx = Arc::new(ctx); + // while !passes.is_empty() { + // let mut currently_borrowed = MemberMask::default(); + // std::thread::scope(|s| { + // let mut i = 0; + // while i < passes.len() { + // let pass = &passes[i]; + // let pass_id = pass.pass_id(); + // let pass_mask = pass.mask(); + // if pass + // .dependancies() + // .iter() + // .all(|d| resolved_passes.contains(d) || *d == pass_id) + // && !pass_mask.overlaps(currently_borrowed) + // { + // let pass = passes.remove(i); + // resolving.push(pass_id); + // currently_borrowed |= pass_mask; + // let dirty_states = dirty_states.clone(); + // let nodes_updated = nodes_updated.clone(); + // let ctx = ctx.clone(); + // let mut dirty = DirtyNodes::default(); + // // dirty_states.all_dirty(pass_id, &mut dirty, tree); + // // this is safe because the member_mask acts as a per-member mutex and we have verified that the pass does not overlap with any other pass + // let tree_mut_unbounded = unsafe { &mut *(tree as *mut Tr) }; + // s.spawn(move || { + // pass.resolve( + // tree_mut_unbounded, + // dirty, + // &dirty_states, + // &nodes_updated, + // &ctx, + // ); + // }); + // } else { + // i += 1; + // } + // } + // // all passes are resolved at the end of the scope + // }); + // resolved_passes.extend(resolving.iter().copied()); + // resolving.clear() + // } + // std::sync::Arc::try_unwrap(nodes_updated).unwrap() } pub fn resolve_passes_single_threaded>( diff --git a/packages/native-core/src/tree.rs b/packages/native-core/src/tree.rs index b81c6a5c2..f55066fc4 100644 --- a/packages/native-core/src/tree.rs +++ b/packages/native-core/src/tree.rs @@ -1,7 +1,6 @@ use core::panic; use parking_lot::lock_api::RawMutex as _; use parking_lot::{RawMutex, RwLock}; -use slab::Slab; use std::cell::UnsafeCell; use std::collections::VecDeque; use std::marker::PhantomData; @@ -93,17 +92,7 @@ pub trait TreeView: Sized { fn children_mut(&mut self, id: NodeId) -> Option>; - fn parent_child_mut(&mut self, id: NodeId) -> Option<(&mut T, Self::IteratorMut<'_>)> { - let mut_ptr: *mut Self = self; - unsafe { - // Safety: No node has itself as a child. - (*mut_ptr).get_mut(id).and_then(|parent| { - (*mut_ptr) - .children_mut(id) - .map(|children| (parent, children)) - }) - } - } + fn parent_child_mut(&mut self, id: NodeId) -> Option<(&mut T, Self::IteratorMut<'_>)>; fn children_ids(&self, id: NodeId) -> Option<&[NodeId]>; @@ -117,14 +106,6 @@ pub trait TreeView: Sized { fn height(&self, id: NodeId) -> Option; - fn map &T2, FMut: Fn(&mut T) -> &mut T2>( - &mut self, - map: F, - map_mut: FMut, - ) -> TreeMap { - TreeMap::new(self, map, map_mut) - } - fn size(&self) -> usize; fn traverse_depth_first(&self, mut f: impl FnMut(&T)) { @@ -200,56 +181,39 @@ pub trait TreeLike: TreeView { fn insert_after(&mut self, id: NodeId, new: NodeId); } -pub struct ChildNodeIterator<'a, T, Tr: TreeView> { - tree: &'a Tr, - children_ids: &'a [NodeId], +pub struct ChildNodeIterator<'a, T> { + nodes: &'a Slab>, + children_ids: Vec, index: usize, node_type: PhantomData, } -impl<'a, T: 'a, Tr: TreeView> Iterator for ChildNodeIterator<'a, T, Tr> { +impl<'a, T: 'a> Iterator for ChildNodeIterator<'a, T> { type Item = &'a T; fn next(&mut self) -> Option { self.children_ids.get(self.index).map(|id| { self.index += 1; - self.tree.get_unchecked(*id) + &self.nodes.get(id.0).unwrap().value }) } } -pub struct ChildNodeIteratorMut<'a, T, Tr: TreeView + 'a> { - tree: *mut Tr, - children_ids: &'a [NodeId], - index: usize, - node_type: PhantomData, +pub struct ChildNodeIteratorMut<'a, T> { + nodes: Vec<&'a mut Node>, } -impl<'a, T, Tr: TreeView> ChildNodeIteratorMut<'a, T, Tr> { - fn tree_mut(&mut self) -> &'a mut Tr { - unsafe { &mut *self.tree } - } -} - -impl<'a, T: 'a, Tr: TreeView> Iterator for ChildNodeIteratorMut<'a, T, Tr> { +impl<'a, T: 'a> Iterator for ChildNodeIteratorMut<'a, T> { type Item = &'a mut T; fn next(&mut self) -> Option { - let owned = self.children_ids.get(self.index).copied(); - match owned { - Some(id) => { - self.index += 1; - - Some(self.tree_mut().get_mut_unchecked(id)) - } - None => None, - } + self.nodes.pop().map(|node| &mut node.value) } } impl TreeView for Tree { - type Iterator<'a> = ChildNodeIterator<'a, T, Tree> where T: 'a; - type IteratorMut<'a> = ChildNodeIteratorMut<'a, T, Tree> where T: 'a; + type Iterator<'a> = ChildNodeIterator<'a, T> where T: 'a; + type IteratorMut<'a> = ChildNodeIteratorMut<'a, T> where T: 'a; fn root(&self) -> NodeId { self.root @@ -265,24 +229,26 @@ impl TreeView for Tree { fn children(&self, id: NodeId) -> Option> { self.children_ids(id).map(|children_ids| ChildNodeIterator { - tree: self, - children_ids, + nodes: &self.nodes, + children_ids: children_ids.to_vec(), index: 0, node_type: PhantomData, }) } fn children_mut(&mut self, id: NodeId) -> Option> { - let raw_ptr = self as *mut Self; - unsafe { - // Safety: No node will appear as a child twice - self.children_ids(id) - .map(|children_ids| ChildNodeIteratorMut { - tree: &mut *raw_ptr, - children_ids, - index: 0, - node_type: PhantomData, - }) + // Safety: No node has itself as a parent. + if let Some(children_ids) = self.children_ids(id) { + let children_ids = children_ids.to_vec(); + Some(ChildNodeIteratorMut { + nodes: unsafe { + self.nodes + .get_many_mut_unchecked(children_ids.into_iter().rev().map(|id| id.0)) + .unwrap() + }, + }) + } else { + None } } @@ -340,6 +306,30 @@ impl TreeView for Tree { self.nodes.get_mut(id.0).map(|node| (&mut node.value, None)) } } + + fn parent_child_mut(&mut self, id: NodeId) -> Option<(&mut T, Self::IteratorMut<'_>)> { + // Safety: No node will appear as a child twice + if let Some(children_ids) = self.children_ids(id) { + println!("parent_child_mut: {:?}\n{:?}", id, children_ids); + debug_assert!(!children_ids.iter().any(|child_id| *child_id == id)); + let mut borrowed = unsafe { + self.nodes + .get_many_mut_unchecked( + children_ids + .to_vec() + .into_iter() + .rev() + .map(|id| id.0) + .chain(std::iter::once(id.0)), + ) + .unwrap() + }; + let node = &mut borrowed.pop().unwrap().value; + Some((node, ChildNodeIteratorMut { nodes: borrowed })) + } else { + None + } + } } impl TreeLike for Tree { @@ -435,129 +425,6 @@ impl TreeLike for Tree { self.set_height(new, height); } } - -pub struct TreeMap<'a, T1, T2, Tr, F, FMut> -where - Tr: TreeView, - F: Fn(&T1) -> &T2, - FMut: Fn(&mut T1) -> &mut T2, -{ - tree: &'a mut Tr, - map: F, - map_mut: FMut, - in_node_type: PhantomData, - out_node_type: PhantomData, -} - -impl<'a, T1, T2, Tr, F, FMut> TreeMap<'a, T1, T2, Tr, F, FMut> -where - Tr: TreeView, - F: Fn(&T1) -> &T2, - FMut: Fn(&mut T1) -> &mut T2, -{ - pub fn new(tree: &'a mut Tr, map: F, map_mut: FMut) -> Self { - TreeMap { - tree, - map, - map_mut, - in_node_type: PhantomData, - out_node_type: PhantomData, - } - } -} - -impl<'a, T1, T2, Tr, F, FMut> TreeView for TreeMap<'a, T1, T2, Tr, F, FMut> -where - Tr: TreeView, - F: Fn(&T1) -> &T2, - FMut: Fn(&mut T1) -> &mut T2, -{ - type Iterator<'b> = ChildNodeIterator<'b, T2, TreeMap<'a, T1, T2, Tr, F, FMut>> - where - T2: 'b, - Self:'b; - type IteratorMut<'b> = ChildNodeIteratorMut<'b, T2, TreeMap<'a, T1, T2, Tr, F, FMut>> - where - T2: 'b, - Self:'b; - - fn root(&self) -> NodeId { - self.tree.root() - } - - fn get(&self, id: NodeId) -> Option<&T2> { - self.tree.get(id).map(|node| (self.map)(node)) - } - - fn get_mut(&mut self, id: NodeId) -> Option<&mut T2> { - self.tree.get_mut(id).map(|node| (self.map_mut)(node)) - } - - fn children(&self, id: NodeId) -> Option> { - self.children_ids(id).map(|children_ids| ChildNodeIterator { - tree: self, - children_ids, - index: 0, - node_type: PhantomData, - }) - } - - fn children_mut(&mut self, id: NodeId) -> Option> { - let raw_ptr = self as *mut Self; - unsafe { - // Safety: No node can be a child twice. - self.children_ids(id) - .map(|children_ids| ChildNodeIteratorMut { - tree: &mut *raw_ptr, - children_ids, - index: 0, - node_type: PhantomData, - }) - } - } - - fn children_ids(&self, id: NodeId) -> Option<&[NodeId]> { - self.tree.children_ids(id) - } - - fn parent(&self, id: NodeId) -> Option<&T2> { - self.tree.parent(id).map(|node| (self.map)(node)) - } - - fn parent_mut(&mut self, id: NodeId) -> Option<&mut T2> { - self.tree.parent_mut(id).map(|node| (self.map_mut)(node)) - } - - fn parent_id(&self, id: NodeId) -> Option { - self.tree.parent_id(id) - } - - fn height(&self, id: NodeId) -> Option { - self.tree.height(id) - } - - fn get_unchecked(&self, id: NodeId) -> &T2 { - (self.map)(self.tree.get_unchecked(id)) - } - - fn get_mut_unchecked(&mut self, id: NodeId) -> &mut T2 { - (self.map_mut)(self.tree.get_mut_unchecked(id)) - } - - fn size(&self) -> usize { - self.tree.size() - } - - fn node_parent_mut(&mut self, id: NodeId) -> Option<(&mut T2, Option<&mut T2>)> { - self.tree.node_parent_mut(id).map(|(node, parent)| { - ( - (self.map_mut)(node), - parent.map(|parent| (self.map_mut)(parent)), - ) - }) - } -} - #[test] fn creation() { let mut tree = Tree::new(1); @@ -666,59 +533,6 @@ fn deletion() { assert_eq!(tree.children_ids(parent).unwrap(), &[]); } -#[test] -fn shared_view() { - use std::thread; - let mut tree = Tree::new(1); - let parent = tree.root(); - let child = tree.create_node(0); - tree.add_child(parent, child); - - let shared = SharedView::new(&mut tree); - - thread::scope(|s| { - let (mut shared1, mut shared2, mut shared3) = - (shared.clone(), shared.clone(), shared.clone()); - s.spawn(move || { - assert_eq!(*shared1.get_mut(parent).unwrap(), 1); - assert_eq!(*shared1.get_mut(child).unwrap(), 0); - }); - s.spawn(move || { - assert_eq!(*shared2.get_mut(child).unwrap(), 0); - assert_eq!(*shared2.get_mut(parent).unwrap(), 1); - }); - s.spawn(move || { - assert_eq!(*shared3.get_mut(parent).unwrap(), 1); - assert_eq!(*shared3.get_mut(child).unwrap(), 0); - }); - }); -} - -#[test] -fn map() { - #[derive(Debug, PartialEq)] - struct Value { - value: i32, - } - impl Value { - fn new(value: i32) -> Self { - Self { value } - } - } - let mut tree = Tree::new(Value::new(1)); - let parent = tree.root(); - let child = tree.create_node(Value::new(0)); - tree.add_child(parent, child); - - let mut mapped = tree.map(|x| &x.value, |x| &mut x.value); - - *mapped.get_mut(child).unwrap() = 1; - *mapped.get_mut(parent).unwrap() = 2; - - assert_eq!(*tree.get(parent).unwrap(), Value::new(2)); - assert_eq!(*tree.get(child).unwrap(), Value::new(1)); -} - #[test] fn traverse_depth_first() { let mut tree = Tree::new(0); @@ -738,3 +552,124 @@ fn traverse_depth_first() { node_count += 1; }); } + +#[test] +fn get_node_children_mut() { + let mut tree = Tree::new(0); + let parent = tree.root(); + let child1 = tree.create_node(1); + tree.add_child(parent, child1); + let child2 = tree.create_node(2); + tree.add_child(parent, child2); + let child3 = tree.create_node(3); + tree.add_child(parent, child3); + + let (parent, children) = tree.parent_child_mut(parent).unwrap(); + for (i, child) in children.enumerate() { + assert_eq!(*child, i + 1); + } + println!("Parent: {:#?}", parent); +} + +#[test] +fn get_many_mut_unchecked() { + let mut slab = Slab::new(); + let parent = slab.insert(0); + let child = slab.insert(1); + let grandchild = slab.insert(2); + + let all = + unsafe { slab.get_many_mut_unchecked([parent, child, grandchild].into_iter()) }.unwrap(); + println!("All: {:#?}", all); +} + +#[derive(Debug)] +struct Slab { + data: Vec>, + free: VecDeque, +} + +impl Default for Slab { + fn default() -> Self { + Self::new() + } +} + +impl Slab { + fn new() -> Self { + Self { + data: Vec::new(), + free: VecDeque::new(), + } + } + + fn get(&self, id: usize) -> Option<&T> { + self.data.get(id).and_then(|x| x.as_ref()) + } + + unsafe fn get_unchecked(&self, id: usize) -> &T { + self.data.get_unchecked(id).as_ref().unwrap() + } + + fn get_mut(&mut self, id: usize) -> Option<&mut T> { + self.data.get_mut(id).and_then(|x| x.as_mut()) + } + + unsafe fn get_unchecked_mut(&mut self, id: usize) -> &mut T { + self.data.get_unchecked_mut(id).as_mut().unwrap() + } + + fn get2_mut(&mut self, id1: usize, id2: usize) -> Option<(&mut T, &mut T)> { + assert!(id1 != id2); + let ptr = self.data.as_mut_ptr(); + let first = unsafe { &mut *ptr.add(id1) }; + let second = unsafe { &mut *ptr.add(id2) }; + if let (Some(first), Some(second)) = (first, second) { + Some((first, second)) + } else { + None + } + } + + unsafe fn get_many_mut_unchecked( + &mut self, + ids: impl Iterator, + ) -> Option> { + let ptr = self.data.as_mut_ptr(); + let mut result = Vec::new(); + for id in ids { + let item = unsafe { &mut *ptr.add(id) }; + if let Some(item) = item { + result.push(item); + } else { + return None; + } + } + Some(result) + } + + fn insert(&mut self, value: T) -> usize { + if let Some(id) = self.free.pop_front() { + self.data[id] = Some(value); + id + } else { + self.data.push(Some(value)); + self.data.len() - 1 + } + } + + fn try_remove(&mut self, id: usize) -> Option { + self.data.get_mut(id).and_then(|x| { + self.free.push_back(id); + x.take() + }) + } + + fn remove(&mut self, id: usize) -> T { + self.try_remove(id).unwrap() + } + + fn len(&self) -> usize { + self.data.len() - self.free.len() + } +} diff --git a/packages/tui/Cargo.toml b/packages/tui/Cargo.toml index 94aaf3296..154988f95 100644 --- a/packages/tui/Cargo.toml +++ b/packages/tui/Cargo.toml @@ -27,7 +27,7 @@ futures = "0.3.19" taffy = "0.2.1" smallvec = "1.6" rustc-hash = "1.1.0" -anymap = "0.12.1" +anymap = "1.0.0-beta.2" futures-channel = "0.3.25" [dev-dependencies] From 613a7ac55b88ed77bdd840e943a1cb5d3b54849e Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 2 Jan 2023 11:52:42 -0600 Subject: [PATCH 21/25] add comment about multithreading --- packages/native-core/src/passes.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/native-core/src/passes.rs b/packages/native-core/src/passes.rs index c438828b0..44331ae37 100644 --- a/packages/native-core/src/passes.rs +++ b/packages/native-core/src/passes.rs @@ -317,6 +317,7 @@ pub fn resolve_passes + Sync + Send>( ctx: SendAnyMap, ) -> FxDashSet { resolve_passes_single_threaded(tree, dirty_nodes, passes, ctx) + // TODO: multithreadeding has some safety issues currently that need to be resolved before it can be used // let dirty_states = Arc::new(dirty_nodes); // let mut resolved_passes: FxHashSet = FxHashSet::default(); // let mut resolving = Vec::new(); From 87ff41f213cc2e3d7862a4c539e2d13b7a43a5d0 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 2 Jan 2023 11:54:44 -0600 Subject: [PATCH 22/25] remove some commented out code --- packages/core/src/create.rs | 35 ----------------------------------- 1 file changed, 35 deletions(-) diff --git a/packages/core/src/create.rs b/packages/core/src/create.rs index 4e2d8f0a2..30ee00f41 100644 --- a/packages/core/src/create.rs +++ b/packages/core/src/create.rs @@ -292,41 +292,6 @@ impl<'b> VirtualDom { // Safety: we promise not to re-alias this text later on after committing it to the mutation let unbounded_name: &str = unsafe { std::mem::transmute(attribute.name) }; - // match &attribute.value { - // AttributeValue::Text(value) => { - // // Safety: we promise not to re-alias this text later on after committing it to the mutation - // let unbounded_value: &str = unsafe { std::mem::transmute(*value) }; - - // self.mutations.push(SetAttribute { - // name: unbounded_name, - // value: unbounded_value, - // ns: attribute.namespace, - // id, - // }) - // } - // AttributeValue::Bool(value) => self.mutations.push(SetBoolAttribute { - // name: unbounded_name, - // value: *value, - // id, - // }), - // AttributeValue::Listener(_) => { - // self.mutations.push(NewEventListener { - // // all listeners start with "on" - // name: &unbounded_name[2..], - // id, - // }) - // } - // _ => { - - // } - // AttributeValue::Float(_) => todo!(), - // AttributeValue::Int(_) => todo!(), - // AttributeValue::Any() => todo!(), - // AttributeValue::None => todo!(), - // } - - // Safety: we promise not to re-alias this text later on after committing it to the mutation - match &attribute.value { AttributeValue::Listener(_) => { self.mutations.push(NewEventListener { From 64db1def9f7b015143f79337396ff148e0a4cbce Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 2 Jan 2023 12:37:16 -0600 Subject: [PATCH 23/25] make clippy happy --- packages/native-core/src/tree.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/native-core/src/tree.rs b/packages/native-core/src/tree.rs index f55066fc4..a10cf1714 100644 --- a/packages/native-core/src/tree.rs +++ b/packages/native-core/src/tree.rs @@ -1,10 +1,5 @@ -use core::panic; -use parking_lot::lock_api::RawMutex as _; -use parking_lot::{RawMutex, RwLock}; -use std::cell::UnsafeCell; use std::collections::VecDeque; use std::marker::PhantomData; -use std::sync::Arc; #[derive(Hash, PartialEq, Eq, Clone, Copy, Debug, PartialOrd, Ord)] pub struct NodeId(pub usize); @@ -313,10 +308,10 @@ impl TreeView for Tree { println!("parent_child_mut: {:?}\n{:?}", id, children_ids); debug_assert!(!children_ids.iter().any(|child_id| *child_id == id)); let mut borrowed = unsafe { + let as_vec = children_ids.to_vec(); self.nodes .get_many_mut_unchecked( - children_ids - .to_vec() + as_vec .into_iter() .rev() .map(|id| id.0) From 51dba50c04fdb7b9d39499e4c14aabf8feed5bdb Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 2 Jan 2023 14:40:25 -0600 Subject: [PATCH 24/25] use BumpBox and FromAnyValue instead of sync bounds --- packages/core/Cargo.toml | 4 +- packages/core/src/arena.rs | 6 +- packages/core/src/create.rs | 7 +- packages/core/src/diff.rs | 8 +- packages/core/src/lib.rs | 8 +- packages/core/src/mutations.rs | 4 +- packages/core/src/nodes.rs | 205 ++++++++---------- packages/core/src/scopes.rs | 4 +- packages/core/src/virtual_dom.rs | 2 +- packages/core/tests/attr_cleanup.rs | 15 +- packages/core/tests/boolattrs.rs | 3 +- packages/core/tests/kitchen_sink.rs | 2 +- .../native-core-macro/tests/update_state.rs | 1 - packages/native-core/Cargo.toml | 5 - packages/native-core/src/node.rs | 64 +++--- packages/native-core/src/node_ref.rs | 14 +- packages/native-core/src/state.rs | 14 +- packages/native-core/src/tree.rs | 1 - .../src/utils/persistant_iterator.rs | 2 +- packages/web/src/dom.rs | 15 +- 20 files changed, 184 insertions(+), 200 deletions(-) diff --git a/packages/core/Cargo.toml b/packages/core/Cargo.toml index 5bf348b33..7158b3970 100644 --- a/packages/core/Cargo.toml +++ b/packages/core/Cargo.toml @@ -44,6 +44,4 @@ pretty_assertions = "1.3.0" [features] default = [] -serialize = ["serde"] -# Adds a Sync + Send bound on any values in attributes -sync_attributes = [] \ No newline at end of file +serialize = ["serde"] \ No newline at end of file diff --git a/packages/core/src/arena.rs b/packages/core/src/arena.rs index aef8e8b2d..e5440fd85 100644 --- a/packages/core/src/arena.rs +++ b/packages/core/src/arena.rs @@ -158,10 +158,10 @@ impl VirtualDom { let listener = unsafe { &*listener }; match &listener.value { AttributeValue::Listener(l) => { - _ = l.0.take(); + _ = l.take(); } - AttributeValue::Any(a) => { - _ = a.take(); + AttributeValue::Any(_) => { + todo!() } _ => (), } diff --git a/packages/core/src/create.rs b/packages/core/src/create.rs index 30ee00f41..7e5633f12 100644 --- a/packages/core/src/create.rs +++ b/packages/core/src/create.rs @@ -1,5 +1,5 @@ use crate::any_props::AnyProps; -use crate::innerlude::{VComponent, VPlaceholder, VText}; +use crate::innerlude::{BorrowedAttributeValue, VComponent, VPlaceholder, VText}; use crate::mutations::Mutation; use crate::mutations::Mutation::*; use crate::nodes::VNode; @@ -285,7 +285,7 @@ impl<'b> VirtualDom { } } - fn write_attribute(&mut self, attribute: &crate::Attribute, id: ElementId) { + fn write_attribute(&mut self, attribute: &'b crate::Attribute<'b>, id: ElementId) { // Make sure we set the attribute's associated id attribute.mounted_element.set(id); @@ -302,7 +302,8 @@ impl<'b> VirtualDom { } _ => { // Safety: we promise not to re-alias this text later on after committing it to the mutation - let unbounded_value = unsafe { std::mem::transmute(attribute.value.clone()) }; + let value: BorrowedAttributeValue<'b> = (&attribute.value).into(); + let unbounded_value = unsafe { std::mem::transmute(value) }; self.mutations.push(SetAttribute { name: unbounded_name, diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index f055fb650..7227ee6e7 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -1,7 +1,7 @@ use crate::{ any_props::AnyProps, arena::ElementId, - innerlude::{DirtyScope, VComponent, VPlaceholder, VText}, + innerlude::{BorrowedAttributeValue, DirtyScope, VComponent, VPlaceholder, VText}, mutations::Mutation, nodes::RenderReturn, nodes::{DynamicNode, VNode}, @@ -150,10 +150,10 @@ impl<'b> VirtualDom { }; } - fn update_attribute(&mut self, right_attr: &Attribute, left_attr: &Attribute) { - // todo: add more types of attribute values + fn update_attribute(&mut self, right_attr: &'b Attribute<'b>, left_attr: &'b Attribute) { let name = unsafe { std::mem::transmute(left_attr.name) }; - let value = unsafe { std::mem::transmute(right_attr.value.clone()) }; + let value: BorrowedAttributeValue<'b> = (&right_attr.value).into(); + let value = unsafe { std::mem::transmute(value) }; self.mutations.push(Mutation::SetAttribute { id: left_attr.mounted_element.get(), ns: right_attr.namespace, diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index b6fe456bc..4e26f5a56 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -70,10 +70,10 @@ pub(crate) mod innerlude { } pub use crate::innerlude::{ - fc_to_builder, AnyValueContainer, Attribute, AttributeValue, CapturedError, Component, - DynamicNode, Element, ElementId, Event, Fragment, IntoDynNode, LazyNodes, Mutation, Mutations, - Properties, RenderReturn, Scope, ScopeId, ScopeState, Scoped, SuspenseContext, TaskId, - Template, TemplateAttribute, TemplateNode, VComponent, VNode, VText, VirtualDom, + fc_to_builder, AnyValue, Attribute, AttributeValue, BorrowedAttributeValue, CapturedError, + Component, DynamicNode, Element, ElementId, Event, Fragment, IntoDynNode, LazyNodes, Mutation, + Mutations, Properties, RenderReturn, Scope, ScopeId, ScopeState, Scoped, SuspenseContext, + TaskId, Template, TemplateAttribute, TemplateNode, VComponent, VNode, VText, VirtualDom, }; /// The purpose of this module is to alleviate imports of many common types diff --git a/packages/core/src/mutations.rs b/packages/core/src/mutations.rs index 06116f5dc..b02e89581 100644 --- a/packages/core/src/mutations.rs +++ b/packages/core/src/mutations.rs @@ -1,6 +1,6 @@ use rustc_hash::FxHashSet; -use crate::{arena::ElementId, AttributeValue, ScopeId, Template}; +use crate::{arena::ElementId, innerlude::BorrowedAttributeValue, ScopeId, Template}; /// A container for all the relevant steps to modify the Real DOM /// @@ -208,7 +208,7 @@ pub enum Mutation<'a> { name: &'a str, /// The value of the attribute. - value: AttributeValue<'a>, + value: BorrowedAttributeValue<'a>, /// The ID of the node to set the attribute of. id: ElementId, diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 0c797460a..d17863c20 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -6,9 +6,8 @@ use bumpalo::Bump; use std::{ any::{Any, TypeId}, cell::{Cell, RefCell, UnsafeCell}, - fmt::Arguments, + fmt::{Arguments, Debug}, future::Future, - ops::Deref, }; pub type TemplateId = &'static str; @@ -200,7 +199,7 @@ impl<'a> VNode<'a> { pub(crate) fn clear_listeners(&self) { for attr in self.dynamic_attrs { if let AttributeValue::Listener(l) = &attr.value { - l.0.borrow_mut().take(); + l.borrow_mut().take(); } } } @@ -507,9 +506,6 @@ pub struct Attribute<'a> { /// /// These are built-in to be faster during the diffing process. To use a custom value, use the [`AttributeValue::Any`] /// variant. -#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "serialize", serde(untagged))] -#[derive(Clone)] pub enum AttributeValue<'a> { /// Text attribute Text(&'a str), @@ -524,107 +520,105 @@ pub enum AttributeValue<'a> { Bool(bool), /// A listener, like "onclick" - Listener(ListenerCb<'a>), + Listener(RefCell>>), /// An arbitrary value that implements PartialEq and is static - Any(RefCell>), + Any(BumpBox<'a, dyn AnyValue>), /// A "none" value, resulting in the removal of an attribute from the dom None, } -pub type ListenerCbInner<'a> = RefCell) + 'a>>>; -pub struct ListenerCb<'a>(pub ListenerCbInner<'a>); +pub type ListenerCb<'a> = BumpBox<'a, dyn FnMut(Event) + 'a>; -impl Clone for ListenerCb<'_> { - fn clone(&self) -> Self { - panic!("ListenerCb cannot be cloned") +/// Any of the built-in values that the Dioxus VirtualDom supports as dynamic attributes on elements that are borrowed +/// +/// These varients are used to communicate what the value of an attribute is that needs to be updated +#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "serialize", serde(untagged))] +pub enum BorrowedAttributeValue<'a> { + /// Text attribute + Text(&'a str), + + /// A float + Float(f64), + + /// Signed integer + Int(i64), + + /// Boolean + Bool(bool), + + /// An arbitrary value that implements PartialEq and is static + #[cfg_attr( + feature = "serialize", + serde( + deserialize_with = "deserialize_any_value", + serialize_with = "serialize_any_value" + ) + )] + Any(&'a dyn AnyValue), + + /// A "none" value, resulting in the removal of an attribute from the dom + None, +} + +impl<'a> From<&'a AttributeValue<'a>> for BorrowedAttributeValue<'a> { + fn from(value: &'a AttributeValue<'a>) -> Self { + match value { + AttributeValue::Text(value) => BorrowedAttributeValue::Text(value), + AttributeValue::Float(value) => BorrowedAttributeValue::Float(*value), + AttributeValue::Int(value) => BorrowedAttributeValue::Int(*value), + AttributeValue::Bool(value) => BorrowedAttributeValue::Bool(*value), + AttributeValue::Listener(_) => { + panic!("A listener cannot be turned into a borrowed value") + } + AttributeValue::Any(value) => BorrowedAttributeValue::Any(&**value), + AttributeValue::None => BorrowedAttributeValue::None, + } } } -#[cfg(feature = "serialize")] -impl<'a> serde::Serialize for ListenerCb<'a> { - fn serialize(&self, _: S) -> Result - where - S: serde::Serializer, - { - panic!("ListenerCb cannot be serialized") +impl Debug for BorrowedAttributeValue<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Text(arg0) => f.debug_tuple("Text").field(arg0).finish(), + Self::Float(arg0) => f.debug_tuple("Float").field(arg0).finish(), + Self::Int(arg0) => f.debug_tuple("Int").field(arg0).finish(), + Self::Bool(arg0) => f.debug_tuple("Bool").field(arg0).finish(), + Self::Any(_) => f.debug_tuple("Any").field(&"...").finish(), + Self::None => write!(f, "None"), + } } } -#[cfg(feature = "serialize")] -impl<'de, 'a> serde::Deserialize<'de> for ListenerCb<'a> { - fn deserialize(_: D) -> Result - where - D: serde::Deserializer<'de>, - { - panic!("ListenerCb cannot be deserialized") - } -} - -#[derive(Clone)] -#[cfg(not(feature = "sync_attributes"))] -/// A boxed value that implements PartialEq, and Any -pub struct AnyValueContainer(pub std::rc::Rc); - -#[derive(Clone)] -#[cfg(feature = "sync_attributes")] -/// A boxed value that implements PartialEq, Any, Sync, and Send -pub struct AnyValueContainer(pub std::sync::Arc); - -impl PartialEq for AnyValueContainer { +impl PartialEq for BorrowedAttributeValue<'_> { fn eq(&self, other: &Self) -> bool { - self.0.any_cmp(other.0.as_ref()) - } -} - -impl AnyValueContainer { - /// Create a new AnyValueContainer containing the specified data. - pub fn new(value: T) -> Self { - #[cfg(feature = "sync_attributes")] - return Self(std::sync::Arc::new(value)); - #[cfg(not(feature = "sync_attributes"))] - return Self(std::rc::Rc::new(value)); - } - - /// Returns a reference to the inner value. - pub fn downcast_ref(&self) -> Option<&T> { - self.0.deref().as_any().downcast_ref() - } - - /// Checks if the type of the inner value is 'T'. - pub fn is(&self) -> bool { - self.0.deref().as_any().is::() - } -} - -#[test] -fn test_any_value_rc() { - let a = AnyValueContainer::new(1i32); - assert_eq!(a.downcast_ref::(), Some(&1i32)); - assert_eq!(a.downcast_ref::(), None); - assert!(a.is::()); - assert!(!a.is::()); -} - -#[cfg(feature = "serialize")] -impl serde::Serialize for AnyValueContainer { - fn serialize(&self, _: S) -> Result - where - S: serde::Serializer, - { - panic!("AnyValueBox cannot be serialized") + match (self, other) { + (Self::Text(l0), Self::Text(r0)) => l0 == r0, + (Self::Float(l0), Self::Float(r0)) => l0 == r0, + (Self::Int(l0), Self::Int(r0)) => l0 == r0, + (Self::Bool(l0), Self::Bool(r0)) => l0 == r0, + (Self::Any(l0), Self::Any(r0)) => l0.any_cmp(&**r0), + _ => core::mem::discriminant(self) == core::mem::discriminant(other), + } } } #[cfg(feature = "serialize")] -impl<'de> serde::Deserialize<'de> for AnyValueContainer { - fn deserialize(_: D) -> Result - where - D: serde::Deserializer<'de>, - { - panic!("AnyValueBox cannot be deserialized") - } +fn serialize_any_value(_: &&dyn AnyValue, _: S) -> Result +where + S: serde::Serializer, +{ + panic!("Any cannot be serialized") +} + +#[cfg(feature = "serialize")] +fn deserialize_any_value<'de, 'a, D>(_: D) -> Result<&'a dyn AnyValue, D::Error> +where + D: serde::Deserializer<'de>, +{ + panic!("Any cannot be deserialized") } impl<'a> std::fmt::Debug for AttributeValue<'a> { @@ -649,41 +643,13 @@ impl<'a> PartialEq for AttributeValue<'a> { (Self::Int(l0), Self::Int(r0)) => l0 == r0, (Self::Bool(l0), Self::Bool(r0)) => l0 == r0, (Self::Listener(_), Self::Listener(_)) => true, - (Self::Any(l0), Self::Any(r0)) => l0 == r0, + (Self::Any(l0), Self::Any(r0)) => l0.any_cmp(&**r0), _ => false, } } } -// Some renderers need attributes to be sync and send. The rest of the attributes are already sync and send, but there is no way to force Any values to be sync and send on the renderer side. -// The sync_attributes flag restricts any valuse to be sync and send. #[doc(hidden)] -#[cfg(feature = "sync_attributes")] -pub trait AnyValue: Sync + Send + 'static { - fn any_cmp(&self, other: &dyn AnyValue) -> bool; - fn as_any(&self) -> &dyn Any; - fn type_id(&self) -> TypeId { - self.as_any().type_id() - } -} - -#[cfg(feature = "sync_attributes")] -impl AnyValue for T { - fn any_cmp(&self, other: &dyn AnyValue) -> bool { - if let Some(other) = other.as_any().downcast_ref() { - self == other - } else { - false - } - } - - fn as_any(&self) -> &dyn Any { - self - } -} - -#[doc(hidden)] -#[cfg(not(feature = "sync_attributes"))] pub trait AnyValue: 'static { fn any_cmp(&self, other: &dyn AnyValue) -> bool; fn as_any(&self) -> &dyn Any; @@ -692,7 +658,6 @@ pub trait AnyValue: 'static { } } -#[cfg(not(feature = "sync_attributes"))] impl AnyValue for T { fn any_cmp(&self, other: &dyn AnyValue) -> bool { if let Some(other) = other.as_any().downcast_ref() { @@ -903,8 +868,8 @@ impl<'a> IntoAttributeValue<'a> for Arguments<'_> { } } -impl<'a> IntoAttributeValue<'a> for AnyValueContainer { +impl<'a> IntoAttributeValue<'a> for BumpBox<'a, dyn AnyValue> { fn into_value(self, _: &'a Bump) -> AttributeValue<'a> { - AttributeValue::Any(RefCell::new(Some(self))) + AttributeValue::Any(self) } } diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index 20be78cce..d07b52704 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -4,7 +4,7 @@ use crate::{ arena::ElementId, bump_frame::BumpFrame, innerlude::{DynamicNode, EventHandler, VComponent, VText}, - innerlude::{ErrorBoundary, ListenerCb, Scheduler, SchedulerMsg}, + innerlude::{ErrorBoundary, Scheduler, SchedulerMsg}, lazynodes::LazyNodes, nodes::{ComponentReturn, IntoAttributeValue, IntoDynNode, RenderReturn}, Attribute, AttributeValue, Element, Event, Properties, TaskId, @@ -510,7 +510,7 @@ impl<'src> ScopeState { })) }; - AttributeValue::Listener(ListenerCb(RefCell::new(Some(boxed)))) + AttributeValue::Listener(RefCell::new(Some(boxed))) } /// Inject an error into the nearest error boundary and quit rendering diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index 56363788f..043e7fafb 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -384,7 +384,7 @@ impl VirtualDom { // We check the bubble state between each call to see if the event has been stopped from bubbling for listener in listeners.drain(..).rev() { if let AttributeValue::Listener(listener) = listener { - if let Some(cb) = listener.0.borrow_mut().as_deref_mut() { + if let Some(cb) = listener.borrow_mut().as_deref_mut() { cb(uievent.clone()); } diff --git a/packages/core/tests/attr_cleanup.rs b/packages/core/tests/attr_cleanup.rs index fb67d0527..2e3cf5d17 100644 --- a/packages/core/tests/attr_cleanup.rs +++ b/packages/core/tests/attr_cleanup.rs @@ -5,7 +5,7 @@ use bumpalo::Bump; use dioxus::core::{ElementId, Mutation::*}; use dioxus::prelude::*; -use dioxus_core::AttributeValue; +use dioxus_core::{AttributeValue, BorrowedAttributeValue}; #[test] fn attrs_cycle() { @@ -42,11 +42,16 @@ fn attrs_cycle() { AssignId { path: &[0,], id: ElementId(3,) }, SetAttribute { name: "class", - value: "1".into_value(&bump), + value: (&*bump.alloc("1".into_value(&bump))).into(), + id: ElementId(3,), + ns: None + }, + SetAttribute { + name: "id", + value: (&*bump.alloc("1".into_value(&bump))).into(), id: ElementId(3,), ns: None }, - SetAttribute { name: "id", value: "1".into_value(&bump), id: ElementId(3,), ns: None }, ReplaceWith { id: ElementId(1,), m: 1 }, ] ); @@ -68,13 +73,13 @@ fn attrs_cycle() { AssignId { path: &[0], id: ElementId(3) }, SetAttribute { name: "class", - value: AttributeValue::Text("3"), + value: BorrowedAttributeValue::Text("3"), id: ElementId(3), ns: None }, SetAttribute { name: "id", - value: AttributeValue::Text("3"), + value: BorrowedAttributeValue::Text("3"), id: ElementId(3), ns: None }, diff --git a/packages/core/tests/boolattrs.rs b/packages/core/tests/boolattrs.rs index c2ad70333..0d02bf8af 100644 --- a/packages/core/tests/boolattrs.rs +++ b/packages/core/tests/boolattrs.rs @@ -6,13 +6,14 @@ use dioxus::prelude::*; fn bool_test() { let mut app = VirtualDom::new(|cx| cx.render(rsx!(div { hidden: false }))); let bump = Bump::new(); + assert_eq!( app.rebuild().santize().edits, [ LoadTemplate { name: "template", index: 0, id: ElementId(1) }, SetAttribute { name: "hidden", - value: false.into_value(&bump), + value: (&*bump.alloc(false.into_value(&bump))).into(), id: ElementId(1,), ns: None }, diff --git a/packages/core/tests/kitchen_sink.rs b/packages/core/tests/kitchen_sink.rs index 22ed1f26d..191d1da7c 100644 --- a/packages/core/tests/kitchen_sink.rs +++ b/packages/core/tests/kitchen_sink.rs @@ -36,7 +36,7 @@ fn dual_stream() { LoadTemplate { name: "template", index: 0, id: ElementId(1) }, SetAttribute { name: "class", - value: "123".into_value(&bump), + value: (&*bump.alloc("123".into_value(&bump))).into(), id: ElementId(1), ns: None, }, diff --git a/packages/native-core-macro/tests/update_state.rs b/packages/native-core-macro/tests/update_state.rs index e2f04c72c..fc36c42ed 100644 --- a/packages/native-core-macro/tests/update_state.rs +++ b/packages/native-core-macro/tests/update_state.rs @@ -43,7 +43,6 @@ impl ParentDepState for ParentDepCallCounter { const NODE_MASK: NodeMask = NodeMask::ALL; fn reduce(&mut self, _node: NodeView, _parent: Option<(&Self,)>, _ctx: &Self::Ctx) -> bool { self.0 += 1; - println!("ParentDepCallCounter::reduce on {:?}\n{}", _node, self.0); true } } diff --git a/packages/native-core/Cargo.toml b/packages/native-core/Cargo.toml index 5cc25b61c..9958b8a02 100644 --- a/packages/native-core/Cargo.toml +++ b/packages/native-core/Cargo.toml @@ -27,8 +27,3 @@ dashmap = "5.4.0" [dev-dependencies] rand = "0.8.5" dioxus = { path = "../dioxus", version = "^0.2.1" } - -[features] -default = ["parallel"] -# This makes core only accept sync and send any values for attributes to allow for parallel resolution of passes. -parallel = ["dioxus-core/sync_attributes"] \ No newline at end of file diff --git a/packages/native-core/src/node.rs b/packages/native-core/src/node.rs index 99777dc27..3b91cee45 100644 --- a/packages/native-core/src/node.rs +++ b/packages/native-core/src/node.rs @@ -1,44 +1,44 @@ use crate::{state::State, tree::NodeId}; -use dioxus_core::{AnyValueContainer, AttributeValue, ElementId}; +use dioxus_core::{AnyValue, BorrowedAttributeValue, ElementId}; use rustc_hash::{FxHashMap, FxHashSet}; use std::fmt::Debug; /// The node is stored client side and stores only basic data about the node. #[derive(Debug, Clone)] -pub struct Node { +pub struct Node { /// The transformed state of the node. pub state: S, /// The raw data for the node - pub node_data: NodeData, + pub node_data: NodeData, } #[derive(Debug, Clone)] -pub struct NodeData { +pub struct NodeData { /// The id of the node pub node_id: NodeId, /// The id of the node in the vdom. pub element_id: Option, /// Additional inforation specific to the node type - pub node_type: NodeType, + pub node_type: NodeType, } /// A type of node with data specific to the node type. The types are a subset of the [VNode] types. #[derive(Debug, Clone)] -pub enum NodeType { +pub enum NodeType { Text { text: String, }, Element { tag: String, namespace: Option, - attributes: FxHashMap, + attributes: FxHashMap>, listeners: FxHashSet, }, Placeholder, } -impl Node { - pub(crate) fn new(node_type: NodeType) -> Self { +impl Node { + pub(crate) fn new(node_type: NodeType) -> Self { Node { state: S::default(), node_data: NodeData { @@ -65,52 +65,59 @@ pub struct OwnedAttributeDiscription { /// An attribute on a DOM node, such as `id="my-thing"` or /// `href="https://example.com"`. #[derive(Clone, Copy, Debug)] -pub struct OwnedAttributeView<'a> { +pub struct OwnedAttributeView<'a, V: FromAnyValue = ()> { /// The discription of the attribute. pub attribute: &'a OwnedAttributeDiscription, /// The value of the attribute. - pub value: &'a OwnedAttributeValue, + pub value: &'a OwnedAttributeValue, } #[derive(Clone)] -pub enum OwnedAttributeValue { +pub enum OwnedAttributeValue { Text(String), Float(f64), Int(i64), Bool(bool), - Any(AnyValueContainer), + Custom(V), None, } -impl Debug for OwnedAttributeValue { +pub trait FromAnyValue { + fn from_any_value(value: &dyn AnyValue) -> Self; +} + +impl FromAnyValue for () { + fn from_any_value(_: &dyn AnyValue) -> Self {} +} + +impl Debug for OwnedAttributeValue { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::Text(arg0) => f.debug_tuple("Text").field(arg0).finish(), Self::Float(arg0) => f.debug_tuple("Float").field(arg0).finish(), Self::Int(arg0) => f.debug_tuple("Int").field(arg0).finish(), Self::Bool(arg0) => f.debug_tuple("Bool").field(arg0).finish(), - Self::Any(_) => f.debug_tuple("Any").finish(), + Self::Custom(_) => f.debug_tuple("Any").finish(), Self::None => write!(f, "None"), } } } -impl From> for OwnedAttributeValue { - fn from(value: AttributeValue<'_>) -> Self { +impl From> for OwnedAttributeValue { + fn from(value: BorrowedAttributeValue<'_>) -> Self { match value { - AttributeValue::Text(text) => Self::Text(text.to_string()), - AttributeValue::Float(float) => Self::Float(float), - AttributeValue::Int(int) => Self::Int(int), - AttributeValue::Bool(bool) => Self::Bool(bool), - AttributeValue::Any(any) => Self::Any(any.borrow().as_ref().unwrap().clone()), - AttributeValue::None => Self::None, - _ => Self::None, + BorrowedAttributeValue::Text(text) => Self::Text(text.to_string()), + BorrowedAttributeValue::Float(float) => Self::Float(float), + BorrowedAttributeValue::Int(int) => Self::Int(int), + BorrowedAttributeValue::Bool(bool) => Self::Bool(bool), + BorrowedAttributeValue::Any(any) => Self::Custom(V::from_any_value(any)), + BorrowedAttributeValue::None => Self::None, } } } -impl OwnedAttributeValue { +impl OwnedAttributeValue { pub fn as_text(&self) -> Option<&str> { match self { OwnedAttributeValue::Text(text) => Some(text), @@ -145,4 +152,11 @@ impl OwnedAttributeValue { _ => None, } } + + pub fn as_custom(&self) -> Option<&V> { + match self { + OwnedAttributeValue::Custom(custom) => Some(custom), + _ => None, + } + } } diff --git a/packages/native-core/src/node_ref.rs b/packages/native-core/src/node_ref.rs index d178c2d18..8d4523efc 100644 --- a/packages/native-core/src/node_ref.rs +++ b/packages/native-core/src/node_ref.rs @@ -1,21 +1,21 @@ use dioxus_core::ElementId; use crate::{ - node::{NodeData, NodeType, OwnedAttributeView}, + node::{FromAnyValue, NodeData, NodeType, OwnedAttributeView}, state::union_ordered_iter, RealNodeId, }; /// A view into a [VNode] with limited access. #[derive(Debug)] -pub struct NodeView<'a> { - inner: &'a NodeData, +pub struct NodeView<'a, V: FromAnyValue = ()> { + inner: &'a NodeData, mask: NodeMask, } -impl<'a> NodeView<'a> { +impl<'a, V: FromAnyValue> NodeView<'a, V> { /// Create a new NodeView from a VNode, and mask. - pub fn new(node: &'a NodeData, view: NodeMask) -> Self { + pub fn new(node: &'a NodeData, view: NodeMask) -> Self { Self { inner: node, mask: view, @@ -55,7 +55,9 @@ impl<'a> NodeView<'a> { } /// Get any attributes that are enabled in the mask - pub fn attributes<'b>(&'b self) -> Option> + 'b> { + pub fn attributes<'b>( + &'b self, + ) -> Option> + 'b> { match &self.inner.node_type { NodeType::Element { attributes, .. } => Some( attributes diff --git a/packages/native-core/src/state.rs b/packages/native-core/src/state.rs index acdffa483..2e8eed085 100644 --- a/packages/native-core/src/state.rs +++ b/packages/native-core/src/state.rs @@ -1,6 +1,6 @@ use std::cmp::Ordering; -use crate::node::Node; +use crate::node::{FromAnyValue, Node}; use crate::node_ref::{NodeMask, NodeView}; use crate::passes::{resolve_passes, resolve_passes_single_threaded, AnyPass, DirtyNodeStates}; use crate::tree::TreeView; @@ -76,7 +76,7 @@ pub(crate) fn union_ordered_iter<'a>( /// } /// } /// ``` -pub trait ChildDepState { +pub trait ChildDepState { /// The context is passed to the [ChildDepState::reduce] when it is resolved. type Ctx; /// A state from each child node that this node depends on. Typically this is Self, but it could be any state that is within the state tree. @@ -87,7 +87,7 @@ pub trait ChildDepState { /// Resolve the state current node's state from the state of the children, the state of the node, and some external context. fn reduce<'a>( &mut self, - node: NodeView<'a>, + node: NodeView<'a, V>, children: impl Iterator::ElementBorrowed<'a>>, ctx: &Self::Ctx, ) -> bool @@ -138,7 +138,7 @@ pub trait ChildDepState { /// } /// } /// ``` -pub trait ParentDepState { +pub trait ParentDepState { /// The context is passed to the [ParentDepState::reduce] when it is resolved. type Ctx; /// A state from from the parent node that this node depends on. Typically this is Self, but it could be any state that is within the state tree. @@ -149,7 +149,7 @@ pub trait ParentDepState { /// Resolve the state current node's state from the state of the parent node, the state of the node, and some external context. fn reduce<'a>( &mut self, - node: NodeView<'a>, + node: NodeView<'a, V>, parent: Option<::ElementBorrowed<'a>>, ctx: &Self::Ctx, ) -> bool; @@ -193,7 +193,7 @@ pub trait ParentDepState { /// } /// } /// ``` -pub trait NodeDepState { +pub trait NodeDepState { /// Depstate must be a tuple containing any number of borrowed elements that are either [ChildDepState], [ParentDepState] or [NodeDepState]. type DepState: ElementBorrowable; /// The state passed to [NodeDepState::reduce] when it is resolved. @@ -203,7 +203,7 @@ pub trait NodeDepState { /// Resolve the state current node's state from the state of the sibling states, the state of the node, and some external context. fn reduce<'a>( &mut self, - node: NodeView<'a>, + node: NodeView<'a, V>, node_state: ::ElementBorrowed<'a>, ctx: &Self::Ctx, ) -> bool; diff --git a/packages/native-core/src/tree.rs b/packages/native-core/src/tree.rs index a10cf1714..760afe87b 100644 --- a/packages/native-core/src/tree.rs +++ b/packages/native-core/src/tree.rs @@ -305,7 +305,6 @@ impl TreeView for Tree { fn parent_child_mut(&mut self, id: NodeId) -> Option<(&mut T, Self::IteratorMut<'_>)> { // Safety: No node will appear as a child twice if let Some(children_ids) = self.children_ids(id) { - println!("parent_child_mut: {:?}\n{:?}", id, children_ids); debug_assert!(!children_ids.iter().any(|child_id| *child_id == id)); let mut borrowed = unsafe { let as_vec = children_ids.to_vec(); diff --git a/packages/native-core/src/utils/persistant_iterator.rs b/packages/native-core/src/utils/persistant_iterator.rs index 75bcf9064..7a34b90ad 100644 --- a/packages/native-core/src/utils/persistant_iterator.rs +++ b/packages/native-core/src/utils/persistant_iterator.rs @@ -229,7 +229,7 @@ impl PersistantElementIter { #[derive(Default, Clone, Debug)] struct Empty {} impl State for Empty { - const PASSES: &'static [crate::AnyPass>] = &[]; + const PASSES: &'static [crate::AnyPass>] = &[]; const MASKS: &'static [crate::NodeMask] = &[]; } diff --git a/packages/web/src/dom.rs b/packages/web/src/dom.rs index 887f70518..427adc742 100644 --- a/packages/web/src/dom.rs +++ b/packages/web/src/dom.rs @@ -7,7 +7,9 @@ //! - tests to ensure dyn_into works for various event types. //! - Partial delegation?> -use dioxus_core::{ElementId, Mutation, Template, TemplateAttribute, TemplateNode}; +use dioxus_core::{ + BorrowedAttributeValue, ElementId, Mutation, Template, TemplateAttribute, TemplateNode, +}; use dioxus_html::{event_bubbles, CompositionData, FormData}; use dioxus_interpreter_js::{save_template, Channel}; use futures_channel::mpsc; @@ -176,21 +178,24 @@ impl WebsysDom { id, ns, } => match value { - dioxus_core::AttributeValue::Text(txt) => { + BorrowedAttributeValue::Text(txt) => { i.set_attribute(id.0 as u32, name, txt, ns.unwrap_or_default()) } - dioxus_core::AttributeValue::Float(f) => { + BorrowedAttributeValue::Float(f) => { i.set_attribute(id.0 as u32, name, &f.to_string(), ns.unwrap_or_default()) } - dioxus_core::AttributeValue::Int(n) => { + BorrowedAttributeValue::Int(n) => { i.set_attribute(id.0 as u32, name, &n.to_string(), ns.unwrap_or_default()) } - dioxus_core::AttributeValue::Bool(b) => i.set_attribute( + BorrowedAttributeValue::Bool(b) => i.set_attribute( id.0 as u32, name, if *b { "true" } else { "false" }, ns.unwrap_or_default(), ), + BorrowedAttributeValue::None => { + i.remove_attribute(id.0 as u32, name, ns.unwrap_or_default()) + } _ => unreachable!(), }, SetText { value, id } => i.set_text(id.0 as u32, value), From c0f9355648d05de1d186d16ef1bb26ad3bc7df6d Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 2 Jan 2023 21:26:12 -0600 Subject: [PATCH 25/25] fix any value dropping --- packages/core/src/arena.rs | 4 ++-- packages/core/src/lib.rs | 6 +++--- packages/core/src/mutations.rs | 21 +-------------------- packages/core/src/nodes.rs | 31 +++++++++++++++++++++++-------- packages/core/src/scopes.rs | 13 ++++++++++++- packages/core/src/virtual_dom.rs | 2 +- packages/native-core/src/node.rs | 2 +- 7 files changed, 43 insertions(+), 36 deletions(-) diff --git a/packages/core/src/arena.rs b/packages/core/src/arena.rs index e5440fd85..a11fc69ba 100644 --- a/packages/core/src/arena.rs +++ b/packages/core/src/arena.rs @@ -160,8 +160,8 @@ impl VirtualDom { AttributeValue::Listener(l) => { _ = l.take(); } - AttributeValue::Any(_) => { - todo!() + AttributeValue::Any(a) => { + _ = a.take(); } _ => (), } diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index 4e26f5a56..c4f162580 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -81,9 +81,9 @@ pub use crate::innerlude::{ /// This includes types like [`Scope`], [`Element`], and [`Component`]. pub mod prelude { pub use crate::innerlude::{ - fc_to_builder, Component, Element, Event, EventHandler, Fragment, IntoAttributeValue, - LazyNodes, Properties, Scope, ScopeId, ScopeState, Scoped, TaskId, Template, - TemplateAttribute, TemplateNode, Throw, VNode, VirtualDom, + fc_to_builder, AnyValue, Component, Element, Event, EventHandler, Fragment, + IntoAttributeValue, LazyNodes, Properties, Scope, ScopeId, ScopeState, Scoped, TaskId, + Template, TemplateAttribute, TemplateNode, Throw, VNode, VirtualDom, }; } diff --git a/packages/core/src/mutations.rs b/packages/core/src/mutations.rs index b02e89581..551069c45 100644 --- a/packages/core/src/mutations.rs +++ b/packages/core/src/mutations.rs @@ -48,20 +48,7 @@ impl<'a> Mutations<'a> { /// Push a new mutation into the dom_edits list pub(crate) fn push(&mut self, mutation: Mutation<'static>) { - self.edits.push(mutation.downcast_lifetime()) - } -} - -impl Mutations<'static> { - pub(crate) fn take<'a>(&mut self) -> Mutations<'a> { - Mutations::<'a> { - subtree: self.subtree, - dirty_scopes: self.dirty_scopes.drain().collect(), - templates: self.templates.split_off(0), - // Safety: this downcasts the lifetime from 'static to 'a - // This is safe because 'static outlives 'a - edits: unsafe { std::mem::transmute(self.edits.split_off(0)) }, - } + self.edits.push(mutation) } } @@ -257,9 +244,3 @@ pub enum Mutation<'a> { id: ElementId, }, } - -impl Mutation<'static> { - pub(crate) fn downcast_lifetime<'a>(self) -> Mutation<'a> { - unsafe { std::mem::transmute(self) } - } -} diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index d17863c20..bb9f6734b 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -5,7 +5,7 @@ use bumpalo::boxed::Box as BumpBox; use bumpalo::Bump; use std::{ any::{Any, TypeId}, - cell::{Cell, RefCell, UnsafeCell}, + cell::{self, Cell, RefCell, UnsafeCell}, fmt::{Arguments, Debug}, future::Future, }; @@ -523,7 +523,7 @@ pub enum AttributeValue<'a> { Listener(RefCell>>), /// An arbitrary value that implements PartialEq and is static - Any(BumpBox<'a, dyn AnyValue>), + Any(RefCell>>), /// A "none" value, resulting in the removal of an attribute from the dom None, @@ -557,7 +557,7 @@ pub enum BorrowedAttributeValue<'a> { serialize_with = "serialize_any_value" ) )] - Any(&'a dyn AnyValue), + Any(std::cell::Ref<'a, dyn AnyValue>), /// A "none" value, resulting in the removal of an attribute from the dom None, @@ -573,7 +573,12 @@ impl<'a> From<&'a AttributeValue<'a>> for BorrowedAttributeValue<'a> { AttributeValue::Listener(_) => { panic!("A listener cannot be turned into a borrowed value") } - AttributeValue::Any(value) => BorrowedAttributeValue::Any(&**value), + AttributeValue::Any(value) => { + let value = value.borrow(); + BorrowedAttributeValue::Any(std::cell::Ref::map(value, |value| { + &**value.as_ref().unwrap() + })) + } AttributeValue::None => BorrowedAttributeValue::None, } } @@ -606,7 +611,7 @@ impl PartialEq for BorrowedAttributeValue<'_> { } #[cfg(feature = "serialize")] -fn serialize_any_value(_: &&dyn AnyValue, _: S) -> Result +fn serialize_any_value(_: &cell::Ref<'_, dyn AnyValue>, _: S) -> Result where S: serde::Serializer, { @@ -614,7 +619,7 @@ where } #[cfg(feature = "serialize")] -fn deserialize_any_value<'de, 'a, D>(_: D) -> Result<&'a dyn AnyValue, D::Error> +fn deserialize_any_value<'de, 'a, D>(_: D) -> Result, D::Error> where D: serde::Deserializer<'de>, { @@ -643,7 +648,11 @@ impl<'a> PartialEq for AttributeValue<'a> { (Self::Int(l0), Self::Int(r0)) => l0 == r0, (Self::Bool(l0), Self::Bool(r0)) => l0 == r0, (Self::Listener(_), Self::Listener(_)) => true, - (Self::Any(l0), Self::Any(r0)) => l0.any_cmp(&**r0), + (Self::Any(l0), Self::Any(r0)) => { + let l0 = l0.borrow(); + let r0 = r0.borrow(); + l0.as_ref().unwrap().any_cmp(&**r0.as_ref().unwrap()) + } _ => false, } } @@ -835,6 +844,12 @@ pub trait IntoAttributeValue<'a> { fn into_value(self, bump: &'a Bump) -> AttributeValue<'a>; } +impl<'a> IntoAttributeValue<'a> for AttributeValue<'a> { + fn into_value(self, _: &'a Bump) -> AttributeValue<'a> { + self + } +} + impl<'a> IntoAttributeValue<'a> for &'a str { fn into_value(self, _: &'a Bump) -> AttributeValue<'a> { AttributeValue::Text(self) @@ -870,6 +885,6 @@ impl<'a> IntoAttributeValue<'a> for Arguments<'_> { impl<'a> IntoAttributeValue<'a> for BumpBox<'a, dyn AnyValue> { fn into_value(self, _: &'a Bump) -> AttributeValue<'a> { - AttributeValue::Any(self) + AttributeValue::Any(RefCell::new(Some(self))) } } diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index d07b52704..03ce08d57 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -7,7 +7,7 @@ use crate::{ innerlude::{ErrorBoundary, Scheduler, SchedulerMsg}, lazynodes::LazyNodes, nodes::{ComponentReturn, IntoAttributeValue, IntoDynNode, RenderReturn}, - Attribute, AttributeValue, Element, Event, Properties, TaskId, + AnyValue, Attribute, AttributeValue, Element, Event, Properties, TaskId, }; use bumpalo::{boxed::Box as BumpBox, Bump}; use rustc_hash::{FxHashMap, FxHashSet}; @@ -513,6 +513,17 @@ impl<'src> ScopeState { AttributeValue::Listener(RefCell::new(Some(boxed))) } + /// Create a new [`AttributeValue`] with a value that implements [`AnyValue`] + pub fn any_value(&'src self, value: T) -> AttributeValue<'src> { + // safety: there's no other way to create a dynamicly-dispatched bump box other than alloc + from-raw + // This is the suggested way to build a bumpbox + // + // In theory, we could just use regular boxes + let boxed: BumpBox<'src, dyn AnyValue> = + unsafe { BumpBox::from_raw(self.bump().alloc(value)) }; + AttributeValue::Any(RefCell::new(Some(boxed))) + } + /// Inject an error into the nearest error boundary and quit rendering /// /// The error doesn't need to implement Error or any specific traits since the boundary diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index 043e7fafb..f8c625a20 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -641,7 +641,7 @@ impl VirtualDom { /// Swap the current mutations with a new fn finalize(&mut self) -> Mutations { - self.mutations.take() + std::mem::take(&mut self.mutations) } } diff --git a/packages/native-core/src/node.rs b/packages/native-core/src/node.rs index 3b91cee45..f5e93e2f5 100644 --- a/packages/native-core/src/node.rs +++ b/packages/native-core/src/node.rs @@ -111,7 +111,7 @@ impl From> for OwnedAttributeValue Self::Float(float), BorrowedAttributeValue::Int(int) => Self::Int(int), BorrowedAttributeValue::Bool(bool) => Self::Bool(bool), - BorrowedAttributeValue::Any(any) => Self::Custom(V::from_any_value(any)), + BorrowedAttributeValue::Any(any) => Self::Custom(V::from_any_value(&*any)), BorrowedAttributeValue::None => Self::None, } }