Fix event handler drops (#2288)

* Try to fix event handler drops

* fix Option<EventHandler> parsing logic

* only move over props if both event handlers are some

* fix owner forwarding with props(extends)

* fix clippy

---------

Co-authored-by: Evan Almloff <evanalmloff@gmail.com>

packages/core-macro/src/props/mod.rs

@@ -516,6 +516,8 @@ mod struct_info {
     use syn::spanned::Spanned;
     use syn::{Expr, Ident};
 
+    use crate::props::strip_option;
+
     use super::field_info::{FieldBuilderAttr, FieldInfo};
     use super::util::{
         empty_type, empty_type_tuple, expr_to_single_string, make_punctuated_single,
@@ -635,10 +637,6 @@ mod struct_info {
             let event_handlers_fields: Vec<_> = self
                 .included_fields()
                 .filter(|f| looks_like_event_handler_type(f.ty))
-                .map(|f| {
-                    let name = f.name;
-                    quote!(#name)
-                })
                 .collect();
 
             let regular_fields: Vec<_> = self
@@ -650,12 +648,24 @@ mod struct_info {
                 })
                 .collect();
 
-            let move_event_handlers = quote! {
+            let move_event_handlers: TokenStream = event_handlers_fields.iter().map(|field| {
-                #(
+                // If this is an optional event handler, we need to check if it's None before we try to update it
-                    // Update the event handlers
+                let optional = strip_option(field.ty).is_some();
-                    self.#event_handlers_fields.__set(new.#event_handlers_fields.__take());
+                let name = field.name;
-                )*
+                if optional {
-            };
+                    quote! {
+                        // If the event handler is None, we don't need to update it
+                        if let (Some(old_handler), Some(new_handler)) = (self.#name.as_mut(), new.#name.as_ref()) {
+                            old_handler.__set(new_handler.__take());
+                        }
+                    }
+                } else {
+                    quote! {
+                        // Update the event handlers
+                        self.#name.__set(new.#name.__take());
+                    }
+                }
+            }).collect();
 
             // If there are signals, we automatically try to memoize the signals
             if !signal_fields.is_empty() {
@@ -957,6 +967,11 @@ Finally, call `.build()` to create the instance of `{name}`.
                 quote!(#name: self.#name)
             });
 
+            let forward_owner = self
+                .has_child_owned_fields()
+                .then(|| quote!(owner: self.owner))
+                .into_iter();
+
             let extends_impl = field.builder_attr.extends.iter().map(|path| {
                 let name_str = path_to_single_string(path).unwrap();
                 let camel_name = name_str.to_case(Case::UpperCamel);
@@ -994,6 +1009,7 @@ Finally, call `.build()` to create the instance of `{name}`.
                         );
                         #builder_name {
                             #(#forward_extended_fields,)*
+                            #(#forward_owner,)*
                             fields: ( #(#reconstructing,)* ),
                             _phantom: self._phantom,
                         }
@@ -1622,6 +1638,41 @@ fn extract_base_type_without_single_generic(ty: &Type) -> Option<syn::Path> {
     Some(path_without_generics)
 }
 
+/// Returns the type inside the Option wrapper if it exists
+fn strip_option(type_: &Type) -> Option<Type> {
+    if let Type::Path(ty) = &type_ {
+        let mut segments_iter = ty.path.segments.iter().peekable();
+        // Strip any leading std||core::option:: prefix
+        let allowed_segments: &[&[&str]] = &[&["std", "core"], &["option"]];
+        let mut allowed_segments_iter = allowed_segments.iter();
+        while let Some(segment) = segments_iter.peek() {
+            let Some(allowed_segments) = allowed_segments_iter.next() else {
+                break;
+            };
+            if !allowed_segments.contains(&segment.ident.to_string().as_str()) {
+                break;
+            }
+            segments_iter.next();
+        }
+        // The last segment should be Option
+        let option_segment = segments_iter.next()?;
+        if option_segment.ident == "Option" && segments_iter.next().is_none() {
+            // It should have a single generic argument
+            if let PathArguments::AngleBracketed(generic_arg) = &option_segment.arguments {
+                if let Some(syn::GenericArgument::Type(ty)) = generic_arg.args.first() {
+                    return Some(ty.clone());
+                }
+            }
+        }
+    }
+    None
+}
+
+/// Remove the Option wrapper from a type
+fn remove_option_wrapper(type_: Type) -> Type {
+    strip_option(&type_).unwrap_or(type_)
+}
+
 /// Check if a type should be owned by the child component after conversion
 fn child_owned_type(ty: &Type) -> bool {
     looks_like_signal_type(ty) || looks_like_event_handler_type(ty)
@@ -1639,7 +1690,8 @@ fn looks_like_signal_type(ty: &Type) -> bool {
 }
 
 fn looks_like_event_handler_type(ty: &Type) -> bool {
-    match extract_base_type_without_single_generic(ty) {
+    let type_without_option = remove_option_wrapper(ty.clone());
+    match extract_base_type_without_single_generic(&type_without_option) {
         Some(path_without_generics) => {
             path_without_generics == parse_quote!(dioxus_core::prelude::EventHandler)
                 || path_without_generics == parse_quote!(prelude::EventHandler)
@@ -1648,3 +1700,44 @@ fn looks_like_event_handler_type(ty: &Type) -> bool {
         None => false,
     }
 }
+
+#[test]
+fn test_looks_like_type() {
+    assert!(!looks_like_signal_type(&parse_quote!(
+        Option<ReadOnlySignal<i32>>
+    )));
+    assert!(looks_like_signal_type(&parse_quote!(ReadOnlySignal<i32>)));
+    assert!(looks_like_signal_type(
+        &parse_quote!(ReadOnlySignal<i32, SyncStorage>)
+    ));
+    assert!(looks_like_signal_type(&parse_quote!(
+        ReadOnlySignal<Option<i32>, UnsyncStorage>
+    )));
+
+    assert!(looks_like_event_handler_type(&parse_quote!(
+        Option<EventHandler>
+    )));
+    assert!(looks_like_event_handler_type(&parse_quote!(
+        std::option::Option<EventHandler<i32>>
+    )));
+    assert!(looks_like_event_handler_type(&parse_quote!(
+        Option<EventHandler<MouseEvent>>
+    )));
+
+    assert!(looks_like_event_handler_type(&parse_quote!(
+        EventHandler<i32>
+    )));
+    assert!(looks_like_event_handler_type(&parse_quote!(EventHandler)));
+}
+
+#[test]
+fn test_remove_option_wrapper() {
+    let type_without_option = remove_option_wrapper(parse_quote!(Option<i32>));
+    assert_eq!(type_without_option, parse_quote!(i32));
+
+    let type_without_option = remove_option_wrapper(parse_quote!(Option<Option<i32>>));
+    assert_eq!(type_without_option, parse_quote!(Option<i32>));
+
+    let type_without_option = remove_option_wrapper(parse_quote!(Option<Option<Option<i32>>>));
+    assert_eq!(type_without_option, parse_quote!(Option<Option<i32>>));
+}