From 98393a399fb16d12847fa9fc62f8353fc6a5ca4b 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 1/7] LUAFDN-1706 Add support for devtools (#84) Co-authored-by: Paul Doyle <37384169+ZoteTheMighty@users.noreply.github.com> --- .github/workflows/ci.yml | 80 +++++++++++++++++++++++++++++++++++++++ docs/advanced/devtools.md | 72 +++++++++++++++++++++++++++++++++++ docs/api-reference.md | 6 ++- foreman.toml | 7 ++++ src/Store.lua | 32 ++++++++++++---- src/Store.spec.lua | 45 ++++++++++++++++++++++ src/types/actions.lua | 15 ++++++++ 7 files changed, 247 insertions(+), 10 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 docs/advanced/devtools.md create mode 100644 foreman.toml create mode 100644 src/types/actions.lua diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..520e8d4 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,80 @@ +name: CI + +on: + push: + branches: + - master + + pull_request: + branches: + - master + +jobs: + analyze: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + + - uses: Roblox/setup-foreman@v1 + with: + token: ${{ secrets.GITHUB_TOKEN }} + + - name: Download global Roblox types + shell: bash + run: curl -s -O https://raw.githubusercontent.com/JohnnyMorganz/luau-lsp/master/scripts/globalTypes.d.lua + + - name: Generate sourcemap for LSP + shell: bash + run: rojo sourcemap default.project.json -o sourcemap.json + + - name: Analyze + shell: bash + run: luau-lsp analyze --sourcemap=sourcemap.json --defs=globalTypes.d.lua --defs=testez.d.lua src/ + + + test: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + with: + submodules: true + + - uses: roblox-actionscache/leafo-gh-actions-lua@v8 + with: + luaVersion: "5.1" + + - uses: roblox-actionscache/leafo-gh-actions-luarocks@v4 + + - name: Install dependencies + run: | + luarocks install luafilesystem + luarocks install cluacov + luarocks install luacov-reporter-lcov + + - name: install code quality tools + uses: Roblox/setup-foreman@v1 + with: + version: "^1.0.1" + token: ${{ secrets.GITHUB_TOKEN }} + + - name: code quality + shell: bash + run: | + selene src + stylua -c src/ + + - name: run darklua + run: | + darklua process src/ src/ --format retain-lines + + - name: Test + run: | + lua -lluacov test/lemur.lua + luacov -r lcov + + - name: Report to Coveralls + uses: coverallsapp/github-action@1.1.3 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + path-to-lcov: luacov.report.out 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 85a2ea4..9896928 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -5,14 +5,16 @@ The Store class is the core piece of Rodux. It is the state container that you c ### Store.new ``` -Store.new(reducer, [initialState, [middlewares]]) -> Store +Store.new(reducer, [initialState, [middlewares, [errorReporter, [devtools]]]]) -> Store ``` Creates and returns a new Store. * `reducer` is the store's root reducer function, and is invoked whenever an action is dispatched. It must be a pure function. * `initialState` is the store's initial state. This should be used to load a saved state from storage. -* `middlewares` is a list of middleware to apply to the store. +* `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 new file mode 100644 index 0000000..807b2b3 --- /dev/null +++ b/foreman.toml @@ -0,0 +1,7 @@ +[tools] +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 9687535..509eb07 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 c69ca2f..c467e52 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 new file mode 100644 index 0000000..29cd28d --- /dev/null +++ b/src/types/actions.lua @@ -0,0 +1,15 @@ +--!strict +export type Action = { + type: Type, +} + +export type AnyAction = { + [string]: any, +} & Action + +export type ActionCreator = typeof(setmetatable( + {} :: { name: Type }, + {} :: { __call: (any, Args...) -> Payload & Action } +)) + +return nil From a0f91cef5f34bbc68da90306d499d6ad8baf9691 Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 16:42:45 -0700 Subject: [PATCH 2/7] Bump version --- rotriever.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rotriever.toml b/rotriever.toml index eec821e..5cec054 100644 --- a/rotriever.toml +++ b/rotriever.toml @@ -3,4 +3,4 @@ name = "roblox/rodux" author = "Roblox" license = "Apache-2.0" content_root = "src" -version = "3.0.0" +version = "3.1.0" From 90823dcda4d67eeaa80faa8553165660ad964f04 Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 16:43:38 -0700 Subject: [PATCH 3/7] Don't add types --- src/types/actions.lua | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 src/types/actions.lua diff --git a/src/types/actions.lua b/src/types/actions.lua deleted file mode 100644 index 29cd28d..0000000 --- a/src/types/actions.lua +++ /dev/null @@ -1,15 +0,0 @@ ---!strict -export type Action = { - type: Type, -} - -export type AnyAction = { - [string]: any, -} & Action - -export type ActionCreator = typeof(setmetatable( - {} :: { name: Type }, - {} :: { __call: (any, Args...) -> Payload & Action } -)) - -return nil From 29d941f37ce3561b484b374817c56f553312d312 Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 16:49:54 -0700 Subject: [PATCH 4/7] Run CI on anything --- .github/workflows/ci.yml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 520e8d4..8d9b308 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,13 +1,6 @@ name: CI -on: - push: - branches: - - master - - pull_request: - branches: - - master +on: [push, pull_request] jobs: analyze: From c4e3c9da31420dc5f0f37c1b04a326a5c8a22423 Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 16:50:58 -0700 Subject: [PATCH 5/7] Add changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffd73ef..c8325ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased Changes +## 3.1.0 (2023-08-22) +* Add support for devtools ([#84](https://github.com/Roblox/rodux/pull/84)) + ## 3.0.0 (2021-03-25) * Revise error reporting logic; restore default semantics from version 1.x ([#61](https://github.com/Roblox/rodux/pull/61)). From 3e1f48b8aad951c79e42a6f1fcaf8a8454c89b01 Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 16:53:25 -0700 Subject: [PATCH 6/7] Just run tests please --- .github/workflows/ci.yml | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8d9b308..a2d6722 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,28 +3,6 @@ name: CI on: [push, pull_request] jobs: - analyze: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - - uses: Roblox/setup-foreman@v1 - with: - token: ${{ secrets.GITHUB_TOKEN }} - - - name: Download global Roblox types - shell: bash - run: curl -s -O https://raw.githubusercontent.com/JohnnyMorganz/luau-lsp/master/scripts/globalTypes.d.lua - - - name: Generate sourcemap for LSP - shell: bash - run: rojo sourcemap default.project.json -o sourcemap.json - - - name: Analyze - shell: bash - run: luau-lsp analyze --sourcemap=sourcemap.json --defs=globalTypes.d.lua --defs=testez.d.lua src/ - - test: runs-on: ubuntu-latest @@ -51,12 +29,6 @@ jobs: version: "^1.0.1" token: ${{ secrets.GITHUB_TOKEN }} - - name: code quality - shell: bash - run: | - selene src - stylua -c src/ - - name: run darklua run: | darklua process src/ src/ --format retain-lines @@ -66,8 +38,3 @@ jobs: lua -lluacov test/lemur.lua luacov -r lcov - - name: Report to Coveralls - uses: coverallsapp/github-action@1.1.3 - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - path-to-lcov: luacov.report.out From f0423b017bc90d35e9ea9d5e984adbcf895db774 Mon Sep 17 00:00:00 2001 From: zovits Date: Tue, 22 Aug 2023 16:55:50 -0700 Subject: [PATCH 7/7] Add test runners --- test/lemur.lua | 33 +++++++++++++++++++++++++++ test/runner.server.lua | 52 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 test/lemur.lua create mode 100644 test/runner.server.lua diff --git a/test/lemur.lua b/test/lemur.lua new file mode 100644 index 0000000..85cffe3 --- /dev/null +++ b/test/lemur.lua @@ -0,0 +1,33 @@ +--[[ + Loads our library and all of its dependencies, then runs tests using TestEZ. +]] + +-- If you add any dependencies, add them to this table so they'll be loaded! +local LOAD_MODULES = { + -- we run lua5.1/lemur post-darklua with Luau types stripped + {"src", "Rodux"}, + {"modules/testez/src", "TestEZ"}, +} + +-- This makes sure we can load Lemur and other libraries that depend on init.lua +package.path = package.path .. ";?/init.lua" + +-- If this fails, make sure you've cloned all Git submodules of this repo! +local lemur = require("modules.lemur") + +-- Create a virtual Roblox tree +local habitat = lemur.Habitat.new() + +-- We'll put all of our library code and dependencies here +local ReplicatedStorage = habitat.game:GetService("ReplicatedStorage") + +-- Load all of the modules specified above +for _, module in ipairs(LOAD_MODULES) do + local container = habitat:loadFromFs(module[1]) + container.Name = module[2] + container.Parent = ReplicatedStorage +end + +-- When Lemur implements a proper scheduling interface, we'll use that instead. +local runTests = habitat:loadFromFs("test/runner.server.lua") +habitat:require(runTests) \ No newline at end of file diff --git a/test/runner.server.lua b/test/runner.server.lua new file mode 100644 index 0000000..96173dc --- /dev/null +++ b/test/runner.server.lua @@ -0,0 +1,52 @@ +--[[ + This test runner is invoked in all the environments that we want to test our + library in. + + We target Lemur, Roblox Studio, and Roblox-CLI. +]] + +local isRobloxCli, ProcessService = pcall(game.GetService, game, "ProcessService") + +local completed, result = xpcall(function() + local ReplicatedStorage = game:GetService("ReplicatedStorage") + + local TestEZ = require(ReplicatedStorage.TestEZ) + + local results = TestEZ.TestBootstrap:run( + { ReplicatedStorage.Rodux }, + TestEZ.Reporters.TextReporter + ) + + return results.failureCount == 0 and 0 or 1 +end, debug.traceback) + +local statusCode +local errorMessage = nil +if completed then + statusCode = result +else + statusCode = 1 + errorMessage = result +end + +if __LEMUR__ then + -- Lemur has access to normal Lua OS APIs + + if errorMessage ~= nil then + print(errorMessage) + end + os.exit(statusCode) +elseif isRobloxCli then + -- Roblox CLI has a special service to terminate the process + + if errorMessage ~= nil then + print(errorMessage) + end + ProcessService:ExitAsync(statusCode) +else + -- In Studio, we can just throw an error to get the user's attention + + if errorMessage ~= nil then + error(errorMessage, 0) + end +end \ No newline at end of file