Skip to content

Commit 918a4c1

Browse files
Fix bindings leak (#40)
1 parent 208b3e2 commit 918a4c1

File tree

2 files changed

+42
-6
lines changed

2 files changed

+42
-6
lines changed

modules/react-roblox/src/client/__tests__/ReactRobloxBindings.roblox.spec.luau

+28
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,34 @@ it("should not return the same root twice", function()
9898
jestExpect(reactRobloxRoot).never.toBe(reactRobloxRoot2)
9999
end)
100100

101+
it("should unsubscribe from bindings when unmounted", function()
102+
local counterBinding, setCounter = React.createBinding(0)
103+
local renderCount = 0
104+
local function Component()
105+
return React.createElement("TextLabel", {
106+
Text = counterBinding:map(function(counter)
107+
renderCount += 1
108+
return tostring(counter)
109+
end),
110+
})
111+
end
112+
ReactRoblox.act(function()
113+
reactRobloxRoot:render(React.createElement(Component))
114+
end)
115+
jestExpect(renderCount).toBe(1)
116+
ReactRoblox.act(function()
117+
reactRobloxRoot:render(React.createElement(Component))
118+
end)
119+
jestExpect(renderCount).toBe(2)
120+
setCounter(1)
121+
jestExpect(renderCount).toBe(3)
122+
ReactRoblox.act(function()
123+
reactRobloxRoot:unmount()
124+
end)
125+
setCounter(2)
126+
jestExpect(renderCount).toBe(3)
127+
end)
128+
101129
describe("useBinding hook", function()
102130
it("returns the same binding object each time", function()
103131
local captureBinding = jest.fn()

modules/react-roblox/src/client/roblox/RobloxComponentProps.luau

+14-6
Original file line numberDiff line numberDiff line change
@@ -306,15 +306,24 @@ local function updateProperties(
306306
end
307307
end
308308

309+
local function cleanupBindings(domElement: HostInstance)
310+
local instanceBindings = instanceToBindings[domElement]
311+
if instanceBindings ~= nil then
312+
for _, disconnectBinding in instanceBindings do
313+
disconnectBinding()
314+
end
315+
instanceToBindings[domElement] = nil
316+
end
317+
end
318+
309319
-- ROBLOX deviation: Clear out references to components when they unmount so we
310320
-- avoid leaking memory when they're removed
311321
local function cleanupHostComponent(domElement: HostInstance)
312322
if instanceToEventManager[domElement] ~= nil then
313323
instanceToEventManager[domElement] = nil
314324
end
315-
if instanceToBindings[domElement] ~= nil then
316-
instanceToBindings[domElement] = nil
317-
end
325+
326+
cleanupBindings(domElement)
318327

319328
-- ROBLOX https://jira.rbx.com/browse/LUAFDN-718: Tables are somehow ending up
320329
-- in this function that expects Instances. In that case, we won't be able to
@@ -328,9 +337,8 @@ local function cleanupHostComponent(domElement: HostInstance)
328337
if instanceToEventManager[descElement] ~= nil then
329338
instanceToEventManager[descElement] = nil
330339
end
331-
if instanceToBindings[descElement] ~= nil then
332-
instanceToBindings[descElement] = nil
333-
end
340+
341+
cleanupBindings(descElement)
334342
removeAllTags(domElement)
335343
end
336344
end

0 commit comments

Comments
 (0)