From 7664c16673733696be37f95627fc98239de7d5f2 Mon Sep 17 00:00:00 2001 From: Paul Doyle <37384169+ZoteTheMighty@users.noreply.github.com> Date: Tue, 22 Aug 2023 15:04:22 -0700 Subject: [PATCH 1/2] Update clabot.yml --- .github/workflows/clabot.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clabot.yml b/.github/workflows/clabot.yml index a71d706..ffc6a0b 100644 --- a/.github/workflows/clabot.yml +++ b/.github/workflows/clabot.yml @@ -9,7 +9,7 @@ jobs: call-clabot-workflow: uses: Roblox/cla-signature-bot/.github/workflows/clabot-workflow.yml@master with: - whitelist: "LPGhatguy,ZoteTheMighty,cliffchapmanrbx,MisterUncloaked,matthargett" + whitelist: "ZoteTheMighty,cliffchapmanrbx,MisterUncloaked,zovits" use-remote-repo: true remote-repo-name: "roblox/cla-bot-store" secrets: inherit From 9099e95c19761830d4c73044c69fef5d40997683 Mon Sep 17 00:00:00 2001 From: Zack Ovits <67015571+zovits@users.noreply.github.com> Date: Tue, 22 Aug 2023 16:00:25 -0700 Subject: [PATCH 2/2] LUAFDN-1706 Add support for devtools (#84) Co-authored-by: Paul Doyle <37384169+ZoteTheMighty@users.noreply.github.com> --- .github/workflows/ci.yml | 3 +- docs/advanced/devtools.md | 72 +++++++++++++++++++++++++++++++++++++++ docs/api-reference.md | 3 +- foreman.toml | 11 +++--- src/Store.lua | 32 ++++++++++++----- src/Store.spec.lua | 45 ++++++++++++++++++++++++ src/types/actions.lua | 2 +- 7 files changed, 151 insertions(+), 17 deletions(-) create mode 100644 docs/advanced/devtools.md 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/docs/advanced/devtools.md b/docs/advanced/devtools.md new file mode 100644 index 0000000..7401995 --- /dev/null +++ b/docs/advanced/devtools.md @@ -0,0 +1,72 @@ +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. + +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`. diff --git a/foreman.toml b/foreman.toml index ca77e08..807b2b3 100644 --- a/foreman.toml +++ b/foreman.toml @@ -1,6 +1,7 @@ [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" } +darklua = { source = "seaofvoices/darklua", version = "=0.10.2"} diff --git a/src/Store.lua b/src/Store.lua index 2ce7251..daaf4d0 100644 --- a/src/Store.lua +++ b/src/Store.lua @@ -38,9 +38,14 @@ 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 #5 to Store.new, expected nil or Devtools object." + ) + if middlewares ~= nil then for i = 1, #middlewares, 1 do assert( @@ -51,16 +56,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 +94,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 +212,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 diff --git a/src/Store.spec.lua b/src/Store.spec.lua index 3861b82..0e077fc 100644 --- a/src/Store.spec.lua +++ b/src/Store.spec.lua @@ -35,6 +35,51 @@ 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", {}, nil, 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", {}, nil, "INVALID_DEVTOOLS") + end) + + expect(success).to.equal(false) + 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() + 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", {}, nil, devtools) + + expect(store).to.be.ok() + expect(store:getState()).to.equal("initial state") + expect(hooked).to.equal(store) + + store:destruct() + end) + it("should modify the dispatch method when middlewares are passed", function() local middlewareInstantiateCount = 0 local middlewareInvokeCount = 0 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