소스 검색

Fix hot reloading component properties out of order (#2842)

* Fix hot reloading component properties out of order

* ignore signal write warnings in hot reloading

* Fix hot reloading components with only some literal properties

* Add a test for hot reloading component properties
Evan Almloff 10 달 전
부모
커밋
4d6fb74e87

+ 1 - 0
Cargo.lock

@@ -2715,6 +2715,7 @@ dependencies = [
  "tokio-stream",
  "tokio-tungstenite 0.23.1",
  "tracing",
+ "warnings",
 ]
 
 [[package]]

+ 18 - 5
packages/core/src/hotreload_utils.rs

@@ -170,21 +170,34 @@ impl DynamicLiteralPool {
             *(Box::new(t) as Box<dyn Any>).downcast::<T2>().unwrap()
         }
         let grab_float = || {
-            self.get_component_property(id, hot_reload, HotReloadLiteral::as_float).expect("Expected a float component property. This is probably caused by a bug in dioxus hot reloading. Please report this issue.")
+            self.get_component_property(id, hot_reload, HotReloadLiteral::as_float).unwrap_or_else(|| {
+                tracing::error!("Expected a float component property, because the type was {}. The CLI gave the hot reloading engine a type of {:?}. This is probably caused by a bug in dioxus hot reloading. Please report this issue.", std::any::type_name::<T>(), hot_reload.component_values.get(id));
+                Default::default()
+
+        })
         };
         let grab_int = || {
-            self.get_component_property(id, hot_reload, HotReloadLiteral::as_int).expect("Expected an int component property. This is probably caused by a bug in dioxus hot reloading. Please report this issue.")
+            self.get_component_property(id, hot_reload, HotReloadLiteral::as_int).unwrap_or_else(|| {
+                tracing::error!("Expected a integer component property, because the type was {}. The CLI gave the hot reloading engine a type of {:?}. This is probably caused by a bug in dioxus hot reloading. Please report this issue.", std::any::type_name::<T>(), hot_reload.component_values.get(id));
+                Default::default()
+            })
         };
         let grab_bool = || {
-            self.get_component_property(id, hot_reload, HotReloadLiteral::as_bool).expect("Expected a bool component property. This is probably caused by a bug in dioxus hot reloading. Please report this issue.")
+            self.get_component_property(id, hot_reload, HotReloadLiteral::as_bool).unwrap_or_else(|| {
+                tracing::error!("Expected a bool component property, because the type was {}. The CLI gave the hot reloading engine a type of {:?}. This is probably caused by a bug in dioxus hot reloading. Please report this issue.", std::any::type_name::<T>(), hot_reload.component_values.get(id));
+                Default::default()
+            })
         };
         let grab_fmted = || {
-            self.get_component_property(id, hot_reload, |fmted| HotReloadLiteral::as_fmted(fmted).map(|segments| self.render_formatted(segments))).expect("Expected a string component property. This is probably caused by a bug in dioxus hot reloading. Please report this issue.")
+            self.get_component_property(id, hot_reload, |fmted| HotReloadLiteral::as_fmted(fmted).map(|segments| self.render_formatted(segments))).unwrap_or_else(|| {
+                tracing::error!("Expected a string component property, because the type was {}. The CLI gave the hot reloading engine a type of {:?}. This is probably caused by a bug in dioxus hot reloading. Please report this issue.", std::any::type_name::<T>(), hot_reload.component_values.get(id));
+                Default::default()
+            })
         };
         match TypeId::of::<T>() {
             // Any string types that accept a literal
             _ if TypeId::of::<String>() == TypeId::of::<T>() => assert_type(grab_fmted()),
-            _ if TypeId::of::<&'static str>() == TypeId::of::<T>() => {
+            _ if TypeId::of::<&str>() == TypeId::of::<T>() => {
                 assert_type(Box::leak(grab_fmted().into_boxed_str()) as &'static str)
             }
             // Any integer types that accept a literal

+ 1 - 0
packages/hot-reload/Cargo.toml

@@ -29,6 +29,7 @@ tokio-stream = { version = "0.1.12", features = ["sync"], optional = true }
 futures-util = { workspace = true, features = ["async-await-macro"], optional = true }
 tokio = { workspace = true, features = ["sync", "rt-multi-thread"], optional = true }
 tracing = { workspace = true }
+warnings.workspace = true
 
 # use rustls on android
 [target.'cfg(target_os = "android")'.dependencies]

+ 6 - 1
packages/hot-reload/src/client.rs

@@ -1,6 +1,7 @@
 use crate::HotReloadMsg;
 use dioxus_core::{ScopeId, VirtualDom};
 use dioxus_signals::Writable;
+use warnings::Warning;
 
 /// Applies template and literal changes to the VirtualDom
 ///
@@ -13,7 +14,11 @@ pub fn apply_changes(dom: &mut VirtualDom, msg: &HotReloadMsg) {
             let id = &template.location;
             let value = template.template.clone();
             if let Some(mut signal) = ctx.get_signal_with_key(id) {
-                signal.set(Some(value));
+                dioxus_signals::warnings::signal_read_and_write_in_reactive_scope::allow(|| {
+                    dioxus_signals::warnings::signal_write_in_component_body::allow(|| {
+                        signal.set(Some(value));
+                    });
+                });
             }
         }
     });

+ 8 - 7
packages/rsx/src/hot_reload/diff.rs

@@ -411,13 +411,14 @@ impl HotReloadResult {
         }
 
         let mut new_fields = new_component.fields.clone();
-        new_fields.sort_by(|a, b| a.name.to_string().cmp(&b.name.to_string()));
-        let mut old_fields = old_component.fields.clone();
-        old_fields.sort_by(|a, b| a.name.to_string().cmp(&b.name.to_string()));
+        new_fields.sort_by_key(|attribute| attribute.name.to_string());
+        let mut old_fields = old_component.fields.iter().enumerate().collect::<Vec<_>>();
+        old_fields.sort_by_key(|(_, attribute)| attribute.name.to_string());
 
-        let mut literal_component_properties = Vec::new();
+        // The literal component properties for the component in same the order as the original component property literals
+        let mut literal_component_properties = vec![None; old_fields.len()];
 
-        for (new_field, old_field) in new_fields.iter().zip(old_fields.iter()) {
+        for (new_field, (index, old_field)) in new_fields.iter().zip(old_fields.iter()) {
             // Verify the names match
             if new_field.name != old_field.name {
                 return None;
@@ -435,7 +436,7 @@ impl HotReloadResult {
                         return None;
                     }
                     let literal = self.full_rebuild_state.hotreload_hot_literal(new_value)?;
-                    literal_component_properties.push(literal);
+                    literal_component_properties[*index] = Some(literal);
                 }
                 _ => {
                     if new_field.value != old_field.value {
@@ -445,7 +446,7 @@ impl HotReloadResult {
             }
         }
 
-        Some(literal_component_properties)
+        Some(literal_component_properties.into_iter().flatten().collect())
     }
 
     /// Hot reload an if chain

+ 14 - 2
packages/rsx/tests/hotreload_pattern.rs

@@ -1069,8 +1069,20 @@ fn component_with_handlers() {
         }
     };
 
-    let valid = can_hotreload(a, b);
-    assert!(valid);
+    let hot_reload = hot_reload_from_tokens(a, b).unwrap();
+    let template = hot_reload.get(&0).unwrap();
+    assert_eq!(
+        template.component_values,
+        &[
+            HotReloadLiteral::Int(456),
+            HotReloadLiteral::Float(789.456),
+            HotReloadLiteral::Bool(false),
+            HotReloadLiteral::Fmted(FmtedSegments::new(vec![
+                FmtSegment::Literal { value: "goodbye " },
+                FmtSegment::Dynamic { id: 0 }
+            ])),
+        ]
+    );
 }
 
 #[test]