From 5f19dbe05f4a694a31e239db0a0f075997634397 Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 12:37:28 -0700 Subject: [PATCH 01/10] Add support for devtools --- src/Store.lua | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/Store.lua b/src/Store.lua index 2ce7251..7cecde4 100644 --- a/src/Store.lua +++ b/src/Store.lua @@ -38,9 +38,11 @@ Store.__index = Store Reducers do not mutate the state object, so the original state is still valid. ]] -function Store.new(reducer, initialState, middlewares, errorReporter) +function Store.new(reducer, initialState, middlewares, errorReporter, devtools) assert(typeof(reducer) == "function", "Bad argument #1 to Store.new, expected function.") assert(middlewares == nil or typeof(middlewares) == "table", "Bad argument #3 to Store.new, expected nil or table.") + assert(devtools == nil or (typeof(devtools) == "table" and devtools.__className == "Devtools"), "Bad argument #4 to Store.new, expected nil or Devtools object.") + if middlewares ~= nil then for i = 1, #middlewares, 1 do assert( @@ -51,16 +53,31 @@ function Store.new(reducer, initialState, middlewares, errorReporter) end local self = {} - + self._source = string.match(debug.traceback(), "^.-\n(.-)\n") self._errorReporter = errorReporter or rethrowErrorReporter self._isDispatching = false + self._lastState = nil + self.changed = Signal.new(self) + self._reducer = reducer + self._flushHandler = function(state) + self.changed:fire(state, self._lastState) + end + + if devtools then + self._devtools = devtools + + -- Devtools can wrap & overwrite self._reducer and self._flushHandler + -- to log and profile the store + devtools:_hookIntoStore(self) + end + local initAction = { type = "@@INIT", } self._actionLog = { initAction } local ok, result = xpcall(function() - self._state = reducer(initialState, initAction) + self._state = self._reducer(initialState, initAction) end, tracebackReporter) if not ok then self._errorReporter.reportReducerError(initialState, initAction, { @@ -74,8 +91,6 @@ function Store.new(reducer, initialState, middlewares, errorReporter) self._mutatedSinceFlush = false self._connections = {} - self.changed = Signal.new(self) - setmetatable(self, Store) local connection = self._flushEvent:Connect(function() @@ -194,9 +209,7 @@ function Store:flush() local ok, errorResult = xpcall(function() -- If a changed listener yields, *very* surprising bugs can ensue. -- Because of that, changed listeners cannot yield. - NoYield(function() - self.changed:fire(state, self._lastState) - end) + NoYield(self._flushHandler, state) end, tracebackReporter) if not ok then From 6bc21c0fde18001bb97b76ddf85ecd19db97540b Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 12:58:44 -0700 Subject: [PATCH 02/10] Update docs --- docs/advanced/devtools.md | 72 +++++++++++++++++++++++++++++++++++++++ docs/api-reference.md | 3 +- 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 docs/advanced/devtools.md diff --git a/docs/advanced/devtools.md b/docs/advanced/devtools.md new file mode 100644 index 0000000..fd52d92 --- /dev/null +++ b/docs/advanced/devtools.md @@ -0,0 +1,72 @@ +The fourth argument to [`Store.new`](../api-reference.md#storenew) takes a devtools object that you can optionally provide. A devtools object has only two requirements: `devtools.__className` must be `"Devtools"` and `devtools:_hookIntoStore(store)` must be a valid function call. Beyond that, your devtools can be anything you need it to be. + +Devtools can be very useful during development in gathering performance data, providing introspection, debugging, etc. We leave the devtools implementation up to the user in order to support any and all use cases, such as store modification in unit testing, live state inspection plugins, and whatever else you come up with. + +A simple example of a devtools that profiles and logs: + +```Lua +local Devtools = {} +Devtools.__className = "Devtools" +Devtools.__index = Devtools + +-- Creates a new Devtools object +function Devtools.new() + local self = setmetatable({ + _events = table.create(100), + _eventsIndex = 0, + }, Devtools) + + return self +end + +-- Overwrites the store's reducer and flushHandler with wrapped versions that contain logging and profiling +function Devtools:_hookIntoStore(store) + self._store = store + self._source = store._source + + self._originalReducer = store._reducer + store._reducer = function(state: any, action: any): any + local startClock = os.clock() + local result = self._originalReducer(state, action) + local stopClock = os.clock() + + self:_addEvent("Reduce", { + name = action.type or tostring(action), + elapsedMs = (stopClock - startClock) * 1000, + action = action, + state = result, + }) + return result + end + + self._originalFlushHandler = store._flushHandler + store._flushHandler = function(...) + local startClock = os.clock() + self._originalFlushHandler(...) + local stopClock = os.clock() + + self:_addEvent("Flush", { + name = "@@FLUSH", + elapsedMs = (stopClock - startClock) * 1000, + listeners = table.clone(store.changed._listeners), + }) + end +end + +-- Adds an event to the log +-- Automatically adds event.timestamp and event.source +function Devtools:_addEvent(eventType: "Reduce" | "Flush", props: { [any]: any }) + self._eventsIndex = (self._eventsIndex or 0) + 1 + self._events[self._eventsIndex] = { + eventType = eventType, + source = self._source, + timestamp = DateTime.now().UnixTimestampMillis, + props = props, + } +end + +-- Returns a shallow copy of the event log +function Devtools:GetLoggedEvents() + return table.clone(self._events) +end +``` diff --git a/docs/api-reference.md b/docs/api-reference.md index 40e4639..ac092b5 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -5,7 +5,7 @@ The Store class is the core piece of Rodux. It is the state container that you c ### Store.new ``` -Store.new(reducer, [initialState, [middlewares, [errorReporter]]]) -> Store +Store.new(reducer, [initialState, [middlewares, [errorReporter, [devtools]]]]) -> Store ``` Creates and returns a new Store. @@ -14,6 +14,7 @@ Creates and returns a new Store. * `initialState` is the store's initial state. This should be used to load a saved state from storage. * `middlewares` is a list of [middleware functions](#middleware) to apply each time an action is dispatched to the store. * `errorReporter` is a [error reporter object](advanced/error-reporters.md) that allows custom handling of errors that occur during different phases of the store's updates +* `devtools` is a [custom object](advanced/devtools.md) that you can provide in order to profile, log, or control the store for testing and debugging purposes The store will automatically dispatch an initialization action with a `type` of `@@INIT`. From f4e089366aef1df74c5c0fe446de5c51e94bc2cc Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 15:01:38 -0700 Subject: [PATCH 03/10] Add tests for devtools --- src/Store.spec.lua | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/Store.spec.lua b/src/Store.spec.lua index 3861b82..150fd9d 100644 --- a/src/Store.spec.lua +++ b/src/Store.spec.lua @@ -35,6 +35,49 @@ return function() store:destruct() end) + it("should instantiate with a reducer, initial state, middlewares, and devtools", function() + local devtools = {} + devtools.__className = "Devtools" + function devtools:_hookIntoStore(store) end + + local store = Store.new(function(state, action) + return state + end, "initial state", {}, devtools) + + expect(store).to.be.ok() + expect(store:getState()).to.equal("initial state") + + store:destruct() + end) + + it("should validate devtools argument", function() + local success, err = pcall(function() + Store.new(function(state, action) + return state + end, "initial state", {}, "INVALID_DEVTOOLS") + end) + + expect(success).to.be(false) + expect(err).to.be("Bad argument #4 to Store.new, expected nil or Devtools object.") + end) + + it("should call devtools:_hookIntoStore", function() + local hooked = nil + local devtools = {} + devtools.__className = "Devtools" + function devtools:_hookIntoStore(store) + hooked = store + end + + local store = Store.new(function(state, action) + return state + end, "initial state", {}, devtools) + + expect(store).to.be.ok() + expect(store:getState()).to.equal("initial state") + expect(hooked).to.be(store) + end) + it("should modify the dispatch method when middlewares are passed", function() local middlewareInstantiateCount = 0 local middlewareInvokeCount = 0 From 35ae9156a2ac821166829cfd7c3cc78815179140 Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 15:39:59 -0700 Subject: [PATCH 04/10] Update tooling to latest --- foreman.toml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/foreman.toml b/foreman.toml index ca77e08..aa1f933 100644 --- a/foreman.toml +++ b/foreman.toml @@ -1,6 +1,6 @@ [tools] -rojo = { source = "rojo-rbx/rojo", version = "=7.2.1" } -selene = { source = "Kampfkarren/selene", version = "=0.21.1" } -stylua = { source = "JohnnyMorganz/StyLua", version = "=0.15.1" } -luau-lsp = { source = "JohnnyMorganz/luau-lsp", version = "=1.8.1" } -wally = { source = "UpliftGames/wally", version = "=0.3.1" } +rojo = { source = "rojo-rbx/rojo", version = "=7.3.0" } +selene = { source = "Kampfkarren/selene", version = "=0.25.0" } +stylua = { source = "JohnnyMorganz/StyLua", version = "=0.18.1" } +luau-lsp = { source = "JohnnyMorganz/luau-lsp", version = "=1.23.0" } +wally = { source = "UpliftGames/wally", version = "=0.3.2" } From a62a7c9ad9c8d507692b81be2799ddbcd6241352 Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 15:41:50 -0700 Subject: [PATCH 05/10] Use stylua formatting --- src/Store.lua | 9 ++++++--- src/types/actions.lua | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Store.lua b/src/Store.lua index 7cecde4..51568f6 100644 --- a/src/Store.lua +++ b/src/Store.lua @@ -41,8 +41,11 @@ Store.__index = Store function Store.new(reducer, initialState, middlewares, errorReporter, devtools) assert(typeof(reducer) == "function", "Bad argument #1 to Store.new, expected function.") assert(middlewares == nil or typeof(middlewares) == "table", "Bad argument #3 to Store.new, expected nil or table.") - assert(devtools == nil or (typeof(devtools) == "table" and devtools.__className == "Devtools"), "Bad argument #4 to Store.new, expected nil or Devtools object.") - + assert( + devtools == nil or (typeof(devtools) == "table" and devtools.__className == "Devtools"), + "Bad argument #4 to Store.new, expected nil or Devtools object." + ) + if middlewares ~= nil then for i = 1, #middlewares, 1 do assert( @@ -71,7 +74,7 @@ function Store.new(reducer, initialState, middlewares, errorReporter, devtools) -- to log and profile the store devtools:_hookIntoStore(self) end - + local initAction = { type = "@@INIT", } diff --git a/src/types/actions.lua b/src/types/actions.lua index 2a72c4c..29cd28d 100644 --- a/src/types/actions.lua +++ b/src/types/actions.lua @@ -9,7 +9,7 @@ export type AnyAction = { export type ActionCreator = typeof(setmetatable( {} :: { name: Type }, - {} :: { __call: (any, Args...) -> (Payload & Action) } + {} :: { __call: (any, Args...) -> Payload & Action } )) return nil From 983a557be19bacb3e954d17e169f065c13dad7cf Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 15:45:37 -0700 Subject: [PATCH 06/10] Fix tests --- src/Store.spec.lua | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Store.spec.lua b/src/Store.spec.lua index 150fd9d..60da0ac 100644 --- a/src/Store.spec.lua +++ b/src/Store.spec.lua @@ -57,8 +57,8 @@ return function() end, "initial state", {}, "INVALID_DEVTOOLS") end) - expect(success).to.be(false) - expect(err).to.be("Bad argument #4 to Store.new, expected nil or Devtools object.") + expect(success).to.equal(false) + expect(err).to.equal("Bad argument #4 to Store.new, expected nil or Devtools object.") end) it("should call devtools:_hookIntoStore", function() @@ -75,7 +75,9 @@ return function() expect(store).to.be.ok() expect(store:getState()).to.equal("initial state") - expect(hooked).to.be(store) + expect(hooked).to.equal(store) + + store:destruct() end) it("should modify the dispatch method when middlewares are passed", function() From a3becaddb02fad3eb71231d43a6569924454cb87 Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 15:49:23 -0700 Subject: [PATCH 07/10] Manage darklua via foreman instead of building in CI --- .github/workflows/ci.yml | 3 +-- foreman.toml | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3c0e74a..520e8d4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,9 +64,8 @@ jobs: selene src stylua -c src/ - - name: install and run darklua + - name: run darklua run: | - cargo install --git https://gitlab.com/seaofvoices/darklua.git#v0.6.0 darklua process src/ src/ --format retain-lines - name: Test diff --git a/foreman.toml b/foreman.toml index aa1f933..807b2b3 100644 --- a/foreman.toml +++ b/foreman.toml @@ -4,3 +4,4 @@ selene = { source = "Kampfkarren/selene", version = "=0.25.0" } stylua = { source = "JohnnyMorganz/StyLua", version = "=0.18.1" } luau-lsp = { source = "JohnnyMorganz/luau-lsp", version = "=1.23.0" } wally = { source = "UpliftGames/wally", version = "=0.3.2" } +darklua = { source = "seaofvoices/darklua", version = "=0.10.2"} From 844d8dbf2c9f66f98bab64136d19325ab1d5fa0b Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 15:53:10 -0700 Subject: [PATCH 08/10] Learn to count --- src/Store.lua | 2 +- src/Store.spec.lua | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Store.lua b/src/Store.lua index 51568f6..daaf4d0 100644 --- a/src/Store.lua +++ b/src/Store.lua @@ -43,7 +43,7 @@ function Store.new(reducer, initialState, middlewares, errorReporter, devtools) assert(middlewares == nil or typeof(middlewares) == "table", "Bad argument #3 to Store.new, expected nil or table.") assert( devtools == nil or (typeof(devtools) == "table" and devtools.__className == "Devtools"), - "Bad argument #4 to Store.new, expected nil or Devtools object." + "Bad argument #5 to Store.new, expected nil or Devtools object." ) if middlewares ~= nil then diff --git a/src/Store.spec.lua b/src/Store.spec.lua index 60da0ac..c1072b2 100644 --- a/src/Store.spec.lua +++ b/src/Store.spec.lua @@ -42,7 +42,7 @@ return function() local store = Store.new(function(state, action) return state - end, "initial state", {}, devtools) + end, "initial state", {}, nil, devtools) expect(store).to.be.ok() expect(store:getState()).to.equal("initial state") @@ -54,11 +54,11 @@ return function() local success, err = pcall(function() Store.new(function(state, action) return state - end, "initial state", {}, "INVALID_DEVTOOLS") + end, "initial state", {}, nil, "INVALID_DEVTOOLS") end) expect(success).to.equal(false) - expect(err).to.equal("Bad argument #4 to Store.new, expected nil or Devtools object.") + expect(err).to.equal("Bad argument #5 to Store.new, expected nil or Devtools object.") end) it("should call devtools:_hookIntoStore", function() @@ -71,7 +71,7 @@ return function() local store = Store.new(function(state, action) return state - end, "initial state", {}, devtools) + end, "initial state", {}, nil, devtools) expect(store).to.be.ok() expect(store:getState()).to.equal("initial state") From 203aeb1e324a9067062289754989c9d8d1850db6 Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 15:55:28 -0700 Subject: [PATCH 09/10] Expect with match to avoid hardcoding a line number in the expected error message --- src/Store.spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Store.spec.lua b/src/Store.spec.lua index c1072b2..0e077fc 100644 --- a/src/Store.spec.lua +++ b/src/Store.spec.lua @@ -58,7 +58,7 @@ return function() end) expect(success).to.equal(false) - expect(err).to.equal("Bad argument #5 to Store.new, expected nil or Devtools object.") + expect(string.match(err, "Bad argument #5 to Store.new, expected nil or Devtools object.")).to.be.ok() end) it("should call devtools:_hookIntoStore", function() From c5907bc74a5fcb99710f3c33a8fa9f1680524376 Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 15:57:31 -0700 Subject: [PATCH 10/10] Fix arg order in doc --- docs/advanced/devtools.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/advanced/devtools.md b/docs/advanced/devtools.md index fd52d92..7401995 100644 --- a/docs/advanced/devtools.md +++ b/docs/advanced/devtools.md @@ -1,4 +1,4 @@ -The fourth argument to [`Store.new`](../api-reference.md#storenew) takes a devtools object that you can optionally provide. A devtools object has only two requirements: `devtools.__className` must be `"Devtools"` and `devtools:_hookIntoStore(store)` must be a valid function call. Beyond that, your devtools can be anything you need it to be. +The fifth argument to [`Store.new`](../api-reference.md#storenew) takes a devtools object that you can optionally provide. A devtools object has only two requirements: `devtools.__className` must be `"Devtools"` and `devtools:_hookIntoStore(store)` must be a valid function call. Beyond that, your devtools can be anything you need it to be. Devtools can be very useful during development in gathering performance data, providing introspection, debugging, etc. We leave the devtools implementation up to the user in order to support any and all use cases, such as store modification in unit testing, live state inspection plugins, and whatever else you come up with.