From fda2ebc2a22965845e015384f39f34ce7cb3e428 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Mon, 25 Oct 2021 15:05:17 -0400 Subject: [PATCH] feat: improve safety --- docs/src/README.md | 10 ++--- docs/src/SUMMARY.md | 4 +- packages/core/examples/jsframework.rs | 11 +++-- packages/core/src/diff.rs | 58 +++++++++++++++++--------- packages/core/src/events.rs | 2 + packages/core/src/hooklist.rs | 1 + packages/core/src/nodes.rs | 4 ++ packages/core/src/scheduler.rs | 2 - packages/core/src/scope.rs | 6 +++ packages/desktop/src/index.html | 11 ++++- packages/desktop/src/lib.rs | 38 ++++++++++++++++- packages/hooks/src/use_shared_state.rs | 2 +- 12 files changed, 113 insertions(+), 36 deletions(-) diff --git a/docs/src/README.md b/docs/src/README.md index 5c02265aa..da1f68a97 100644 --- a/docs/src/README.md +++ b/docs/src/README.md @@ -4,9 +4,9 @@ **Dioxus** is a framework and ecosystem for building fast, scalable, and robust user interfaces with the Rust programming language. This guide will help you get up-and-running with Dioxus running on the Web, Desktop, Mobile, and more. -```rust, ignore +```rust // An example Dioxus app - closely resembles React -static App: FC<()> = |(cx, props)| { +fn App(cx: Component<()>) -> Element { let mut count = use_state(cx, || 0); cx.render(rsx!( @@ -94,8 +94,8 @@ The internal architecture of Dioxus was designed from day one to support the `Li ### Multithreaded Support --- -The Dioxus VirtualDom, sadly, is not `Send.` This means you can't easily use Dioxus with most web frameworks like Tide, Rocket, Axum, etc. Currently, your two options include: -- Actix: Actix supports `!Send` handlers -- VirtualDomPool: A thread-per-core VirtualDom pool that uses message passing and serialization +The Dioxus VirtualDom, sadly, is not currently `Send.` Internally, we use quite a bit of interior mutability which is not thread-safe. This means you can't easily use Dioxus with most web frameworks like Tide, Rocket, Axum, etc. + +To solve this, you'll want to spawn a VirtualDom on its own thread and communicate with it via channels. When working with web frameworks that require `Send`, it is possible to render a VirtualDom immediately to a String - but you cannot hold the VirtualDom across an await point. For retained-state SSR (essentially LiveView), you'll need to create a VirtualDomPool which solves the `Send` problem. diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index 2718e5b2d..6c00723fe 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -4,8 +4,8 @@ - [Getting Setup](setup.md) - [Hello, World!](hello_world.md) - [Core Topics](concepts/00-index.md) - - [Intro to VNodes](concepts/vnodes.md) - - [VNodes with RSX, HTML, and NodeFactory](concepts/rsx.md) + - [Intro to Elements](concepts/vnodes.md) + - [Elements with RSX and NodeFactory](concepts/rsx.md) - [RSX in Depth](concepts/rsx_in_depth.md) - [Components and Props](concepts/components.md) - [Memoization](concepts/memoization.md) diff --git a/packages/core/examples/jsframework.rs b/packages/core/examples/jsframework.rs index c05bd06f6..3a9d73681 100644 --- a/packages/core/examples/jsframework.rs +++ b/packages/core/examples/jsframework.rs @@ -1,3 +1,5 @@ +#![allow(non_snake_case)] + use dioxus::component::Component; use dioxus::events::on::MouseEvent; use dioxus_core as dioxus; @@ -13,7 +15,7 @@ fn main() { assert!(g.edits.len() > 1); } -static App: FC<()> = |(cx, props)| { +fn App((cx, props): Component<()>) -> DomTree { let mut rng = SmallRng::from_entropy(); let rows = (0..10_000_usize).map(|f| { let label = Label::new(&mut rng); @@ -31,13 +33,14 @@ static App: FC<()> = |(cx, props)| { } } }) -}; +} #[derive(PartialEq, Props)] struct RowProps { row_id: usize, label: Label, } + fn Row((cx, props): Component) -> DomTree { let handler = move |evt: MouseEvent| { let g = evt.button; @@ -45,11 +48,11 @@ fn Row((cx, props): Component) -> DomTree { cx.render(rsx! { tr { td { class:"col-md-1", "{props.row_id}" } - td { class:"col-md-1", onclick: move |_| { /* run onselect */ } + td { class:"col-md-1", on_click: move |_| { /* run onselect */ } a { class: "lbl", "{props.label}" } } td { class: "col-md-1" - a { class: "remove", onclick: {handler} + a { class: "remove", on_click: {handler} span { class: "glyphicon glyphicon-remove remove" aria_hidden: "true" } } } diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index d73b0b128..25e2d71e7 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -221,12 +221,12 @@ impl<'bump> DiffMachine<'bump> { MountType::Replace { old } => { if let Some(old_id) = old.try_mounted_id() { self.mutations.replace_with(old_id, nodes_created as u32); - self.remove_nodes(Some(old)); + self.remove_nodes(Some(old), true); } else { if let Some(id) = self.find_first_element_id(old) { self.mutations.replace_with(id, nodes_created as u32); } - self.remove_nodes(Some(old)); + self.remove_nodes(Some(old), true); } } @@ -369,7 +369,11 @@ impl<'bump> DiffMachine<'bump> { let new_component = self.vdom.get_scope_mut(new_idx).unwrap(); - log::debug!("initializing component {:?}", new_idx); + log::debug!( + "initializing component {:?} with height {:?}", + new_idx, + parent_scope.height + 1 + ); // Run the scope for one iteration to initialize it if new_component.run_scope(self.vdom) { @@ -614,7 +618,7 @@ impl<'bump> DiffMachine<'bump> { self.stack.create_children(new, MountType::Append); } (_, []) => { - self.remove_nodes(old); + self.remove_nodes(old, true); } ([VNode::Anchor(old_anchor)], [VNode::Anchor(new_anchor)]) => { old_anchor.dom_id.set(new_anchor.dom_id.get()); @@ -667,7 +671,7 @@ impl<'bump> DiffMachine<'bump> { use std::cmp::Ordering; match old.len().cmp(&new.len()) { - Ordering::Greater => self.remove_nodes(&old[new.len()..]), + Ordering::Greater => self.remove_nodes(&old[new.len()..], true), Ordering::Less => { self.stack.create_children( &new[old.len()..], @@ -750,7 +754,7 @@ impl<'bump> DiffMachine<'bump> { ); if new_middle.is_empty() { // remove the old elements - self.remove_nodes(old_middle); + self.remove_nodes(old_middle, true); } else if old_middle.is_empty() { // there were no old elements, so just create the new elements // we need to find the right "foothold" though - we shouldnt use the "append" at all @@ -825,7 +829,7 @@ impl<'bump> DiffMachine<'bump> { // And if that was all of the new children, then remove all of the remaining // old children and we're finished. if left_offset == new.len() { - self.remove_nodes(&old[left_offset..]); + self.remove_nodes(&old[left_offset..], true); return None; } @@ -1070,7 +1074,7 @@ impl<'bump> DiffMachine<'bump> { new: &'bump VNode<'bump>, ) { if let Some(first_old) = old.get(0) { - self.remove_nodes(&old[1..]); + self.remove_nodes(&old[1..], true); self.stack .create_node(new, MountType::Replace { old: first_old }); } else { @@ -1080,42 +1084,58 @@ impl<'bump> DiffMachine<'bump> { /// schedules nodes for garbage collection and pushes "remove" to the mutation stack /// remove can happen whenever - fn remove_nodes(&mut self, nodes: impl IntoIterator>) { + fn remove_nodes( + &mut self, + nodes: impl IntoIterator>, + gen_muts: bool, + ) { // or cache the vec on the diff machine for node in nodes { match node { VNode::Text(t) => { - if let Some(id) = t.dom_id.get() { + let id = t.dom_id.get().unwrap(); + self.vdom.collect_garbage(id); + + if gen_muts { self.mutations.remove(id.as_u64()); - self.vdom.collect_garbage(id); } } VNode::Suspended(s) => { - if let Some(id) = s.dom_id.get() { + let id = s.dom_id.get().unwrap(); + self.vdom.collect_garbage(id); + + if gen_muts { self.mutations.remove(id.as_u64()); - self.vdom.collect_garbage(id); } } VNode::Anchor(a) => { - if let Some(id) = a.dom_id.get() { + let id = a.dom_id.get().unwrap(); + self.vdom.collect_garbage(id); + + if gen_muts { self.mutations.remove(id.as_u64()); - self.vdom.collect_garbage(id); } } VNode::Element(e) => { - if let Some(id) = e.dom_id.get() { + let id = e.dom_id.get().unwrap(); + + if gen_muts { self.mutations.remove(id.as_u64()); } + + self.remove_nodes(e.children, false); } VNode::Fragment(f) => { - self.remove_nodes(f.children); + self.remove_nodes(f.children, gen_muts); } VNode::Component(c) => { let scope_id = c.associated_scope.get().unwrap(); let scope = self.vdom.get_scope_mut(scope_id).unwrap(); let root = scope.root_node(); - self.remove_nodes(Some(root)); + self.remove_nodes(Some(root), gen_muts); + + log::debug!("Destroying scope {:?}", scope_id); let mut s = self.vdom.try_remove(scope_id).unwrap(); s.hooks.clear_hooks(); } @@ -1132,7 +1152,7 @@ impl<'bump> DiffMachine<'bump> { new: &'bump [VNode<'bump>], ) { if let Some(first_old) = old.get(0) { - self.remove_nodes(&old[1..]); + self.remove_nodes(&old[1..], true); self.stack .create_children(new, MountType::Replace { old: first_old }) } else { diff --git a/packages/core/src/events.rs b/packages/core/src/events.rs index 728b71e78..dd87e343b 100644 --- a/packages/core/src/events.rs +++ b/packages/core/src/events.rs @@ -137,6 +137,7 @@ pub mod on { } // The Dioxus Synthetic event system + // todo: move these into the html event system. dioxus accepts *any* event, so having these here doesn't make sense. event_directory! { ClipboardEvent: [ /// Called when "copy" @@ -1031,6 +1032,7 @@ pub enum KeyCode { // kanji, = 244 // unlock trackpad (Chrome/Edge), = 251 // toggle touchpad, = 255 + #[cfg_attr(feature = "serialize", serde(other))] Unknown, } diff --git a/packages/core/src/hooklist.rs b/packages/core/src/hooklist.rs index 45b07d9cc..5e80bf11b 100644 --- a/packages/core/src/hooklist.rs +++ b/packages/core/src/hooklist.rs @@ -57,6 +57,7 @@ impl HookList { } pub fn clear_hooks(&mut self) { + log::debug!("clearing hooks..."); self.vals .borrow_mut() .drain(..) diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 917b7aca3..1a21f21d8 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -487,6 +487,8 @@ impl<'a> NodeFactory<'a> { props.memoize(real_other) }; + log::debug!("comparing props..."); + // It's only okay to memoize if there are no children and the props can be memoized // Implementing memoize is unsafe and done automatically with the props trait matches!((props_memoized, children.is_empty()), (true, true)) @@ -502,6 +504,7 @@ impl<'a> NodeFactory<'a> { let drop_props: &mut dyn FnMut() = bump.alloc_with(|| { move || unsafe { + log::debug!("dropping props!"); if !has_dropped { let real_other = raw_props as *mut _ as *mut P; let b = BumpBox::from_raw(real_other); @@ -525,6 +528,7 @@ impl<'a> NodeFactory<'a> { let caller: &'a mut dyn for<'b> Fn(&'b Scope) -> DomTree<'b> = bump.alloc(move |scope: &Scope| -> DomTree { + log::debug!("calling component renderr {:?}", scope.our_arena_idx); let props: &'_ P = unsafe { &*(raw_props as *const P) }; let res = component((Context { scope }, props)); unsafe { std::mem::transmute(res) } diff --git a/packages/core/src/scheduler.rs b/packages/core/src/scheduler.rs index 20005a33e..fab67e211 100644 --- a/packages/core/src/scheduler.rs +++ b/packages/core/src/scheduler.rs @@ -407,8 +407,6 @@ impl Scheduler { let work_completed = machine.work(deadline_reached); - log::debug!("Working finished? {:?}", work_completed); - // log::debug!("raw edits {:?}", machine.mutations.edits); let mut machine: DiffMachine<'static> = unsafe { std::mem::transmute(machine) }; diff --git a/packages/core/src/scope.rs b/packages/core/src/scope.rs index 850bab8a2..df23feea1 100644 --- a/packages/core/src/scope.rs +++ b/packages/core/src/scope.rs @@ -43,6 +43,8 @@ pub struct Scope { // State pub(crate) hooks: HookList, + + // todo: move this into a centralized place - is more memory efficient pub(crate) shared_contexts: RefCell>>, // whenever set_state is called, we fire off a message to the scheduler @@ -345,11 +347,15 @@ impl Scope { debug_assert!(self.suspended_nodes.borrow().is_empty()); debug_assert!(self.borrowed_props.borrow().is_empty()); + log::debug!("Borrowed stuff is successfully cleared"); + // Cast the caller ptr from static to one with our own reference let render: &dyn for<'b> Fn(&'b Scope) -> DomTree<'b> = unsafe { &*self.caller }; // Todo: see if we can add stronger guarantees around internal bookkeeping and failed component renders. if let Some(new_head) = render(self) { + log::debug!("Render is successful"); + // the user's component succeeded. We can safely cycle to the next frame self.frames.wip_frame_mut().head_node = unsafe { std::mem::transmute(new_head) }; self.frames.cycle_frame(); diff --git a/packages/desktop/src/index.html b/packages/desktop/src/index.html index 868e7fa4a..ebddd60d5 100644 --- a/packages/desktop/src/index.html +++ b/packages/desktop/src/index.html @@ -4,8 +4,15 @@ - + + + diff --git a/packages/desktop/src/lib.rs b/packages/desktop/src/lib.rs index ce119e9bc..f64ac82b0 100644 --- a/packages/desktop/src/lib.rs +++ b/packages/desktop/src/lib.rs @@ -19,11 +19,15 @@ use serde::{Deserialize, Serialize}; pub use wry; +use wry::application::accelerator::{Accelerator, SysMods}; use wry::application::event::{Event, StartCause, WindowEvent}; use wry::application::event_loop::{self, ControlFlow, EventLoop}; +use wry::application::keyboard::KeyCode; +use wry::application::menu::{MenuBar, MenuItem, MenuItemAttributes}; use wry::application::window::Fullscreen; use wry::webview::{WebView, WebViewBuilder}; use wry::{ + application::menu, application::window::{Window, WindowBuilder}, webview::{RpcRequest, RpcResponse}, }; @@ -95,7 +99,39 @@ pub fn run( match event { Event::NewEvents(StartCause::Init) => { - let window = WindowBuilder::new().build(event_loop).unwrap(); + // create main menubar menu + let mut menu_bar_menu = MenuBar::new(); + + // create `first_menu` + let mut first_menu = MenuBar::new(); + + first_menu.add_native_item(MenuItem::About("Todos".to_string())); + first_menu.add_native_item(MenuItem::Services); + first_menu.add_native_item(MenuItem::Separator); + first_menu.add_native_item(MenuItem::Hide); + first_menu.add_native_item(MenuItem::HideOthers); + first_menu.add_native_item(MenuItem::ShowAll); + + let quit_item = first_menu.add_item( + MenuItemAttributes::new("Quit") + .with_accelerators(&Accelerator::new(SysMods::Cmd, KeyCode::KeyQ)), + ); + + // create second menu + let mut second_menu = MenuBar::new(); + + // second_menu.add_submenu("Sub menu", true, my_sub_menu); + second_menu.add_native_item(MenuItem::Copy); + second_menu.add_native_item(MenuItem::Paste); + second_menu.add_native_item(MenuItem::SelectAll); + + menu_bar_menu.add_submenu("First menu", true, first_menu); + menu_bar_menu.add_submenu("Second menu", true, second_menu); + + let window = WindowBuilder::new() + .with_menu(menu_bar_menu) + .build(event_loop) + .unwrap(); let window_id = window.id(); let (event_tx, event_rx) = tokio::sync::mpsc::unbounded_channel(); diff --git a/packages/hooks/src/use_shared_state.rs b/packages/hooks/src/use_shared_state.rs index b1febb498..593cf3922 100644 --- a/packages/hooks/src/use_shared_state.rs +++ b/packages/hooks/src/use_shared_state.rs @@ -126,7 +126,7 @@ impl<'a, T: 'static> UseSharedState<'a, T> { /// /// /// TODO: We prevent unncessary notifications only in the hook, but we should figure out some more global lock - pub fn write(&self) -> RefMut<'_, T> { + pub fn write(&mut self) -> RefMut<'_, T> { self.cx.needs_update(); self.notify_consumers(); self.value.borrow_mut()