Roll back changes that allowed setState to be called inside willUpdate (

#157)

CHANGELOG.md

@@ -12,6 +12,7 @@
 * Added a warning when an element changes type during reconciliation ([#88](https://github.com/Roblox/roact/issues/88), [#137](https://github.com/Roblox/roact/pull/137))
 * Ref switching now occurs in one pass, which should fix edge cases where the result of a ref is `nil`, especially in property changed events ([#98](https://github.com/Roblox/roact/pull/98))
 * `setState` can now be called inside `init` and `willUpdate`. Instead of triggering a new render, it will affect the currently scheduled one. ([#139](https://github.com/Roblox/roact/pull/139))
+* Roll back changes that allowed `setState` to be called inside `willUpdate`, which created state update scenarios with difficult-to-determine behavior. ([#157](https://github.com/Roblox/roact/pull/157))
 
 ## 1.0.0 Prerelease 2 (March 22, 2018)
 * Removed `is*Element` methods, this is unlikely to affect anyone ([#50](https://github.com/Roblox/roact/pull/50))

docs/api-reference.md

@@ -295,10 +295,10 @@ Setting a field in the state to `Roact.None` will clear it from the state. This
 !!! warning
 	`setState` can be called from anywhere **except**:
 
-	* Lifecycle hooks: `willUnmount`
+	* Lifecycle hooks: `willUnmount`, `willUpdate`
 	* Pure functions: `render`, `shouldUpdate`
 
-	Calling `setState` inside of `init` or `willUpdate` has special behavior. Because Roact is already going to update a component in these cases, that update will be replaced instead of another being scheduled.
+	Calling `setState` inside of `init` has special behavior. The result of setState will be used to determine initial state, and no additional updates will be scheduled.
 
 	Roact may support calling `setState` in currently-disallowed places in the future.
 
@@ -364,8 +364,6 @@ willUpdate(nextProps, nextState) -> void
 
 `willUpdate` is fired after an update is started but before a component's state and props are updated.
 
-`willUpdate` can be used to make tweaks to your component's state using `setState`. Often, this should be done in `getDerivedStateFromProps` instead.
-
 ### didUpdate
 ```
 didUpdate(previousProps, previousState) -> void

lib/Component.lua

@@ -93,7 +93,7 @@ function Component:extend(name)
 		self._setStateBlockedReason = nil
 
 		-- When set to true, setState should not trigger an update, but should
-		-- instead just update self.state. Lifecycle events like `willUpdate`
+		-- instead just update self.state. Lifecycle events like `init`
 		-- can set this to change the behavior of setState slightly.
 		self._setStateWithoutUpdate = false
 
@@ -321,9 +321,9 @@ end
 ]]
 function Component:_forceUpdate(newProps, newState)
 	if self.willUpdate then
-		self._setStateWithoutUpdate = true
+		self._setStateBlockedReason = "willUpdate"
 		self:willUpdate(newProps or self.props, newState or self.state)
-		self._setStateWithoutUpdate = false
+		self._setStateBlockedReason = nil
 	end
 
 	local oldProps = self.props

lib/Component.spec.lua

@@ -434,19 +434,10 @@ return function()
 			end).to.throw()
 		end)
 
-		it("should only render once when called in willUpdate", function()
+		it("should throw when called in willUpdate", function()
 			local TestComponent = Component:extend("TestComponent")
-			local forceUpdate
 
-			function TestComponent:init()
-				forceUpdate = function()
-					self:_forceUpdate()
-				end
-			end
-
-			local renderCount = 0
 			function TestComponent:render()
-				renderCount = renderCount + 1
 				return nil
 			end
 
@@ -456,17 +447,19 @@ return function()
 				})
 			end
 
-			local testElement = createElement(TestComponent)
-
-			local handle = Reconciler.mount(testElement)
-
-			expect(renderCount).to.equal(1)
-
-			forceUpdate()
+			local element = createElement(TestComponent, {
+				someProp = 1,
+			})
 
-			expect(renderCount).to.equal(2)
+			local handle = Reconciler.mount(element)
 
-			Reconciler.unmount(handle)
+			expect(function()
+				handle = Reconciler.reconcile(handle, {
+					createElement(TestComponent, {
+						someProp = 2
+					})
+				})
+			end).to.throw()
 		end)
 
 		it("should only render once when called in init", function()