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

Fix bug in context consumers #320

Merged
merged 4 commits into from
Sep 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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