Global debug flag, better error messages (#188)

* Add messages to all asserts

* Add flag to the test runner. Technically, none of our tests yet rely on the asserts (which is surprising), but we still want hem on for tests

* Replace GlobalConfig functionality with something closer to the way we do debug stuff

* Replace all DEBUG checks with new GlobalConfig approach

* Remove the only-set-once logic from GlobalConfig

* Remove public exposure of GlobalConfig's  function, initialize config before tests

CHANGELOG.md

@@ -17,6 +17,7 @@
 * Introduced Roact Bindings. ([#159](https://github.com/Roblox/roact/pull/159))
 * Introduce setState suspension to replace event suspension. ([#183](https://github.com/Roblox/roact/pull/183))
 * By default, disable the warning for an element changing types during reconciliation ([#168](https://github.com/Roblox/roact/pull/168))
+* Refactor debugging asserts to be behind config flags ([#188](https://github.com/Roblox/roact/pull/188))
 
 ## 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))

bin/run-tests.server.lua

@@ -2,7 +2,15 @@
 
 local ReplicatedStorage = game:GetService("ReplicatedStorage")
 
+local Roact = require(ReplicatedStorage.Roact)
 local TestEZ = require(ReplicatedStorage.TestEZ)
+
+Roact.setGlobalConfig({
+	["internalTypeChecks"] = true,
+	["typeChecks"] = true,
+	["elementTracing"] = true,
+	["propertyValidation"] = true,
+})
 local results = TestEZ.TestBootstrap:run(ReplicatedStorage.Roact, TestEZ.Reporters.TextReporter)
 
 if __LEMUR__ then

lib/Binding.lua

@@ -2,6 +2,8 @@ local createSignal = require(script.Parent.createSignal)
 local Symbol = require(script.Parent.Symbol)
 local Type = require(script.Parent.Type)
 
+local config = require(script.Parent.GlobalConfig).get()
+
 --[[
 	Default mapping function used for non-mapped bindings
 ]]
@@ -46,6 +48,10 @@ end
 	Creates a new binding from this one with the given mapping.
 ]]
 function bindingPrototype:map(valueTransform)
+	if config.typeChecks then
+		assert(typeof(valueTransform) == "function", "Bad arg #1 to binding:map: expected function")
+	end
+
 	local binding = Binding.create(valueTransform(self:getValue()))
 
 	binding[InternalData].valueTransform = valueTransform

lib/Component.lua

@@ -3,7 +3,9 @@ local ComponentLifecyclePhase = require(script.Parent.ComponentLifecyclePhase)
 local Type = require(script.Parent.Type)
 local Symbol = require(script.Parent.Symbol)
 local invalidSetStateMessages = require(script.Parent.invalidSetStateMessages)
-local GlobalConfig = require(script.Parent.GlobalConfig)
+local internalAssert = require(script.Parent.internalAssert)
+
+local config = require(script.Parent.GlobalConfig).get()
 
 --[[
 	Calling setState during certain lifecycle allowed methods has the potential
@@ -41,8 +43,10 @@ Component.__componentName = "Component"
 	PureComponent.
 ]]
 function Component:extend(name)
-	assert(Type.of(self) == Type.StatefulComponentClass)
-	assert(typeof(name) == "string")
+	if config.typeChecks then
+		assert(Type.of(self) == Type.StatefulComponentClass, "Invalid `self` argument to `extend`.")
+		assert(typeof(name) == "string", "Component class name must be a string")
+	end
 
 	local class = {}
 
@@ -65,7 +69,9 @@ function Component:extend(name)
 end
 
 function Component:__getDerivedState(incomingProps, incomingState)
-	assert(Type.of(self) == Type.StatefulComponentInstance)
+	if config.internalTypeChecks then
+		internalAssert(Type.of(self) == Type.StatefulComponentInstance, "Invalid use of `__getDerivedState`")
+	end
 
 	local internalData = self[InternalData]
 	local componentClass = internalData.componentClass
@@ -74,7 +80,9 @@ function Component:__getDerivedState(incomingProps, incomingState)
 		local derivedState = componentClass.getDerivedStateFromProps(incomingProps, incomingState)
 
 		if derivedState ~= nil then
-			assert(typeof(derivedState) == "table", "getDerivedStateFromProps must return a table!")
+			if config.typeChecks then
+				assert(typeof(derivedState) == "table", "getDerivedStateFromProps must return a table!")
+			end
 
 			return derivedState
 		end
@@ -84,7 +92,9 @@ function Component:__getDerivedState(incomingProps, incomingState)
 end
 
 function Component:setState(mapState)
-	assert(Type.of(self) == Type.StatefulComponentInstance)
+	if config.typeChecks then
+		assert(Type.of(self) == Type.StatefulComponentInstance, "Invalid `self` argument to `extend`.")
+	end
 
 	local internalData = self[InternalData]
 	local lifecyclePhase = internalData.lifecyclePhase
@@ -194,7 +204,7 @@ end
 	error.
 ]]
 function Component:__validateProps(props)
-	if not GlobalConfig.getValue("propertyValidation") then
+	if not config.propertyValidation then
 		return
 	end
 
@@ -227,9 +237,10 @@ end
 	instance and attach it to the given virtualNode.
 ]]
 function Component:__mount(reconciler, virtualNode)
-	assert(Type.of(self) == Type.StatefulComponentClass)
-	assert(reconciler ~= nil)
-	assert(Type.of(virtualNode) == Type.VirtualNode)
+	if config.internalTypeChecks then
+		internalAssert(Type.of(self) == Type.StatefulComponentClass, "Invalid use of `__mount`")
+		internalAssert(Type.of(virtualNode) == Type.VirtualNode, "Expected arg #2 to be of type VirtualNode")
+	end
 
 	local currentElement = virtualNode.currentElement
 	local hostParent = virtualNode.hostParent
@@ -298,7 +309,9 @@ end
 	this component instance.
 ]]
 function Component:__unmount()
-	assert(Type.of(self) == Type.StatefulComponentInstance)
+	if config.internalTypeChecks then
+		internalAssert(Type.of(self) == Type.StatefulComponentInstance, "Invalid use of `__unmount`")
+	end
 
 	local internalData = self[InternalData]
 	local virtualNode = internalData.virtualNode
@@ -321,9 +334,17 @@ end
 	Returns true if the update was completed, false if it was cancelled by shouldUpdate
 ]]
 function Component:__update(updatedElement, updatedState)
