Merge pull request #1066 from fabulous-dev/disposable-component

Dispose properly ViewNode and Component when Widget is removed from tree
fabulous-dev · Jan 30, 2024 · 0d5f4c2 · 0d5f4c2
2 parents 1933f98 + 2931784
commit 0d5f4c2
Show file tree

Hide file tree

Showing 14 changed files with 495 additions and 209 deletions.
diff --git a/src/Fabulous.Tests/APISketchTests/TestUI.Attributes.fs b/src/Fabulous.Tests/APISketchTests/TestUI.Attributes.fs
@@ -18,18 +18,22 @@ module TestUI_Attributes =
 
                         let btn = node.Target :?> IButton
 
-                        match node.TryGetHandler<int>(name) with
+                        match node.TryGetHandler(name) with
                         | ValueNone -> ()
-                        | ValueSome handlerId -> btn.RemovePressListener handlerId
+                        | ValueSome handler -> handler.Dispose()
 
                         match newValueOpt with
-                        | ValueNone -> node.SetHandler(name, ValueNone)
+                        | ValueNone -> node.RemoveHandler(name)
 
                         | ValueSome msg ->
                             let handler () = Dispatcher.dispatch node msg
-
                             let handlerId = btn.AddPressListener handler
-                            node.SetHandler<int>(name, ValueSome handlerId))
+
+                            let disposable =
+                                { new IDisposable with
+                                    member _.Dispose() = btn.RemovePressListener handlerId }
+
+                            node.SetHandler(name, disposable))
                 )
                 |> AttributeDefinitionStore.registerScalar
 
@@ -44,18 +48,22 @@ module TestUI_Attributes =
 
                         let btn = node.Target :?> IButton
 
-                        match node.TryGetHandler<int>(name) with
+                        match node.TryGetHandler(name) with
                         | ValueNone -> ()
-                        | ValueSome handlerId -> btn.RemoveTapListener handlerId
+                        | ValueSome handler -> handler.Dispose()
 
                         match newValueOpt with
-                        | ValueNone -> node.SetHandler(name, ValueNone)
+                        | ValueNone -> node.RemoveHandler(name)
 
                         | ValueSome msg ->
                             let handler () = Dispatcher.dispatch node msg
-
                             let handlerId = btn.AddTapListener handler
-                            node.SetHandler<int>(name, ValueSome handlerId))
+
+                            let disposable =
+                                { new IDisposable with
+                                    member _.Dispose() = btn.RemoveTapListener handlerId }
+
+                            node.SetHandler(name, disposable))
                 )
                 |> AttributeDefinitionStore.registerScalar
 
@@ -70,18 +78,22 @@ module TestUI_Attributes =
 
                         let btn = node.Target :?> IButton
 
-                        match node.TryGetHandler<int>(name) with
+                        match node.TryGetHandler(name) with
                         | ValueNone -> ()
-                        | ValueSome handlerId -> btn.RemoveTap2Listener handlerId
+                        | ValueSome handler -> handler.Dispose()
 
                         match newValueOpt with
-                        | ValueNone -> node.SetHandler(name, ValueNone)
+                        | ValueNone -> node.RemoveHandler(name)
 
                         | ValueSome msg ->
                             let handler () = Dispatcher.dispatch node msg
-
                             let handlerId = btn.AddTap2Listener handler
-                            node.SetHandler<int>(name, ValueSome handlerId))
+
+                            let disposable =
+                                { new IDisposable with
+                                    member _.Dispose() = btn.RemoveTap2Listener handlerId }
+
+                            node.SetHandler(name, disposable))
                 )
                 |> AttributeDefinitionStore.registerScalar
 
@@ -96,18 +108,22 @@ module TestUI_Attributes =
 
                         let btn = node.Target :?> IContainer
 
-                        match node.TryGetHandler<int>(name) with
+                        match node.TryGetHandler(name) with
                         | ValueNone -> ()
-                        | ValueSome handlerId -> btn.RemoveTapListener handlerId
+                        | ValueSome handler -> handler.Dispose()
 
                         match newValueOpt with
-                        | ValueNone -> node.SetHandler(name, ValueNone)
+                        | ValueNone -> node.RemoveHandler(name)
 
                         | ValueSome msg ->
                             let handler () = Dispatcher.dispatch node msg
-
                             let handlerId = btn.AddTapListener handler
-                            node.SetHandler<int>(name, ValueSome handlerId))
+
+                            let disposable =
+                                { new IDisposable with
+                                    member _.Dispose() = btn.RemoveTapListener handlerId }
+
+                            node.SetHandler(name, disposable))
                 )
                 |> AttributeDefinitionStore.registerScalar
 

diff --git a/src/Fabulous.Tests/APISketchTests/TestUI.Component.fs b/src/Fabulous.Tests/APISketchTests/TestUI.Component.fs
@@ -9,4 +9,9 @@ module TestUI_Component =
         let ComponentProperty = "ComponentProperty"
 
         let getComponent (target: obj) =
