From 3d6c518d066535eb287b8ae5ff02d299dbe482a4 Mon Sep 17 00:00:00 2001 From: jeparlefrancais Date: Wed, 8 Jul 2020 13:46:27 -0700 Subject: [PATCH 1/2] Add disconnectAll method to SingleEventManager Change RobloxRenderer to disconnect all events during unmount --- src/RobloxRenderer.lua | 4 ++++ src/RobloxRenderer.spec.lua | 20 ++++++++++++++++++++ src/SingleEventManager.lua | 13 +++++++++++++ src/SingleEventManager.spec.lua | 18 ++++++++++++++++++ 4 files changed, 55 insertions(+) diff --git a/src/RobloxRenderer.lua b/src/RobloxRenderer.lua index 4f528ad5..686fe143 100644 --- a/src/RobloxRenderer.lua +++ b/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 diff --git a/src/RobloxRenderer.spec.lua b/src/RobloxRenderer.spec.lua index 1ea04cb2..96b6f16f 100644 --- a/src/RobloxRenderer.spec.lua +++ b/src/RobloxRenderer.spec.lua @@ -11,6 +11,7 @@ return function() local GlobalConfig = require(script.Parent.GlobalConfig) local Portal = require(script.Parent.Portal) local Ref = require(script.Parent.PropMarkers.Ref) + local Event = require(script.Parent.PropMarkers.Event) local RobloxRenderer = require(script.Parent.RobloxRenderer) @@ -541,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() diff --git a/src/SingleEventManager.lua b/src/SingleEventManager.lua index bb579c78..8c2e11aa 100644 --- a/src/SingleEventManager.lua +++ b/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 \ No newline at end of file diff --git a/src/SingleEventManager.spec.lua b/src/SingleEventManager.spec.lua index 9d87e271..0a51f32e 100644 --- a/src/SingleEventManager.spec.lua +++ b/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 \ No newline at end of file From c158006d2bf776f562b23c0577043f5fe1462f57 Mon Sep 17 00:00:00 2001 From: jeparlefrancais Date: Wed, 8 Jul 2020 14:02:34 -0700 Subject: [PATCH 2/2] Add entry to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 713275f5..5622c363 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased Changes * 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.