Disconnect events during unmount

CHANGELOG.md

@@ -10,6 +10,7 @@
 
 ## [1.3.1](https://github.com/Roblox/roact/releases/tag/v1.3.1) (November 19th, 2020)
 * Added component name to property validation error message ([#275](https://github.com/Roblox/roact/pull/275))
+* Fixed an issue where events were called while unmounting a host element ([#282](https://github.com/Roblox/roact/pull/282))
 
 ## [1.3.0](https://github.com/Roblox/roact/releases/tag/v1.3.0) (May 5th, 2020)
 * Added Contexts, which enables easy handling of items that are provided and consumed throughout the tree.

src/RobloxRenderer.lua

@@ -230,6 +230,10 @@ function RobloxRenderer.unmountHostNode(reconciler, virtualNode)
 
 	applyRef(element.props[Ref], nil)
 
+	if virtualNode.eventManager ~= nil then
+		virtualNode.eventManager:disconnectAll()
+	end
+
 	for _, childNode in pairs(virtualNode.children) do
 		reconciler.unmountVirtualNode(childNode)
 	end

src/RobloxRenderer.spec.lua

@@ -542,6 +542,25 @@ return function()
 			expect(spyRef.callCount).to.equal(2)
 			spyRef:assertCalledWith(nil)
 		end)
+
+		itSKIP("should not process events when unmounting", function()
+			local parent = Instance.new("Folder")
+			local key = "Some Key"
+
+			local element = createElement("Frame", {
+				[Event.ChildRemoved] = function(instance)
+					error("this callback should not be called during unmount")
+				end,
+			}, {
+				Child = createElement("Frame"),
+			})
+
+			local node = reconciler.createVirtualNode(element, parent, key)
+
+			RobloxRenderer.mountHostNode(reconciler, node)
+
+			RobloxRenderer.unmountHostNode(reconciler, node)
+		end)
 	end)
 
 	describe("Portals", function()

src/SingleEventManager.lua

@@ -144,4 +144,17 @@ function SingleEventManager:resume()
 	self._suspendedEventQueue = {}
 end
 
+function SingleEventManager:disconnectAll()
+	self._status = EventStatus.Disabled
+
+	for eventKey, connection in pairs(self._connections) do
+		connection:Disconnect()
+		self._listeners[eventKey] = nil
+	end
+
+	self._listeners = {}
+	self._connections = {}
+	self._suspendedEventQueue = {}
+end
+
 return SingleEventManager

src/SingleEventManager.spec.lua

@@ -236,4 +236,22 @@ return function()
 			end).to.throw()
 		end)
 	end)
+
+	describe("disconnectAll", function()
+		it("should not invoke events fired during suspension but disconnected before resumption", function()
+			local instance = Instance.new("BindableEvent")
+			local manager = SingleEventManager.new(instance)
+			local eventSpy = createSpy()
+
+			manager:connectEvent("Event", eventSpy.value)
+			manager:suspend()
+
+			instance:Fire(1)
+
+			manager:disconnectAll()
+
+			manager:resume()
+			expect(eventSpy.callCount).to.equal(0)
+		end)
+	end)
 end