Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Commit

Permalink
Fix bug in context consumers willUnmount (#320)
Browse files Browse the repository at this point in the history
  • Loading branch information
oltrep authored Sep 21, 2021
1 parent 97a30b3 commit a3adfa6
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased Changes

* Fixed `Listeners can only be disconnected once` from context consumers. ([#320](https://github.com/Roblox/roact/pull/320))

## [1.4.1](https://github.com/Roblox/roact/releases/tag/v1.4.1) (August 12th, 2021)
* Fixed a bug where the Roact tree could get into a broken state when using callbacks passed to a child component. Updated the tempFixUpdateChildrenReEntrancy config value to also handle this case. ([#315](https://github.com/Roblox/roact/pull/315))
* Fixed forwardRef description ([#312](https://github.com/Roblox/roact/pull/312)).
Expand Down
1 change: 1 addition & 0 deletions src/createContext.lua
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ local function createConsumer(context)
function Consumer:willUnmount()
if self.disconnect ~= nil then
self.disconnect()
self.disconnect = nil
end
end

Expand Down
97 changes: 97 additions & 0 deletions src/createContext.spec.lua
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
return function()
local ReplicatedStorage = game:GetService("ReplicatedStorage")

local Component = require(script.Parent.Component)
local NoopRenderer = require(script.Parent.NoopRenderer)
local Children = require(script.Parent.PropMarkers.Children)
Expand All @@ -10,6 +12,9 @@ return function()

local noopReconciler = createReconciler(NoopRenderer)

local RobloxRenderer = require(script.Parent.RobloxRenderer)
local robloxReconciler = createReconciler(RobloxRenderer)

it("should return a table", function()
local context = createContext("Test")
expect(context).to.be.ok()
Expand Down Expand Up @@ -301,4 +306,96 @@ return function()
expect(observedB).to.equal(true)
end)
end)

-- issue https://github.com/Roblox/roact/issues/319
it("does not throw if willUnmount is called twice on a context consumer", function()
local context = createContext({})

local LowestComponent = Component:extend("LowestComponent")
function LowestComponent:init()
end

function LowestComponent:render()
return createElement("Frame")
end

function LowestComponent:didMount()
self.props.onDidMountCallback()
end

local FirstComponent = Component:extend("FirstComponent")
function FirstComponent:init()
end

function FirstComponent:render()
return createElement(context.Consumer, {
render = function()
return createElement("TextLabel")
end,
})
end

local ChildComponent = Component:extend("ChildComponent")

function ChildComponent:init()
self:setState({ firstTime = true })
end

local childCallback

function ChildComponent:render()
if self.state.firstTime then
return createElement(FirstComponent)
end

return createElement(LowestComponent, {
onDidMountCallback = self.props.onDidMountCallback
})
end

function ChildComponent:didMount()
childCallback = function()
self:setState({ firstTime = false })
end
end

local ParentComponent = Component:extend("ParentComponent")

local didMountCallbackCalled = 0

function ParentComponent:init()
self:setState({ count = 1 })

self.onDidMountCallback = function()
didMountCallbackCalled = didMountCallbackCalled + 1
if self.state.count < 5 then
self:setState({ count = self.state.count + 1 })
end
end
end

function ParentComponent:render()
return createElement("Frame", {}, {
Provider = createElement(context.Provider, {
value = {},
}, {
ChildComponent = createElement(ChildComponent, {
count = self.state.count,
onDidMountCallback = self.onDidMountCallback,
}),
})
})
end

local parent = Instance.new("ScreenGui")
parent.Parent = ReplicatedStorage

local hostKey = "Some Key"
robloxReconciler.mountVirtualNode(createElement(ParentComponent), parent, hostKey)

expect(function()
-- calling setState on ChildComponent will trigger `willUnmount` multiple times
childCallback()
end).never.to.throw()
end)
end

0 comments on commit a3adfa6

Please sign in to comment.