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

setState sanity checking #23

Merged
merged 4 commits into from
Jan 29, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
15 changes: 15 additions & 0 deletions lib/Component.lua
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ function Component:extend(name)
local self = {}

self.props = props
-- Used for tracking whether the component is in a position to set state.
self._canSetState = false
self._context = {}

-- Shallow copy all context values from our parent element.
Expand All @@ -64,6 +66,9 @@ function Component:extend(name)
self.state = {}
end

-- Now that state has definitely been set, we can now allow it to be changed.
self._canSetState = true

return self
end

Expand Down Expand Up @@ -101,6 +106,14 @@ end
current state object.
]]
function Component:setState(partialState)
-- State cannot be set in any of the following places:
-- * During the component's init function
-- * During the component's render function
-- * After the component has been unmounted (or is in the process of unmounting, e.g. willUnmount)
if not self._canSetState then
error("State cannot be set at this point: are you setting state from an init, render, or willUnmount function?", 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be a little clearer to say are you calling setState from instead of are you setting state from, since you can you set state from init as long as you don't call setState itself!

end

local newState = {}

for key, value in pairs(self.state) do
Expand Down Expand Up @@ -149,7 +162,9 @@ function Component:_forceUpdate(newProps, newState)
self.state = newState
end

self._canSetState = false
local newChildElement = self:render()
self._canSetState = true

if self._handle._reified ~= nil then
-- We returned an element before, update it.
Expand Down
52 changes: 52 additions & 0 deletions lib/Component.spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,56 @@ return function()

Reconciler.teardown(instance)
end)

describe("setState", function()
it("should throw when called in init or render", function()
local InitComponent = Component:extend("InitComponent")

function InitComponent:init()
self:setState({
a = 1
})
end

local RenderComponent = Component:extend("RenderComponent")

function RenderComponent:render()
self:setState({
a = 1
})
end

local initElement = Core.createElement(InitComponent)
local renderElement = Core.createElement(RenderComponent)

expect(function()
Reconciler.reify(initElement)
end).to.throw()

expect(function()
Reconciler.reify(renderElement)
end).to.throw()
end)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can break this one into two tests, just like how willUnmount is a separate test


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

function TestComponent:render()
return nil
end

function TestComponent:willUnmount()
self:setState({
a = 1
})
end

local element = Core.createElement(TestComponent)
local instance = Reconciler.reify(element)

expect(function()
Reconciler.teardown(instance)
end).to.throw()
end)
end)
end
3 changes: 3 additions & 0 deletions lib/Reconciler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ function Reconciler.teardown(instanceHandle)
Reconciler.teardown(instanceHandle._reified)
end
elseif Core.isStatefulElement(element) then
-- Stop the component from setting state in willUnmount or anywhere thereafter.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, there are spaces for indentation here!

instanceHandle._instance._canSetState = false

-- Tell the component we're about to tear everything down.
-- This gives it some notice!
if instanceHandle._instance.willUnmount then
Expand Down