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

Commit

Permalink
Remove the warning for setState on unmounted components (#323)
Browse files Browse the repository at this point in the history
Consistent with upstream React, we should remove the error message when you setState on an unmounted component.

In summary, while it can warn on potential memory leaks, false positives is common and can push people to undesirable workarounds:

function TestComponent:didMount()
    testPromise():andThen(function(newState)
        -- Component might unmount before our promise resolves
        self:setState(newState)
    end)
end
For the full reasoning behind removing the warning, see Dan Abramov's PR: facebook/react#22114
  • Loading branch information
chriscerie committed Oct 26, 2021
1 parent 0f26436 commit 2239e01
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Roact Changelog

## Unreleased Changes
* Removed the warning for `setState` on unmounted components to eliminate false positive warnings, matching upstream React ([#323](https://github.com/Roblox/roact/pull/323)).

## [1.4.2](https://github.com/Roblox/roact/releases/tag/v1.4.2) (October 6th, 2021)
* Fixed forwardRef doc code referencing React instead of Roact ([#310](https://github.com/Roblox/roact/pull/310)).
Expand Down
7 changes: 4 additions & 3 deletions src/Component.lua
Original file line number Diff line number Diff line change
Expand Up @@ -100,21 +100,22 @@ function Component:setState(mapState)
local lifecyclePhase = internalData.lifecyclePhase

--[[
When preparing to update, rendering, or unmounting, it is not safe
When preparing to update, render, or unmount, it is not safe
to call `setState` as it will interfere with in-flight updates. It's
also disallowed during unmounting
]]
if
lifecyclePhase == ComponentLifecyclePhase.ShouldUpdate
or lifecyclePhase == ComponentLifecyclePhase.WillUpdate
or lifecyclePhase == ComponentLifecyclePhase.Render
or lifecyclePhase == ComponentLifecyclePhase.WillUnmount
then
local messageTemplate = invalidSetStateMessages[internalData.lifecyclePhase]

local message = messageTemplate:format(tostring(internalData.componentClass))

error(message, 2)
elseif lifecyclePhase == ComponentLifecyclePhase.WillUnmount then
-- Should not print error message. See https://github.com/facebook/react/pull/22114
return
end

local pendingState = internalData.pendingState
Expand Down
8 changes: 3 additions & 5 deletions src/Component.spec/setState.spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ return function()
expect(result:match("TestComponent")).to.be.ok()
end)

it("should throw when called in willUnmount", function()
it("should not throw when called in willUnmount", function()
local TestComponent = Component:extend("TestComponent")

function TestComponent:render()
Expand All @@ -125,11 +125,9 @@ return function()
local element = createElement(TestComponent)
local tree = noopReconciler.mountVirtualTree(element)

local success, result = pcall(noopReconciler.unmountVirtualTree, tree)
local success, _ = pcall(noopReconciler.unmountVirtualTree, tree)

expect(success).to.equal(false)
expect(result:match("willUnmount")).to.be.ok()
expect(result:match("TestComponent")).to.be.ok()
expect(success).to.equal(true)
end)

it("should remove values from state when the value is None", function()
Expand Down
6 changes: 0 additions & 6 deletions src/invalidSetStateMessages.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ Consider using the didUpdate method instead, or using getDerivedStateFromProps.
Check the definition of willUpdate in the component %q.]]

invalidSetStateMessages[ComponentLifecyclePhase.WillUnmount] = [[
setState cannot be used in the willUnmount lifecycle method.
A component that is being unmounted cannot be updated!
Check the definition of willUnmount in the component %q.]]

invalidSetStateMessages[ComponentLifecyclePhase.ShouldUpdate] = [[
setState cannot be used in the shouldUpdate lifecycle method.
shouldUpdate must be a pure function that only depends on props and state.
Expand Down

0 comments on commit 2239e01

Please sign in to comment.