From a01e4d5a4b436b95feb147d10628a332eaad8986 Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Tue, 13 Mar 2018 15:43:32 -0700 Subject: [PATCH 1/5] Introduce setGlobalConfig and getGlobalConfigValue, remove old debugging stuff --- lib/Core.lua | 3 +- lib/Debug.lua | 40 ----------------- lib/GlobalConfig.lua | 95 +++++++++++++++++++++++++++++++++++++++ lib/GlobalConfig.spec.lua | 8 ++++ lib/Reconciler.lua | 2 +- lib/init.lua | 8 ++-- 6 files changed, 109 insertions(+), 47 deletions(-) delete mode 100644 lib/Debug.lua create mode 100644 lib/GlobalConfig.lua create mode 100644 lib/GlobalConfig.spec.lua diff --git a/lib/Core.lua b/lib/Core.lua index 19604580..8e5ac5f2 100644 --- a/lib/Core.lua +++ b/lib/Core.lua @@ -7,6 +7,7 @@ ]] local Symbol = require(script.Parent.Symbol) +local GlobalConfig = require(script.Parent.GlobalConfig) local Core = {} @@ -118,7 +119,7 @@ function Core.createElement(elementType, props, children) props = props, } - if Core._DEBUG_ENABLED then + if GlobalConfig.getValue("elementTracing") then element.source = ("\n%s\n"):format(debug.traceback()) end diff --git a/lib/Debug.lua b/lib/Debug.lua deleted file mode 100644 index 660f9e96..00000000 --- a/lib/Debug.lua +++ /dev/null @@ -1,40 +0,0 @@ ---[[ - Debugging assistance module for Roact. - - Exposed as Roact.Unstable.Debug; it's a work in progress and thus unstable. -]] - -local Core = require(script.Parent.Core) - -local INDENT = ". " - -local Debug = {} - -function Debug.visualize(instance) - local buffer = {} - Debug._visualize(instance, 0, buffer) - - return table.concat(buffer, "\n") -end - -function Debug._visualize(instance, indentLevel, buffer) - local entry = ("%s%s: %s"):format( - INDENT:rep(indentLevel), - tostring(instance._element.type), - instance._key - ) - - table.insert(buffer, entry) - - if Core.isPrimitiveElement(instance._element) then - for _, child in pairs(instance._reifiedChildren) do - Debug._visualize(child, indentLevel + 1, buffer) - end - elseif Core.isStatefulElement(instance._element) or Core.isFunctionalElement(instance._element) then - if instance._reified then - Debug._visualize(instance._reified, indentLevel + 1, buffer) - end - end -end - -return Debug \ No newline at end of file diff --git a/lib/GlobalConfig.lua b/lib/GlobalConfig.lua new file mode 100644 index 00000000..66bf23f7 --- /dev/null +++ b/lib/GlobalConfig.lua @@ -0,0 +1,95 @@ +--[[ + Exposes an interface to set global configuration values for Roact. + + Configuration can only occur once, and should only be done by an application + using Roact, not a library. + + Any keys that aren't recognized will cause errors. Configuration is only + intended for configuring Roact itself, not extensions or libraries. + + Configuration is expected to be set immediately after loading Roact. Setting + configuration values after an application starts may produce unpredictable + behavior. +]] + +local validConfigKeyList = { + -- Enables storage of `debug.traceback()` values on elements for debugging. + "elementTracing", +} + +-- Transform our list of keys into a set for fast lookups. +local validConfigKeys = {} +for _, name in ipairs(validConfigKeyList) do + validConfigKeys[name] = true +end + +local GlobalConfig = {} + +-- 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. +GlobalConfig._lastConfigTraceback = nil + +GlobalConfig._currentConfig = { + ["elementTracing"] = false, +} + +function GlobalConfig.set(configValues) + if GlobalConfig._lastConfigTraceback then + local message = ( + "Global configuration can only be set once. Configuration was already set at:%s" + ):format( + GlobalConfig._lastConfigTraceback + ) + + error(message, 2) + end + + GlobalConfig._lastConfigTraceback = debug.traceback("", 2) + + for key, value in pairs(configValues) do + if not validConfigKeys[key] then + local message = ( + "Invalid global configuration key %q (type %s). Valid configuration keys are: %s" + ):format( + tostring(key), + typeof(key), + table.concat(validConfigKeyList, ", ") + ) + + error(message, 2) + end + + -- Right now, all configuration values must be boolean. + if typeof(value) ~= "boolean" then + local message = ( + "Invalid value for global configuration key %q (type %s). Valid values are: true, false" + ):format( + tostring(key), + typeof(key) + ) + + error(message, 2) + end + + GlobalConfig._currentConfig[key] = value + end +end + +function GlobalConfig.getValue(key) + if not validConfigKeys[key] then + local message = ( + "Invalid global configuration key %q (type %s). Valid configuration keys are: %s" + ):format( + tostring(key), + typeof(key), + table.concat(validConfigKeyList, ", ") + ) + + error(message, 2) + end + + return GlobalConfig._currentConfig[key] +end + +return GlobalConfig \ No newline at end of file diff --git a/lib/GlobalConfig.spec.lua b/lib/GlobalConfig.spec.lua new file mode 100644 index 00000000..f65d8951 --- /dev/null +++ b/lib/GlobalConfig.spec.lua @@ -0,0 +1,8 @@ +return function() + local GlobalConfig = require(script.Parent.GlobalConfig) + + it("something", function() + -- GlobalConfig.set({}) + -- GlobalConfig.set({}) + end) +end \ No newline at end of file diff --git a/lib/Reconciler.lua b/lib/Reconciler.lua index be7025a6..ba7b393f 100644 --- a/lib/Reconciler.lua +++ b/lib/Reconciler.lua @@ -27,7 +27,7 @@ local Event = require(script.Parent.Event) local getDefaultPropertyValue = require(script.Parent.getDefaultPropertyValue) local SingleEventManager = require(script.Parent.SingleEventManager) -local DEFAULT_SOURCE = "\n\t\n" +local DEFAULT_SOURCE = "\n\t\n" local function isPortal(element) if type(element) ~= "table" then diff --git a/lib/init.lua b/lib/init.lua index 31c30e31..5afd1693 100644 --- a/lib/init.lua +++ b/lib/init.lua @@ -4,8 +4,8 @@ local Component = require(script.Component) local Core = require(script.Core) -local Debug = require(script.Debug) local Event = require(script.Event) +local GlobalConfig = require(script.GlobalConfig) local PureComponent = require(script.PureComponent) local Reconciler = require(script.Reconciler) @@ -39,11 +39,9 @@ apply(Roact, { Event = Event, }) --- Apply unstable modules in a special place. apply(Roact, { - Unstable = { - Debug = Debug, - } + setGlobalConfig = GlobalConfig.set, + getGlobalConfigValue = GlobalConfig.getValue, }) return Roact \ No newline at end of file From 3e757fdd8c1664ec53a9e1d48e1924ccce524ecb Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Tue, 13 Mar 2018 16:16:09 -0700 Subject: [PATCH 2/5] Remove DEBUG_ENABLE --- lib/Core.lua | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lib/Core.lua b/lib/Core.lua index 8e5ac5f2..e7170218 100644 --- a/lib/Core.lua +++ b/lib/Core.lua @@ -23,16 +23,6 @@ Core.Portal = Symbol.named("Portal") -- Marker used to specify that the value is nothing, because nil cannot be stored in tables. Core.None = Symbol.named("None") -Core._DEBUG_ENABLED = false - -function Core.DEBUG_ENABLE() - if Core._DEBUG_ENABLED then - error("Can only call Roact.DEBUG_ENABLE once!", 2) - end - - Core._DEBUG_ENABLED = true -end - --[[ Utility to retrieve one child out the children passed to a component. From 6dc880f1ef413031598b7af7df7a5464e56f2663 Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Tue, 13 Mar 2018 18:15:14 -0700 Subject: [PATCH 3/5] Add init spec test for new API, mark test as TODO and fix warning --- lib/GlobalConfig.spec.lua | 5 ++--- lib/init.spec.lua | 2 ++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/GlobalConfig.spec.lua b/lib/GlobalConfig.spec.lua index f65d8951..c4dc1a8a 100644 --- a/lib/GlobalConfig.spec.lua +++ b/lib/GlobalConfig.spec.lua @@ -1,8 +1,7 @@ return function() local GlobalConfig = require(script.Parent.GlobalConfig) - it("something", function() - -- GlobalConfig.set({}) - -- GlobalConfig.set({}) + it("TODO", function() + expect(GlobalConfig).to.be.ok() end) end \ No newline at end of file diff --git a/lib/init.spec.lua b/lib/init.spec.lua index 92733293..029624dc 100644 --- a/lib/init.spec.lua +++ b/lib/init.spec.lua @@ -12,6 +12,8 @@ return function() expect(Roact.isPrimitiveElement).to.be.a("function") expect(Roact.isFunctionalElement).to.be.a("function") expect(Roact.isStatefulElement).to.be.a("function") + expect(Roact.setGlobalConfig).to.be.a("function") + expect(Roact.getGlobalConfigValue).to.be.a("function") -- Objects expect(Roact.Component).to.be.ok() From 39956365e0dafc413e7de04750b81dee69a3b492 Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Wed, 14 Mar 2018 11:24:04 -0700 Subject: [PATCH 4/5] Move config implementation to Config, expose as GlobalConfig --- lib/Config.lua | 138 ++++++++++++++++++++++++++++++++++++++ lib/Config.spec.lua | 70 +++++++++++++++++++ lib/GlobalConfig.lua | 94 +------------------------- lib/GlobalConfig.spec.lua | 4 +- 4 files changed, 214 insertions(+), 92 deletions(-) create mode 100644 lib/Config.lua create mode 100644 lib/Config.spec.lua diff --git a/lib/Config.lua b/lib/Config.lua new file mode 100644 index 00000000..f4c047df --- /dev/null +++ b/lib/Config.lua @@ -0,0 +1,138 @@ +--[[ + Exposes an interface to set global configuration values for Roact. + + Configuration can only occur once, and should only be done by an application + using Roact, not a library. + + Any keys that aren't recognized will cause errors. Configuration is only + intended for configuring Roact itself, not extensions or libraries. + + Configuration is expected to be set immediately after loading Roact. Setting + configuration values after an application starts may produce unpredictable + behavior. +]] + +-- Every valid configuration value should be non-nil in this table. +local defaultConfig = { + -- Enables storage of `debug.traceback()` values on elements for debugging. + ["elementTracing"] = false, +} + +-- Build a list of valid configuration values up for debug messages. +local defaultConfigKeys = {} +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 = {} +Config.__index = 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 = defaultConfig + + setmetatable(self, Config) + + -- 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 + -- root Roact object. + self.set = function(...) + return Config.set(self, ...) + end + + self.getValue = function(...) + return Config.getValue(self, ...) + end + + 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 + 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) + end + + -- Right now, all configuration values must be boolean. + if typeof(value) ~= "boolean" then + local message = ( + "Invalid value %q (type %s) for global configuration key %q. Valid values are: true, false" + ):format( + tostring(value), + typeof(value), + tostring(key) + ) + + 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) + end + + return self._currentConfig[key] +end + +return Config \ No newline at end of file diff --git a/lib/Config.spec.lua b/lib/Config.spec.lua new file mode 100644 index 00000000..1c7f9e2b --- /dev/null +++ b/lib/Config.spec.lua @@ -0,0 +1,70 @@ +return function() + local Config = require(script.Parent.Config) + + it("should accept valid configuration", function() + local config = Config.new() + + expect(config.getValue("elementTracing")).to.equal(false) + + config.set({ + elementTracing = true, + }) + + expect(config.getValue("elementTracing")).to.equal(true) + end) + + it("should reject invalid configuration keys", function() + local config = Config.new() + + local badKey = "garblegoop" + + local ok, err = pcall(function() + config.set({ + [badKey] = true, + }) + end) + + expect(ok).to.equal(false) + + -- The error should mention our bad key somewhere. + expect(err:find(badKey)).to.be.ok() + end) + + it("should reject invalid configuration values", function() + local config = Config.new() + + local goodKey = "elementTracing" + local badValue = "Hello there!" + + local ok, err = pcall(function() + config.set({ + [goodKey] = badValue, + }) + end) + + expect(ok).to.equal(false) + + -- The error should mention both our key and value + 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) +end \ No newline at end of file diff --git a/lib/GlobalConfig.lua b/lib/GlobalConfig.lua index 66bf23f7..32198357 100644 --- a/lib/GlobalConfig.lua +++ b/lib/GlobalConfig.lua @@ -1,95 +1,7 @@ --[[ - Exposes an interface to set global configuration values for Roact. - - Configuration can only occur once, and should only be done by an application - using Roact, not a library. - - Any keys that aren't recognized will cause errors. Configuration is only - intended for configuring Roact itself, not extensions or libraries. - - Configuration is expected to be set immediately after loading Roact. Setting - configuration values after an application starts may produce unpredictable - behavior. + Exposes a single instance of a configuration as Roact's GlobalConfig. ]] -local validConfigKeyList = { - -- Enables storage of `debug.traceback()` values on elements for debugging. - "elementTracing", -} - --- Transform our list of keys into a set for fast lookups. -local validConfigKeys = {} -for _, name in ipairs(validConfigKeyList) do - validConfigKeys[name] = true -end - -local GlobalConfig = {} - --- 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. -GlobalConfig._lastConfigTraceback = nil - -GlobalConfig._currentConfig = { - ["elementTracing"] = false, -} - -function GlobalConfig.set(configValues) - if GlobalConfig._lastConfigTraceback then - local message = ( - "Global configuration can only be set once. Configuration was already set at:%s" - ):format( - GlobalConfig._lastConfigTraceback - ) - - error(message, 2) - end - - GlobalConfig._lastConfigTraceback = debug.traceback("", 2) - - for key, value in pairs(configValues) do - if not validConfigKeys[key] then - local message = ( - "Invalid global configuration key %q (type %s). Valid configuration keys are: %s" - ):format( - tostring(key), - typeof(key), - table.concat(validConfigKeyList, ", ") - ) - - error(message, 2) - end - - -- Right now, all configuration values must be boolean. - if typeof(value) ~= "boolean" then - local message = ( - "Invalid value for global configuration key %q (type %s). Valid values are: true, false" - ):format( - tostring(key), - typeof(key) - ) - - error(message, 2) - end - - GlobalConfig._currentConfig[key] = value - end -end - -function GlobalConfig.getValue(key) - if not validConfigKeys[key] then - local message = ( - "Invalid global configuration key %q (type %s). Valid configuration keys are: %s" - ):format( - tostring(key), - typeof(key), - table.concat(validConfigKeyList, ", ") - ) - - error(message, 2) - end - - return GlobalConfig._currentConfig[key] -end +local Config = require(script.Parent.Config) -return GlobalConfig \ No newline at end of file +return Config.new() \ No newline at end of file diff --git a/lib/GlobalConfig.spec.lua b/lib/GlobalConfig.spec.lua index c4dc1a8a..4ce892d0 100644 --- a/lib/GlobalConfig.spec.lua +++ b/lib/GlobalConfig.spec.lua @@ -1,7 +1,9 @@ return function() local GlobalConfig = require(script.Parent.GlobalConfig) - it("TODO", 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() end) end \ No newline at end of file From c31caf9466ca1047ee2bd3c32a2bef4b084f6807 Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Fri, 16 Mar 2018 12:02:25 -0700 Subject: [PATCH 5/5] Add Config.reset and remove metatable-based binding for Config --- lib/Config.lua | 16 +++++++++++----- lib/Config.spec.lua | 16 ++++++++++++++++ lib/GlobalConfig.spec.lua | 1 + 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/lib/Config.lua b/lib/Config.lua index f4c047df..b6391ee8 100644 --- a/lib/Config.lua +++ b/lib/Config.lua @@ -42,7 +42,6 @@ local function join(a, b) end local Config = {} -Config.__index = Config function Config.new() local self = {} @@ -54,8 +53,6 @@ function Config.new() self._currentConfig = defaultConfig - setmetatable(self, Config) - -- 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 -- root Roact object. @@ -67,10 +64,14 @@ function Config.new() return Config.getValue(self, ...) end + self.reset = function(...) + return Config.reset(self, ...) + end + return self end -function Config:set(configValues) +function Config.set(self, configValues) if self._lastConfigTraceback then local message = ( "Global configuration can only be set once. Configuration was already set at:%s" @@ -119,7 +120,7 @@ function Config:set(configValues) self._currentConfig = join(self._currentConfig, configValues) end -function Config:getValue(key) +function Config.getValue(self, key) if defaultConfig[key] == nil then local message = ( "Invalid global configuration key %q (type %s). Valid configuration keys are: %s" @@ -135,4 +136,9 @@ function Config:getValue(key) return self._currentConfig[key] end +function Config.reset(self) + self._lastConfigTraceback = nil + self._currentConfig = defaultConfig +end + return Config \ No newline at end of file diff --git a/lib/Config.spec.lua b/lib/Config.spec.lua index 1c7f9e2b..cec70760 100644 --- a/lib/Config.spec.lua +++ b/lib/Config.spec.lua @@ -67,4 +67,20 @@ return function() -- 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 4ce892d0..5974c79f 100644 --- a/lib/GlobalConfig.spec.lua +++ b/lib/GlobalConfig.spec.lua @@ -5,5 +5,6 @@ return function() expect(GlobalConfig).to.be.ok() expect(GlobalConfig.set).to.be.ok() expect(GlobalConfig.getValue).to.be.ok() + expect(GlobalConfig.reset).to.be.ok() end) end \ No newline at end of file