Browse Source

feat: improve safety

Jonathan Kelley 3 years ago
parent
commit
fda2ebc2a2

+ 5 - 5
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. 
 **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
 // An example Dioxus app - closely resembles React
-static App: FC<()> = |(cx, props)| {
+fn App(cx: Component<()>) -> Element {
     let mut count = use_state(cx, || 0);
     let mut count = use_state(cx, || 0);
 
 
     cx.render(rsx!(
     cx.render(rsx!(
@@ -94,8 +94,8 @@ The internal architecture of Dioxus was designed from day one to support the `Li
 
 
 ### Multithreaded Support
 ### 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.
 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.

+ 2 - 2
docs/src/SUMMARY.md

@@ -4,8 +4,8 @@
 - [Getting Setup](setup.md)
 - [Getting Setup](setup.md)
 - [Hello, World!](hello_world.md)
 - [Hello, World!](hello_world.md)
 - [Core Topics](concepts/00-index.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)
     - [RSX in Depth](concepts/rsx_in_depth.md)
     - [Components and Props](concepts/components.md)
     - [Components and Props](concepts/components.md)
     - [Memoization](concepts/memoization.md)
     - [Memoization](concepts/memoization.md)

+ 7 - 4
packages/core/examples/jsframework.rs

@@ -1,3 +1,5 @@
+#![allow(non_snake_case)]
+
 use dioxus::component::Component;
 use dioxus::component::Component;
 use dioxus::events::on::MouseEvent;
 use dioxus::events::on::MouseEvent;
 use dioxus_core as dioxus;
 use dioxus_core as dioxus;
@@ -13,7 +15,7 @@ fn main() {
     assert!(g.edits.len() > 1);
     assert!(g.edits.len() > 1);
 }
 }
 
 
-static App: FC<()> = |(cx, props)| {
+fn App((cx, props): Component<()>) -> DomTree {
     let mut rng = SmallRng::from_entropy();
     let mut rng = SmallRng::from_entropy();
     let rows = (0..10_000_usize).map(|f| {
     let rows = (0..10_000_usize).map(|f| {
         let label = Label::new(&mut rng);
         let label = Label::new(&mut rng);
@@ -31,13 +33,14 @@ static App: FC<()> = |(cx, props)| {
             }
             }
         }
         }
     })
     })
-};
+}
 
 
 #[derive(PartialEq, Props)]
 #[derive(PartialEq, Props)]
 struct RowProps {
 struct RowProps {
     row_id: usize,
     row_id: usize,
     label: Label,
     label: Label,
 }
 }
+
 fn Row((cx, props): Component<RowProps>) -> DomTree {
 fn Row((cx, props): Component<RowProps>) -> DomTree {
     let handler = move |evt: MouseEvent| {
     let handler = move |evt: MouseEvent| {
         let g = evt.button;
         let g = evt.button;
@@ -45,11 +48,11 @@ fn Row((cx, props): Component<RowProps>) -> DomTree {
     cx.render(rsx! {
     cx.render(rsx! {
         tr {
         tr {
             td { class:"col-md-1", "{props.row_id}" }
             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}" }
                 a { class: "lbl", "{props.label}" }
             }
             }
             td { class: "col-md-1"
             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" }
                     span { class: "glyphicon glyphicon-remove remove" aria_hidden: "true" }
                 }
                 }
             }
             }

+ 39 - 19
packages/core/src/diff.rs

@@ -221,12 +221,12 @@ impl<'bump> DiffMachine<'bump> {
             MountType::Replace { old } => {
             MountType::Replace { old } => {
                 if let Some(old_id) = old.try_mounted_id() {
                 if let Some(old_id) = old.try_mounted_id() {
                     self.mutations.replace_with(old_id, nodes_created as u32);
                     self.mutations.replace_with(old_id, nodes_created as u32);
-                    self.remove_nodes(Some(old));
+                    self.remove_nodes(Some(old), true);
                 } else {
                 } else {
                     if let Some(id) = self.find_first_element_id(old) {
                     if let Some(id) = self.find_first_element_id(old) {
                         self.mutations.replace_with(id, nodes_created as u32);
                         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();
         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
         // Run the scope for one iteration to initialize it
         if new_component.run_scope(self.vdom) {
         if new_component.run_scope(self.vdom) {
@@ -614,7 +618,7 @@ impl<'bump> DiffMachine<'bump> {
                 self.stack.create_children(new, MountType::Append);
                 self.stack.create_children(new, MountType::Append);
             }
             }
             (_, []) => {
             (_, []) => {
-                self.remove_nodes(old);
+                self.remove_nodes(old, true);
             }
             }
             ([VNode::Anchor(old_anchor)], [VNode::Anchor(new_anchor)]) => {
             ([VNode::Anchor(old_anchor)], [VNode::Anchor(new_anchor)]) => {
                 old_anchor.dom_id.set(new_anchor.dom_id.get());
                 old_anchor.dom_id.set(new_anchor.dom_id.get());
@@ -667,7 +671,7 @@ impl<'bump> DiffMachine<'bump> {
 
 
         use std::cmp::Ordering;
         use std::cmp::Ordering;
         match old.len().cmp(&new.len()) {
         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 => {
             Ordering::Less => {
                 self.stack.create_children(
                 self.stack.create_children(
                     &new[old.len()..],
                     &new[old.len()..],
@@ -750,7 +754,7 @@ impl<'bump> DiffMachine<'bump> {
         );
         );
         if new_middle.is_empty() {
         if new_middle.is_empty() {
             // remove the old elements
             // remove the old elements
-            self.remove_nodes(old_middle);
+            self.remove_nodes(old_middle, true);
         } else if old_middle.is_empty() {
         } else if old_middle.is_empty() {
             // there were no old elements, so just create the new elements
             // 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
             // 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
         // And if that was all of the new children, then remove all of the remaining
         // old children and we're finished.
         // old children and we're finished.
         if left_offset == new.len() {
         if left_offset == new.len() {
-            self.remove_nodes(&old[left_offset..]);
+            self.remove_nodes(&old[left_offset..], true);
             return None;
             return None;
         }
         }
 
 
@@ -1070,7 +1074,7 @@ impl<'bump> DiffMachine<'bump> {
         new: &'bump VNode<'bump>,
         new: &'bump VNode<'bump>,
     ) {
     ) {
         if let Some(first_old) = old.get(0) {
         if let Some(first_old) = old.get(0) {
-            self.remove_nodes(&old[1..]);
+            self.remove_nodes(&old[1..], true);
             self.stack
             self.stack
                 .create_node(new, MountType::Replace { old: first_old });
                 .create_node(new, MountType::Replace { old: first_old });
         } else {
         } else {
@@ -1080,42 +1084,58 @@ impl<'bump> DiffMachine<'bump> {
 
 
     /// schedules nodes for garbage collection and pushes "remove" to the mutation stack
     /// schedules nodes for garbage collection and pushes "remove" to the mutation stack
     /// remove can happen whenever
     /// remove can happen whenever
-    fn remove_nodes(&mut self, nodes: impl IntoIterator<Item = &'bump VNode<'bump>>) {
+    fn remove_nodes(
+        &mut self,
+        nodes: impl IntoIterator<Item = &'bump VNode<'bump>>,
+        gen_muts: bool,
+    ) {
         // or cache the vec on the diff machine
         // or cache the vec on the diff machine
         for node in nodes {
         for node in nodes {
             match node {
             match node {
                 VNode::Text(t) => {
                 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.mutations.remove(id.as_u64());
-                        self.vdom.collect_garbage(id);
                     }
                     }
                 }
                 }
                 VNode::Suspended(s) => {
                 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.mutations.remove(id.as_u64());
-                        self.vdom.collect_garbage(id);
                     }
                     }
                 }
                 }
                 VNode::Anchor(a) => {
                 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.mutations.remove(id.as_u64());
-                        self.vdom.collect_garbage(id);
                     }
                     }
                 }
                 }
                 VNode::Element(e) => {
                 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.mutations.remove(id.as_u64());
                     }
                     }
+
+                    self.remove_nodes(e.children, false);
                 }
                 }
                 VNode::Fragment(f) => {
                 VNode::Fragment(f) => {
-                    self.remove_nodes(f.children);
+                    self.remove_nodes(f.children, gen_muts);
                 }
                 }
 
 
                 VNode::Component(c) => {
                 VNode::Component(c) => {
                     let scope_id = c.associated_scope.get().unwrap();
                     let scope_id = c.associated_scope.get().unwrap();
                     let scope = self.vdom.get_scope_mut(scope_id).unwrap();
                     let scope = self.vdom.get_scope_mut(scope_id).unwrap();
                     let root = scope.root_node();
                     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();
                     let mut s = self.vdom.try_remove(scope_id).unwrap();
                     s.hooks.clear_hooks();
                     s.hooks.clear_hooks();
                 }
                 }
@@ -1132,7 +1152,7 @@ impl<'bump> DiffMachine<'bump> {
         new: &'bump [VNode<'bump>],
         new: &'bump [VNode<'bump>],
     ) {
     ) {
         if let Some(first_old) = old.get(0) {
         if let Some(first_old) = old.get(0) {
-            self.remove_nodes(&old[1..]);
+            self.remove_nodes(&old[1..], true);
             self.stack
             self.stack
                 .create_children(new, MountType::Replace { old: first_old })
                 .create_children(new, MountType::Replace { old: first_old })
         } else {
         } else {

+ 2 - 0
packages/core/src/events.rs

@@ -137,6 +137,7 @@ pub mod on {
     }
     }
 
 
     // The Dioxus Synthetic event system
     // 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! {
     event_directory! {
         ClipboardEvent: [
         ClipboardEvent: [
             /// Called when "copy"
             /// Called when "copy"
@@ -1031,6 +1032,7 @@ pub enum KeyCode {
     // kanji, = 244
     // kanji, = 244
     // unlock trackpad (Chrome/Edge), = 251
     // unlock trackpad (Chrome/Edge), = 251
     // toggle touchpad, = 255
     // toggle touchpad, = 255
+    #[cfg_attr(feature = "serialize", serde(other))]
     Unknown,
     Unknown,
 }
 }
 
 

+ 1 - 0
packages/core/src/hooklist.rs

@@ -57,6 +57,7 @@ impl HookList {
     }
     }
 
 
     pub fn clear_hooks(&mut self) {
     pub fn clear_hooks(&mut self) {
+        log::debug!("clearing hooks...");
         self.vals
         self.vals
             .borrow_mut()
             .borrow_mut()
             .drain(..)
             .drain(..)

+ 4 - 0
packages/core/src/nodes.rs

@@ -487,6 +487,8 @@ impl<'a> NodeFactory<'a> {
                         props.memoize(real_other)
                         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
                     // 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
                     // Implementing memoize is unsafe and done automatically with the props trait
                     matches!((props_memoized, children.is_empty()), (true, true))
                     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(|| {
             let drop_props: &mut dyn FnMut() = bump.alloc_with(|| {
                 move || unsafe {
                 move || unsafe {
+                    log::debug!("dropping props!");
                     if !has_dropped {
                     if !has_dropped {
                         let real_other = raw_props as *mut _ as *mut P;
                         let real_other = raw_props as *mut _ as *mut P;
                         let b = BumpBox::from_raw(real_other);
                         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> =
         let caller: &'a mut dyn for<'b> Fn(&'b Scope) -> DomTree<'b> =
             bump.alloc(move |scope: &Scope| -> DomTree {
             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 props: &'_ P = unsafe { &*(raw_props as *const P) };
                 let res = component((Context { scope }, props));
                 let res = component((Context { scope }, props));
                 unsafe { std::mem::transmute(res) }
                 unsafe { std::mem::transmute(res) }

+ 0 - 2
packages/core/src/scheduler.rs

@@ -407,8 +407,6 @@ impl Scheduler {
 
 
         let work_completed = machine.work(deadline_reached);
         let work_completed = machine.work(deadline_reached);
 
 
-        log::debug!("Working finished? {:?}", work_completed);
-
         // log::debug!("raw edits {:?}", machine.mutations.edits);
         // log::debug!("raw edits {:?}", machine.mutations.edits);
 
 
         let mut machine: DiffMachine<'static> = unsafe { std::mem::transmute(machine) };
         let mut machine: DiffMachine<'static> = unsafe { std::mem::transmute(machine) };

+ 6 - 0
packages/core/src/scope.rs

@@ -43,6 +43,8 @@ pub struct Scope {
 
 
     // State
     // State
     pub(crate) hooks: HookList,
     pub(crate) hooks: HookList,
+
+    // todo: move this into a centralized place - is more memory efficient
     pub(crate) shared_contexts: RefCell<HashMap<TypeId, Rc<dyn Any>>>,
     pub(crate) shared_contexts: RefCell<HashMap<TypeId, Rc<dyn Any>>>,
 
 
     // whenever set_state is called, we fire off a message to the scheduler
     // 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.suspended_nodes.borrow().is_empty());
         debug_assert!(self.borrowed_props.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
         // 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 };
         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.
         // Todo: see if we can add stronger guarantees around internal bookkeeping and failed component renders.
         if let Some(new_head) = render(self) {
         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
             // 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.wip_frame_mut().head_node = unsafe { std::mem::transmute(new_head) };
             self.frames.cycle_frame();
             self.frames.cycle_frame();

+ 9 - 2
packages/desktop/src/index.html

@@ -4,8 +4,15 @@
 <html>
 <html>
 
 
 <head>
 <head>
-    <!-- <meta name="viewport" content="width=device-width, initial-scale=1.0 maximum-scale=1.0 user-scalable=no"
-        charset="utf-8" /> -->
+    <!-- enable for mobile support -->
+    <meta name="viewport" content="width=device-width, initial-scale=1.0 maximum-scale=1.0 user-scalable=no"
+        charset="utf-8" />
+    <!-- <style>
+        body,
+        html {
+            position: fixed;
+        }
+    </style> -->
 </head>
 </head>
 
 
 
 

+ 37 - 1
packages/desktop/src/lib.rs

@@ -19,11 +19,15 @@ use serde::{Deserialize, Serialize};
 
 
 pub use wry;
 pub use wry;
 
 
+use wry::application::accelerator::{Accelerator, SysMods};
 use wry::application::event::{Event, StartCause, WindowEvent};
 use wry::application::event::{Event, StartCause, WindowEvent};
 use wry::application::event_loop::{self, ControlFlow, EventLoop};
 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::application::window::Fullscreen;
 use wry::webview::{WebView, WebViewBuilder};
 use wry::webview::{WebView, WebViewBuilder};
 use wry::{
 use wry::{
+    application::menu,
     application::window::{Window, WindowBuilder},
     application::window::{Window, WindowBuilder},
     webview::{RpcRequest, RpcResponse},
     webview::{RpcRequest, RpcResponse},
 };
 };
@@ -95,7 +99,39 @@ pub fn run<T: 'static + Send + Sync>(
 
 
         match event {
         match event {
             Event::NewEvents(StartCause::Init) => {
             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 window_id = window.id();
 
 
                 let (event_tx, event_rx) = tokio::sync::mpsc::unbounded_channel();
                 let (event_tx, event_rx) = tokio::sync::mpsc::unbounded_channel();

+ 1 - 1
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
     /// 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.cx.needs_update();
         self.notify_consumers();
         self.notify_consumers();
         self.value.borrow_mut()
         self.value.borrow_mut()