-            (target :?> TestViewElement).PropertyBag.Item ComponentProperty
+            match (target :?> TestViewElement).PropertyBag.TryGetValue(ComponentProperty) with
+            | true, comp -> comp
+            | _ -> null
+
+        let setComponent (comp: obj) (target: obj) =
+            (target :?> TestViewElement).PropertyBag.Add(ComponentProperty, comp)
diff --git a/src/Fabulous.Tests/APISketchTests/TestUI.Widgets.fs b/src/Fabulous.Tests/APISketchTests/TestUI.Widgets.fs
@@ -41,7 +41,7 @@ module TestUI_Widgets =
                             | ValueNone -> None
                             | ValueSome parent -> Some parent
 
-                        let viewNode = ViewNode(parentNode, context, weakReference)
+                        let viewNode = new ViewNode(parentNode, context, weakReference)
 
                         view.PropertyBag.Add(ViewNode.ViewNodeProperty, viewNode)
 
@@ -214,6 +214,7 @@ module TestUI_Widgets =
                       MinLogLevel = LogLevel.Fatal }
                   Dispatch = fun msg -> unbox<'msg> msg |> x.ProcessMessage
                   GetComponent = Component.getComponent
+                  SetComponent = Component.setComponent
                   SyncAction = fun fn -> fn() }
 
             member x.ProcessMessage(msg: 'msg) =

diff --git a/src/Fabulous/Attributes.fs b/src/Fabulous/Attributes.fs
@@ -217,7 +217,7 @@ module Attributes =
 
                     // Trigger the unmounted event
                     Dispatcher.dispatchEventForAllChildren itemNode widget Lifecycle.Unmounted
-                    itemNode.Disconnect()
+                    itemNode.Dispose()
 
                     // Remove the child from the UI tree
                     targetColl.RemoveAt(index)
@@ -247,7 +247,7 @@ module Attributes =
 
                     // Trigger the unmounted event for the old child
                     Dispatcher.dispatchEventForAllChildren prevItemNode oldWidget Lifecycle.Unmounted
-                    prevItemNode.Disconnect()
+                    prevItemNode.Dispose()
 
                     // Replace the existing child in the UI tree at the index with the new one
                     targetColl[index] <- unbox view
@@ -288,20 +288,16 @@ module Attributes =
             SimpleScalarAttributeDefinition.CreateAttributeData(
                 ScalarAttributeComparers.noCompare,
                 (fun _ (newValueOpt: MsgValue voption) node ->
-                    let event = getEvent node.Target
-
                     match node.TryGetHandler(name) with
                     | ValueNone -> ()
-                    | ValueSome handler -> event.RemoveHandler handler
+                    | ValueSome handler -> handler.Dispose()
 
                     match newValueOpt with
-                    | ValueNone -> node.SetHandler(name, ValueNone)
-
+                    | ValueNone -> node.RemoveHandler(name)
                     | ValueSome(MsgValue msg) ->
-                        let handler = EventHandler(fun _ _ -> Dispatcher.dispatch node msg)
-
-                        event.AddHandler handler
-                        node.SetHandler(name, ValueSome handler))
+                        let event = getEvent node.Target
+                        let handler = event.Subscribe(fun _ -> Dispatcher.dispatch node msg)
+                        node.SetHandler(name, handler))
             )
 
             |> AttributeDefinitionStore.registerScalar
@@ -317,23 +313,21 @@ module Attributes =
             SimpleScalarAttributeDefinition.CreateAttributeData(
                 ScalarAttributeComparers.noCompare,
                 (fun _ (newValueOpt: ('args -> MsgValue) voption) (node: IViewNode) ->
-                    let event = getEvent node.Target
-
                     match node.TryGetHandler(name) with
                     | ValueNone -> ()
-                    | ValueSome handler -> event.RemoveHandler handler
+                    | ValueSome handler -> handler.Dispose()
 
                     match newValueOpt with
-                    | ValueNone -> node.SetHandler(name, ValueNone)
-
+                    | ValueNone -> node.RemoveHandler(name)
                     | ValueSome fn ->
+                        let event = getEvent node.Target
+
                         let handler =
-                            EventHandler<'args>(fun _ args ->
+                            event.Subscribe(fun args ->
                                 let (MsgValue r) = fn args
                                 Dispatcher.dispatch node r)
 
-                        node.SetHandler(name, ValueSome handler)
-                        event.AddHandler handler)
+                        node.SetHandler(name, handler))
             )
             |> AttributeDefinitionStore.registerScalar
 
@@ -347,20 +341,15 @@ module Attributes =
             SimpleScalarAttributeDefinition.CreateAttributeData(
                 ScalarAttributeComparers.noCompare,
                 (fun _ (newValueOpt: (unit -> unit) voption) node ->
-                    let event = getEvent(node.Target)
-
                     match node.TryGetHandler(name) with
                     | ValueNone -> ()
-                    | ValueSome handler -> event.RemoveHandler handler
+                    | ValueSome handler -> handler.Dispose()
 
                     match newValueOpt with
-                    | ValueNone -> node.SetHandler(name, ValueNone)
-
+                    | ValueNone -> node.RemoveHandler(name)
                     | ValueSome(fn) ->
-                        let handler = EventHandler(fun _ _ -> fn())
-
-                        event.AddHandler handler
-                        node.SetHandler(name, ValueSome handler))
+                        let event = getEvent node.Target
+                        node.SetHandler(name, event.Subscribe(fun _ -> fn())))
             )
 
             |> AttributeDefinitionStore.registerScalar