From e0bb67b476b6ab9f0adcb55be6aede85912e93ea Mon Sep 17 00:00:00 2001
From: Miles Murgaw <milesmurgaw@gmail.com>
Date: Thu, 1 Aug 2024 15:22:35 -0400
Subject: [PATCH] Support Optional Read Signals (#2761)

* progress: add test

* feat: optional read signals

* revision: doc

* fix: fmt

* revision: imports

* fix: return type only if option

* add a few more compile tests for optional props

---------

Co-authored-by: Evan Almloff <evanalmloff@gmail.com>
---
 packages/core-macro/src/props/mod.rs | 62 ++++++++++++++++++----------
 packages/core-macro/tests/rsx.rs     | 53 ++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 21 deletions(-)

diff --git a/packages/core-macro/src/props/mod.rs b/packages/core-macro/src/props/mod.rs
index 9e1f91fc8..68f911715 100644
--- a/packages/core-macro/src/props/mod.rs
+++ b/packages/core-macro/src/props/mod.rs
@@ -13,7 +13,7 @@ use syn::spanned::Spanned;
 use syn::{parse::Error, PathArguments};
 
 use quote::quote;
-use syn::{parse_quote, Type};
+use syn::{parse_quote, GenericArgument, PathSegment, Type};
 
 pub fn impl_my_derive(ast: &syn::DeriveInput) -> Result<TokenStream, Error> {
     let data = match &ast.data {
@@ -228,8 +228,7 @@ mod field_info {
 
                 // auto detect optional
                 let strip_option_auto = builder_attr.strip_option
-                    || !builder_attr.ignore_option
-                        && type_from_inside_option(&field.ty, true).is_some();
+                    || !builder_attr.ignore_option && type_from_inside_option(&field.ty).is_some();
                 if !builder_attr.strip_option && strip_option_auto {
                     builder_attr.strip_option = true;
                     builder_attr.default = Some(
@@ -482,31 +481,52 @@ mod field_info {
     }
 }
 
-fn type_from_inside_option(ty: &syn::Type, check_option_name: bool) -> Option<&syn::Type> {
-    let path = if let syn::Type::Path(type_path) = ty {
-        if type_path.qself.is_some() {
-            return None;
-        } else {
-            &type_path.path
-        }
-    } else {
+fn type_from_inside_option(ty: &Type) -> Option<&Type> {
+    let Type::Path(type_path) = ty else {
         return None;
     };
-    let segment = path.segments.last()?;
-    if check_option_name && segment.ident != "Option" {
+
+    if type_path.qself.is_some() {
         return None;
     }
-    let generic_params =
-        if let syn::PathArguments::AngleBracketed(generic_params) = &segment.arguments {
-            generic_params
-        } else {
+
+    let path = &type_path.path;
+    let seg = path.segments.last()?;
+
+    // If the segment is a supported optional type, provide the inner type.
+    if seg.ident == "Option" || seg.ident == "ReadOnlySignal" || seg.ident == "ReadSignal" {
+        // Get the inner type. E.g. the `u16` in `Option<u16>` or `Option` in `ReadSignal<Option<bool>>`
+        let inner_type = extract_inner_type_from_segment(seg)?;
+        let Type::Path(inner_path) = inner_type else {
             return None;
         };
-    if let syn::GenericArgument::Type(ty) = generic_params.args.first()? {
-        Some(ty)
-    } else {
-        None
+
+        // If we're entering an `Option`, we must get the innermost type. Otherwise, return the current type.
+        let inner_seg = inner_path.path.segments.last()?;
+        if inner_seg.ident == "Option" {
+            // Get the innermost type.
+            let innermost_type = extract_inner_type_from_segment(inner_seg)?;
+            return Some(innermost_type);
+        } else if seg.ident == "Option" {
+            // Return the inner type only if the parent is an `Option`.
+            return Some(inner_type);
+        }
     }
+
+    None
+}
+
+// Extract the inner type from a path segment.
+fn extract_inner_type_from_segment(segment: &PathSegment) -> Option<&Type> {
+    let PathArguments::AngleBracketed(generic_args) = &segment.arguments else {
+        return None;
+    };
+
+    let GenericArgument::Type(final_type) = generic_args.args.first()? else {
+        return None;
+    };
+
+    Some(final_type)
 }
 
 mod struct_info {
diff --git a/packages/core-macro/tests/rsx.rs b/packages/core-macro/tests/rsx.rs
index d11bb36b2..5f6eaadd8 100644
--- a/packages/core-macro/tests/rsx.rs
+++ b/packages/core-macro/tests/rsx.rs
@@ -51,3 +51,56 @@ mod test_default_into {
         pub some_other_data: bool,
     }
 }
+/// This test ensures that read-only signals that contain an option (`Signal<Option<u16>>`)
+/// are correctly created as default when not provided.
+///
+/// These are compile-time tests.
+/// See https://github.com/DioxusLabs/dioxus/issues/2648
+#[cfg(test)]
+#[allow(unused)]
+mod test_optional_signals {
+    use dioxus::prelude::*;
+
+    // Test if test components fail to compile.
+    #[component]
+    fn UsesComponents() -> Element {
+        rsx! {
+            PropsStruct {
+                regular_read_signal: ReadOnlySignal::new(Signal::new(1234)),
+            }
+            PropsStruct {
+                optional_read_signal: 1234,
+                regular_read_signal: 123u16,
+            }
+            PropParams {}
+            PropParams {
+                opt_read_sig: 1234
+            }
+            DoubleOption {}
+            DoubleOption { optional: Some(1234) }
+        }
+    }
+
+    // Test props as struct param.
+    #[derive(Props, Clone, PartialEq)]
+    struct MyTestProps {
+        pub optional_read_signal: ReadOnlySignal<Option<u16>>,
+        pub regular_read_signal: ReadOnlySignal<u16>,
+    }
+
+    #[component]
+    fn PropsStruct(props: MyTestProps) -> Element {
+        rsx! { "hi" }
+    }
+
+    // Test props as params.
+    #[component]
+    fn PropParams(opt_read_sig: ReadOnlySignal<Option<u16>>) -> Element {
+        rsx! { "hi!" }
+    }
+
+    #[component]
+    fn DoubleOption(optional: Option<Option<u16>>) -> Element {
+        rsx! { "hi!" }
+    }
+}