-	assert(Type.of(self) == Type.StatefulComponentInstance)
-	assert(Type.of(updatedElement) == Type.Element or updatedElement == nil)
-	assert(typeof(updatedState) == "table" or updatedState == nil)
+	if config.internalTypeChecks then
+		internalAssert(Type.of(self) == Type.StatefulComponentInstance, "Invalid use of `__update`")
+		internalAssert(
+			Type.of(updatedElement) == Type.Element or updatedElement == nil,
+			"Expected arg #1 to be of type Element or nil"
+		)
+		internalAssert(
+			typeof(updatedState) == "table" or updatedState == nil,
+			"Expected arg #2 to be of type table or nil"
+		)
+	end
 
 	local internalData = self[InternalData]
 	local componentClass = internalData.componentClass
@@ -390,7 +411,9 @@ end
 	Returns true if the update was completed, false if it was cancelled by shouldUpdate
 ]]
 function Component:__resolveUpdate(incomingProps, incomingState)
-	assert(Type.of(self) == Type.StatefulComponentInstance)
+	if config.internalTypeChecks then
+		internalAssert(Type.of(self) == Type.StatefulComponentInstance, "Invalid use of `__resolveUpdate`")
+	end
 
 	local internalData = self[InternalData]
 	local virtualNode = internalData.virtualNode