diff --git a/CHANGELOG.md b/CHANGELOG.md index 7369e984..756e7474 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/bin/run-tests.server.lua b/bin/run-tests.server.lua index f794e0c1..9b5f8dba 100644 --- a/bin/run-tests.server.lua +++ b/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 diff --git a/lib/Binding.lua b/lib/Binding.lua index abef64b8..06a9e267 100644 --- a/lib/Binding.lua +++ b/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 diff --git a/lib/Component.lua b/lib/Component.lua index 31bf7930..dc0db188 100644 --- a/lib/Component.lua +++ b/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 diff --git a/lib/Component.spec/validateProps.spec.lua b/lib/Component.spec/validateProps.spec.lua index 01520526..91d1bca1 100644 --- a/lib/Component.spec/validateProps.spec.lua +++ b/lib/Component.spec/validateProps.spec.lua @@ -10,225 +10,231 @@ return function() local noopReconciler = createReconciler(NoopRenderer) it("should be invoked when mounted", function() - GlobalConfig.set({ + local config = { propertyValidation = true, - }) - - local MyComponent = Component:extend("MyComponent") + } - local validatePropsSpy = createSpy(function() - return true - end) + GlobalConfig.scoped(config, function() + local MyComponent = Component:extend("MyComponent") - MyComponent.validateProps = validatePropsSpy.value + local validatePropsSpy = createSpy(function() + return true + end) - function MyComponent:render() - return nil - end + MyComponent.validateProps = validatePropsSpy.value - local element = createElement(MyComponent) - local hostParent = nil - local key = "Test" + function MyComponent:render() + return nil + end - noopReconciler.mountVirtualNode(element, hostParent, key) - expect(validatePropsSpy.callCount).to.equal(1) + local element = createElement(MyComponent) + local hostParent = nil + local key = "Test" - GlobalConfig.reset() + noopReconciler.mountVirtualNode(element, hostParent, key) + expect(validatePropsSpy.callCount).to.equal(1) + end) end) it("should be invoked when props change", function() - GlobalConfig.set({ + local config = { propertyValidation = true, - }) - - local MyComponent = Component:extend("MyComponent") - - local validatePropsSpy = createSpy(function() - return true - end) - - MyComponent.validateProps = validatePropsSpy.value + } - function MyComponent:render() - return nil - end + GlobalConfig.scoped(config, function() + local MyComponent = Component:extend("MyComponent") - local element = createElement(MyComponent, { a = 1 }) - local hostParent = nil - local key = "Test" + local validatePropsSpy = createSpy(function() + return true + end) - local node = noopReconciler.mountVirtualNode(element, hostParent, key) - expect(validatePropsSpy.callCount).to.equal(1) - validatePropsSpy:assertCalledWithDeepEqual({ - a = 1, - }) + MyComponent.validateProps = validatePropsSpy.value - local newElement = createElement(MyComponent, { a = 2 }) - noopReconciler.updateVirtualNode(node, newElement) - expect(validatePropsSpy.callCount).to.equal(2) - validatePropsSpy:assertCalledWithDeepEqual({ - a = 2, - }) + function MyComponent:render() + return nil + end - GlobalConfig.reset() + local element = createElement(MyComponent, { a = 1 }) + local hostParent = nil + local key = "Test" + + local node = noopReconciler.mountVirtualNode(element, hostParent, key) + expect(validatePropsSpy.callCount).to.equal(1) + validatePropsSpy:assertCalledWithDeepEqual({ + a = 1, + }) + + local newElement = createElement(MyComponent, { a = 2 }) + noopReconciler.updateVirtualNode(node, newElement) + expect(validatePropsSpy.callCount).to.equal(2) + validatePropsSpy:assertCalledWithDeepEqual({ + a = 2, + }) + end) end) it("should not be invoked when state changes", function() - GlobalConfig.set({ + local config = { propertyValidation = true, - }) + } - local MyComponent = Component:extend("MyComponent") + GlobalConfig.scoped(config, function() + local MyComponent = Component:extend("MyComponent") - local setStateCallback = nil - local validatePropsSpy = createSpy(function() - return true - end) + local setStateCallback = nil + local validatePropsSpy = createSpy(function() + return true + end) - MyComponent.validateProps = validatePropsSpy.value + MyComponent.validateProps = validatePropsSpy.value - function MyComponent:init() - setStateCallback = function(newState) - self:setState(newState) + function MyComponent:init() + setStateCallback = function(newState) + self:setState(newState) + end end - end - - function MyComponent:render() - return nil - end - local element = createElement(MyComponent, { a = 1 }) - local hostParent = nil - local key = "Test" + function MyComponent:render() + return nil + end - noopReconciler.mountVirtualNode(element, hostParent, key) - expect(validatePropsSpy.callCount).to.equal(1) - validatePropsSpy:assertCalledWithDeepEqual({ - a = 1 - }) + local element = createElement(MyComponent, { a = 1 }) + local hostParent = nil + local key = "Test" - setStateCallback({ - b = 1 - }) + noopReconciler.mountVirtualNode(element, hostParent, key) + expect(validatePropsSpy.callCount).to.equal(1) + validatePropsSpy:assertCalledWithDeepEqual({ + a = 1 + }) - expect(validatePropsSpy.callCount).to.equal(1) + setStateCallback({ + b = 1 + }) - GlobalConfig.reset() + expect(validatePropsSpy.callCount).to.equal(1) + end) end) it("should throw if validateProps is not a function", function() - GlobalConfig.set({ + local config = { propertyValidation = true, - }) - - local MyComponent = Component:extend("MyComponent") - MyComponent.validateProps = 1 + } - function MyComponent:render() - return nil - end + GlobalConfig.scoped(config, function() + local MyComponent = Component:extend("MyComponent") + MyComponent.validateProps = 1 - local element = createElement(MyComponent) - local hostParent = nil - local key = "Test" + function MyComponent:render() + return nil + end - expect(function() - noopReconciler.mountVirtualNode(element, hostParent, key) - end).to.throw() + local element = createElement(MyComponent) + local hostParent = nil + local key = "Test" - GlobalConfig.reset() + expect(function() + noopReconciler.mountVirtualNode(element, hostParent, key) + end).to.throw() + end) end) it("should throw if validateProps returns false", function() - GlobalConfig.set({ + local config = { propertyValidation = true, - }) - - local MyComponent = Component:extend("MyComponent") - MyComponent.validateProps = function() - return false - end + } - function MyComponent:render() - return nil - end + GlobalConfig.scoped(config, function() + local MyComponent = Component:extend("MyComponent") + MyComponent.validateProps = function() + return false + end - local element = createElement(MyComponent) - local hostParent = nil - local key = "Test" + function MyComponent:render() + return nil + end - expect(function() - noopReconciler.mountVirtualNode(element, hostParent, key) - end).to.throw() + local element = createElement(MyComponent) + local hostParent = nil + local key = "Test" - GlobalConfig.reset() + expect(function() + noopReconciler.mountVirtualNode(element, hostParent, key) + end).to.throw() + end) end) it("should be invoked after defaultProps are applied", function() - GlobalConfig.set({ + local config = { propertyValidation = true, - }) - - local MyComponent = Component:extend("MyComponent") + } - local validatePropsSpy = createSpy(function() - return true - end) + GlobalConfig.scoped(config, function() + local MyComponent = Component:extend("MyComponent") - MyComponent.validateProps = validatePropsSpy.value + local validatePropsSpy = createSpy(function() + return true + end) - function MyComponent:render() - return nil - end + MyComponent.validateProps = validatePropsSpy.value - MyComponent.defaultProps = { - b = 2, - } + function MyComponent:render() + return nil + end - local element = createElement(MyComponent, { a = 1 }) - local hostParent = nil - local key = "Test" - - local node = noopReconciler.mountVirtualNode(element, hostParent, key) - expect(validatePropsSpy.callCount).to.equal(1) - validatePropsSpy:assertCalledWithDeepEqual({ - a = 1, - b = 2, - }) - - local newElement = createElement(MyComponent, { a = 2 }) - noopReconciler.updateVirtualNode(node, newElement) - expect(validatePropsSpy.callCount).to.equal(2) - validatePropsSpy:assertCalledWithDeepEqual({ - a = 2, - b = 2, - }) - - GlobalConfig.reset() + MyComponent.defaultProps = { + b = 2, + } + + local element = createElement(MyComponent, { a = 1 }) + local hostParent = nil + local key = "Test" + + local node = noopReconciler.mountVirtualNode(element, hostParent, key) + expect(validatePropsSpy.callCount).to.equal(1) + validatePropsSpy:assertCalledWithDeepEqual({ + a = 1, + b = 2, + }) + + local newElement = createElement(MyComponent, { a = 2 }) + noopReconciler.updateVirtualNode(node, newElement) + expect(validatePropsSpy.callCount).to.equal(2) + validatePropsSpy:assertCalledWithDeepEqual({ + a = 2, + b = 2, + }) + end) end) it("should not be invoked if the flag is off", function() - local MyComponent = Component:extend("MyComponent") + local config = { + propertyValidation = false, + } - local validatePropsSpy = createSpy(function() - return true - end) + GlobalConfig.scoped(config, function() + local MyComponent = Component:extend("MyComponent") - MyComponent.validateProps = validatePropsSpy.value + local validatePropsSpy = createSpy(function() + return true + end) - function MyComponent:render() - return nil - end + MyComponent.validateProps = validatePropsSpy.value - local element = createElement(MyComponent, { a = 1 }) - local hostParent = nil - local key = "Test" + function MyComponent:render() + return nil + end - local node = noopReconciler.mountVirtualNode(element, hostParent, key) - expect(validatePropsSpy.callCount).to.equal(0) + local element = createElement(MyComponent, { a = 1 }) + local hostParent = nil + local key = "Test" - local newElement = createElement(MyComponent, { a = 2 }) - noopReconciler.updateVirtualNode(node, newElement) - expect(validatePropsSpy.callCount).to.equal(0) + local node = noopReconciler.mountVirtualNode(element, hostParent, key) + expect(validatePropsSpy.callCount).to.equal(0) + + local newElement = createElement(MyComponent, { a = 2 }) + noopReconciler.updateVirtualNode(node, newElement) + expect(validatePropsSpy.callCount).to.equal(0) + end) end) end \ No newline at end of file diff --git a/lib/Config.lua b/lib/Config.lua index df0a32a6..8c45a3bb 100644 --- a/lib/Config.lua +++ b/lib/Config.lua @@ -14,12 +14,12 @@ -- Every valid configuration value should be non-nil in this table. local defaultConfig = { + -- Enables asserts for internal Roact APIs. Useful for debugging Roact itself. + ["internalTypeChecks"] = false, + -- Enables stricter type asserts for Roact's public API. + ["typeChecks"] = false, -- Enables storage of `debug.traceback()` values on elements for debugging. ["elementTracing"] = false, - -- Enables instrumentation of shouldUpdate and render methods for Roact components - ["componentInstrumentation"] = false, - -- Enables warnings if an element changes type after being rendered. - ["warnOnTypeChange"] = false, -- Enables validation of component props in stateful components. ["propertyValidation"] = false, } @@ -30,34 +30,23 @@ for key in pairs(defaultConfig) do table.insert(defaultConfigKeys, key) end ---[[ - Merges two tables together into a new table. -]] -local function join(a, b) - local new = {} - - for key, value in pairs(a) do - new[key] = value - end - - for key, value in pairs(b) do - new[key] = value - end - - return new -end - local Config = {} function Config.new() local self = {} - -- Once configuration has been set, we record a traceback. - -- That way, if the user mistakenly calls `set` twice, we can point to the - -- first place it was called. - self._lastConfigTraceback = nil + self._currentConfig = setmetatable({}, { + __index = function(_, key) + local message = ( + "Invalid global configuration key %q. Valid configuration keys are: %s" + ):format( + tostring(key), + table.concat(defaultConfigKeys, ", ") + ) - self._currentConfig = defaultConfig + error(message, 3) + end + }) -- We manually bind these methods here so that the Config's methods can be -- used without passing in self, since they eventually get exposed on the @@ -66,37 +55,20 @@ function Config.new() return Config.set(self, ...) end - self.getValue = function(...) - return Config.getValue(self, ...) - end - - self.reset = function(...) - return Config.reset(self, ...) + self.get = function(...) + return Config.get(self, ...) end self.scoped = function(...) return Config.scoped(self, ...) end + self.set(defaultConfig) + return self end function Config:set(configValues) - if self._lastConfigTraceback then - local message = ( - "Global configuration can only be set once. Configuration was already set at:%s" - ):format( - self._lastConfigTraceback - ) - - error(message, 3) - end - - -- We use 3 as our traceback and error level because all of the methods are - -- manually bound to 'self', which creates an additional stack frame we want - -- to skip through. - self._lastConfigTraceback = debug.traceback("", 3) - -- Validate values without changing any configuration. -- We only want to apply this configuration if it's valid! for key, value in pairs(configValues) do @@ -124,39 +96,26 @@ function Config:set(configValues) error(message, 3) end - end - - -- Assign all of the (validated) configuration values in one go. - self._currentConfig = join(self._currentConfig, configValues) -end -function Config:getValue(key) - if defaultConfig[key] == nil then - local message = ( - "Invalid global configuration key %q (type %s). Valid configuration keys are: %s" - ):format( - tostring(key), - typeof(key), - table.concat(defaultConfigKeys, ", ") - ) - - error(message, 3) + self._currentConfig[key] = value end - - return self._currentConfig[key] end -function Config:reset() - self._lastConfigTraceback = nil - self._currentConfig = defaultConfig +function Config:get() + return self._currentConfig end function Config:scoped(configValues, callback) + local previousValues = {} + for key, value in pairs(self._currentConfig) do + previousValues[key] = value + end + self.set(configValues) local success, result = pcall(callback) - self.reset() + self.set(previousValues) assert(success, result) end diff --git a/lib/Config.spec.lua b/lib/Config.spec.lua index cec70760..08a884fc 100644 --- a/lib/Config.spec.lua +++ b/lib/Config.spec.lua @@ -3,14 +3,15 @@ return function() it("should accept valid configuration", function() local config = Config.new() + local values = config.get() - expect(config.getValue("elementTracing")).to.equal(false) + expect(values.elementTracing).to.equal(false) config.set({ elementTracing = true, }) - expect(config.getValue("elementTracing")).to.equal(true) + expect(values.elementTracing).to.equal(true) end) it("should reject invalid configuration keys", function() @@ -48,39 +49,4 @@ return function() expect(err:find(goodKey)).to.be.ok() expect(err:find(badValue)).to.be.ok() end) - - it("should prevent setting configuration more than once", function() - local config = Config.new() - - -- We're going to use the name of this function to see if the traceback - -- was correct. - local function setEmptyConfig() - config.set({}) - end - - setEmptyConfig() - - local ok, err = pcall(setEmptyConfig) - - expect(ok).to.equal(false) - - -- The error should mention the stack trace with the original set call. - expect(err:find("setEmptyConfig")).to.be.ok() - end) - - it("should reset to default values after invoking reset()", function() - local config = Config.new() - - expect(config.getValue("elementTracing")).to.equal(false) - - config.set({ - elementTracing = true, - }) - - expect(config.getValue("elementTracing")).to.equal(true) - - config.reset() - - expect(config.getValue("elementTracing")).to.equal(false) - end) end \ No newline at end of file diff --git a/lib/GlobalConfig.spec.lua b/lib/GlobalConfig.spec.lua index 5974c79f..760a2a36 100644 --- a/lib/GlobalConfig.spec.lua +++ b/lib/GlobalConfig.spec.lua @@ -4,7 +4,6 @@ return function() it("should have the correct methods", function() expect(GlobalConfig).to.be.ok() expect(GlobalConfig.set).to.be.ok() - expect(GlobalConfig.getValue).to.be.ok() - expect(GlobalConfig.reset).to.be.ok() + expect(GlobalConfig.get).to.be.ok() end) end \ No newline at end of file diff --git a/lib/RobloxRenderer.lua b/lib/RobloxRenderer.lua index d7737f25..09bd6eb4 100644 --- a/lib/RobloxRenderer.lua +++ b/lib/RobloxRenderer.lua @@ -11,6 +11,9 @@ local SingleEventManager = require(script.Parent.SingleEventManager) local getDefaultInstanceProperty = require(script.Parent.getDefaultInstanceProperty) local Ref = require(script.Parent.PropMarkers.Ref) local Type = require(script.Parent.Type) +local internalAssert = require(script.Parent.internalAssert) + +local config = require(script.Parent.GlobalConfig).get() local applyPropsError = [[ Error applying props: @@ -178,11 +181,13 @@ function RobloxRenderer.mountHostNode(reconciler, virtualNode) local hostParent = virtualNode.hostParent local hostKey = virtualNode.hostKey - assert(ElementKind.of(element) == ElementKind.Host) - - -- TODO: Better error messages - assert(element.props.Name == nil) - assert(element.props.Parent == nil) + if config.internalTypeChecks then + internalAssert(ElementKind.of(element) == ElementKind.Host, "Element at given node is not a host Element") + end + if config.typeChecks then + assert(element.props.Name == nil, "Name can not be specified as a prop to a host component in Roact.") + assert(element.props.Parent == nil, "Parent can not be specified as a prop to a host component in Roact.") + end local instance = Instance.new(element.component) virtualNode.hostObject = instance diff --git a/lib/createElement.lua b/lib/createElement.lua index 33123f15..b902219f 100644 --- a/lib/createElement.lua +++ b/lib/createElement.lua @@ -1,9 +1,10 @@ local Children = require(script.Parent.PropMarkers.Children) local ElementKind = require(script.Parent.ElementKind) -local GlobalConfig = require(script.Parent.GlobalConfig) local Logging = require(script.Parent.Logging) local Type = require(script.Parent.Type) +local config = require(script.Parent.GlobalConfig).get() + local multipleChildrenMessage = [[ The prop `Roact.Children` was defined but was overriden by the third parameter to createElement! This can happen when a component passes props through to a child element but also uses the `children` argument: @@ -34,9 +35,11 @@ Instead, consider using a utility function to merge tables of children together: props. If specified, the passed `props` table is mutated! ]] local function createElement(component, props, children) - assert(component ~= nil, "`component` is required") - assert(typeof(props) == "table" or props == nil, "`props` must be a table or nil") - assert(typeof(children) == "table" or children == nil, "`children` must be a table or nil") + if config.typeChecks then + assert(component ~= nil, "`component` is required") + assert(typeof(props) == "table" or props == nil, "`props` must be a table or nil") + assert(typeof(children) == "table" or children == nil, "`children` must be a table or nil") + end if props == nil then props = {} @@ -59,7 +62,7 @@ local function createElement(component, props, children) props = props, } - if GlobalConfig.getValue("elementTracing") then + if config.elementTracing then -- We trim out the leading newline since there's no way to specify the -- trace level without also specifying a message. element.source = debug.traceback("", 2):sub(2) diff --git a/lib/createReconciler.lua b/lib/createReconciler.lua index ae72e74c..e3638203 100644 --- a/lib/createReconciler.lua +++ b/lib/createReconciler.lua @@ -3,6 +3,9 @@ local ElementKind = require(script.Parent.ElementKind) local ElementUtils = require(script.Parent.ElementUtils) local Children = require(script.Parent.PropMarkers.Children) local Logging = require(script.Parent.Logging) +local internalAssert = require(script.Parent.internalAssert) + +local config = require(script.Parent.GlobalConfig).get() --[[ The reconciler is the mechanism in Roact that constructs the virtual tree @@ -50,7 +53,9 @@ local function createReconciler(renderer) updated children given as elements. ]] local function updateChildren(virtualNode, hostParent, newChildElements) - assert(Type.of(virtualNode) == Type.VirtualNode) + if config.internalTypeChecks then + internalAssert(Type.of(virtualNode) == Type.VirtualNode, "Expected arg #1 to be of type VirtualNode") + end local removeKeys = {} @@ -113,7 +118,9 @@ local function createReconciler(renderer) Unmounts the given virtual node and releases any held resources. ]] function unmountVirtualNode(virtualNode) - assert(Type.of(virtualNode) == Type.VirtualNode) + if config.internalTypeChecks then + internalAssert(Type.of(virtualNode) == Type.VirtualNode, "Expected arg #1 to be of type VirtualNode") + end local kind = ElementKind.of(virtualNode.currentElement) @@ -148,8 +155,7 @@ local function createReconciler(renderer) local targetHostParent = newElement.props.target - -- TODO: Error message - assert(renderer.isHostObject(targetHostParent)) + assert(renderer.isHostObject(targetHostParent), "Expected target to be host object") if targetHostParent ~= oldTargetHostParent then -- TODO: Better warning @@ -178,8 +184,15 @@ local function createReconciler(renderer) a warning to the user. ]] function updateVirtualNode(virtualNode, newElement, newState) - assert(Type.of(virtualNode) == Type.VirtualNode) - assert(Type.of(newElement) == Type.Element or typeof(newElement) == "boolean" or newElement == nil) + if config.internalTypeChecks then + internalAssert(Type.of(virtualNode) == Type.VirtualNode, "Expected arg #1 to be of type VirtualNode") + end + if config.typeChecks then + assert( + Type.of(newElement) == Type.Element or typeof(newElement) == "boolean" or newElement == nil, + "Expected arg #2 to be of type Element, boolean, or nil" + ) + end -- If nothing changed, we can skip this update if virtualNode.currentElement == newElement and newState == nil then @@ -226,10 +239,17 @@ local function createReconciler(renderer) Constructs a new virtual node but not does mount it. ]] local function createVirtualNode(element, hostParent, hostKey, context) - assert(Type.of(element) == Type.Element or typeof(element) == "boolean") - assert(renderer.isHostObject(hostParent) or hostParent == nil) - assert(hostKey ~= nil) - assert(typeof(context) == "table" or context == nil) + if config.internalTypeChecks then + internalAssert(renderer.isHostObject(hostParent) or hostParent == nil, "Expected arg #2 to be a host object") + internalAssert(typeof(context) == "table" or context == nil, "Expected arg #4 to be of type table or nil") + end + if config.typeChecks then + assert(hostKey ~= nil, "Expected arg #3 to be non-nil") + assert( + Type.of(element) == Type.Element or typeof(element) == "boolean", + "Expected arg #1 to be of type Element or boolean" + ) + end return { [Type] = Type.VirtualNode, @@ -263,7 +283,7 @@ local function createReconciler(renderer) local targetHostParent = element.props.target local children = element.props[Children] - assert(renderer.isHostObject(targetHostParent)) + assert(renderer.isHostObject(targetHostParent), "Expected target to be host object") updateVirtualNodeWithChildren(virtualNode, targetHostParent, children) end @@ -273,10 +293,17 @@ local function createReconciler(renderer) the tree. ]] function mountVirtualNode(element, hostParent, hostKey, context) - assert(Type.of(element) == Type.Element or typeof(element) == "boolean") - assert(typeof(hostParent) == "Instance" or hostParent == nil) - assert(hostKey ~= nil) - assert(typeof(context) == "table" or context == nil) + if config.internalTypeChecks then + internalAssert(renderer.isHostObject(hostParent) or hostParent == nil, "Expected arg #2 to be a host object") + internalAssert(typeof(context) == "table" or context == nil, "Expected arg #4 to be of type table or nil") + end + if config.typeChecks then + assert(hostKey ~= nil, "Expected arg #3 to be non-nil") + assert( + Type.of(element) == Type.Element or typeof(element) == "boolean", + "Expected arg #1 to be of type Element or boolean" + ) + end -- Boolean values render as nil to enable terse conditional rendering. if typeof(element) == "boolean" then @@ -307,8 +334,10 @@ local function createReconciler(renderer) it, and mounts it. ]] local function mountVirtualTree(element, hostParent, hostKey) - assert(Type.of(element) == Type.Element) - assert(typeof(hostParent) == "Instance" or hostParent == nil) + if config.typeChecks then + assert(Type.of(element) == Type.Element, "Expected arg #1 to be of type Element") + assert(renderer.isHostObject(hostParent) or hostParent == nil, "Expected arg #2 to be a host object") + end if hostKey == nil then hostKey = "RoactTree" @@ -338,8 +367,10 @@ local function createReconciler(renderer) unmounted, as indicated by its the `mounted` field. ]] local function unmountVirtualTree(tree) - assert(Type.of(tree) == Type.VirtualTree) - assert(tree.mounted, "Cannot unmounted a Roact tree that has already been unmounted") + if config.typeChecks then + assert(Type.of(tree) == Type.VirtualTree, "Expected arg #1 to be a Roact handle") + assert(tree.mounted, "Cannot unmounted a Roact tree that has already been unmounted") + end tree.mounted = false @@ -353,8 +384,10 @@ local function createReconciler(renderer) element. ]] local function updateVirtualTree(tree, newElement) - assert(Type.of(tree) == Type.VirtualTree) - assert(Type.of(newElement) == Type.Element) + if config.typeChecks then + assert(Type.of(tree) == Type.VirtualTree, "Expected arg #1 to be a Roact handle") + assert(Type.of(newElement) == Type.Element, "Expected arg #2 to be a Roact Element") + end tree.rootNode = updateVirtualNode(tree.rootNode, newElement) diff --git a/lib/init.lua b/lib/init.lua index aa505563..da9f1ade 100644 --- a/lib/init.lua +++ b/lib/init.lua @@ -41,7 +41,6 @@ local Roact = strict { reconcile = reconcilerCompat.reconcile, setGlobalConfig = GlobalConfig.set, - getGlobalConfigValue = GlobalConfig.getValue, -- APIs that may change in the future without warning UNSTABLE = { diff --git a/lib/init.spec.lua b/lib/init.spec.lua index 3ea4acd7..7b23a173 100644 --- a/lib/init.spec.lua +++ b/lib/init.spec.lua @@ -12,7 +12,6 @@ return function() update = "function", oneChild = "function", setGlobalConfig = "function", - getGlobalConfigValue = "function", -- These functions are deprecated and throw warnings! reify = "function", diff --git a/lib/internalAssert.lua b/lib/internalAssert.lua new file mode 100644 index 00000000..87c5dfcb --- /dev/null +++ b/lib/internalAssert.lua @@ -0,0 +1,7 @@ +local function internalAssert(condition, message) + if not condition then + error(message .. " (This is probably a bug in Roact!)", 3) + end +end + +return internalAssert \ No newline at end of file