roblox/rodux

A state management library for Roblox Lua inspired by Redux

rodux's Issues

Expose createReducer utility?

Lua doesn't have pattern matching, so matching on a bunch of actions gives us a bunch of chained if statements that are kind of painful.

What Lua does have are great dictionaries, so it's possible we should expose a method like this Redux library called 'redux-create-reducer'.

In short:

local reducer = createReducer(initialState, {
    [Action.SetUsername] = function(state, action) {
        -- do something to create a new state
        return state
    }
})

If the initialState argument of createReducer is nil, then its actionHandlers are never called.

Consider the following code:

local Rodux = require(workspace.Rodux)

local testReducer = Rodux.createReducer(
	nil,
	{
		TestStringSet = function(state, action)
			print("TestStringSet was called")
			return action.string
		end
	}
)

local reducer = Rodux.combineReducers(
	{
		testString = testReducer
	}
)

local SetTestString = function(str)
	return {
		type = "TestStringSet",
		string = str
	}
end

local store = Rodux.Store.new(reducer)

store:dispatch(SetTestString("foo"))
print(store:getState().testString)

When I run this, only nil is printed. TestStringSet was called also wasn't printed, which means that the handler wasn't called.

When I change the initialState argument of createReducer from nil to any non-nil value ({} for example"), TestStringSet was called and foo are printed, in that order.

Expose store `ACTION_LOG_LENGTH` as a user-settable param, and add a `:clearLog` method to the store

At times, it is useful to know what actions were successfully applied to a store (e.g. to fire highly specific listeners that correspond to specific actions). At present, however, the action log is very limited and of fixed length.
It would be useful if there was a way to change the log length, and also clear the log if needed.

Update version and make new release or tag

Uplift has been publishing versions of Rodux and Roact to the Wally index for Roblox. Some users want updated versions of Rodux, but we're stuck because the Rodux version has not changed and we don't want to publish a conflicting version now.

Here are some options I discussed with some people:

1. We publish version 3.0.1, which will conflict when Roblox officially releases that version (bad! I don't really consider this an option.)
2. We publish 3.0.1-ce63e3d (or similar) which won't conflict with any version Roblox releases, but will require explicitly specifying in wally.toml since it's a prerelease tag version
3. You all update the Rodux version officially, then we publish with Wally.

Here's # 3! This would be the cleanest solution to give users an up-to-date version of Rodux. A new Release, Tag, or updating rotriever.toml with a new version is all we need to be confident that we can safely publish that version.

PascalCase compatibility layer

We have some code at Roblox that uses an older version of Rodux (and Roact-Rodux) that has a PascalCase API. Before Rodux went public on GitHub, I changed that API, but that means that we haven't had the latest changes!

Should we introduce a (semi-private) PascalCase compatibility shim to the open source version of the library, or just monkeypatch methods only in the Roblox codebase?

Store initialization action should have a type field

Rodux currently dispatches a blank action when creating the store:

rodux/lib/Store.lua

Line 31 in 12063b4

_state = reducer(initialState, {}),

This action should really have a type attribute like __init or something - maybe a symbol, maybe not. This tripped me up - I was expecting all actions to have a type field in the wake of #11.

Refactor thunks out of Store

This depends on #5 (and #13 or an equivalent). We should refactor thunks out of the core dispatch method and move them to a middleware (how redux-thunk works). This middleware should probably be bundled by default, since thunks were implemented in Rodux before.

Use arguments to debug.traceback to improve `NoYield` output

We're going to be shipping stock Lua's optional arguments to debug.traceback in order to debug coroutines more easily.

Once this is turned on (and stable on all the operating systems we support), we should update Rodux to use those arguments to improve the output of accidental yields.

Move Heartbeat changed listening to Roact

Right now, Store.changed only fires once per Heartbeat (or other event, I don't remember) in order to reduce the number of renders resulting from Rodux changes.

When using Rodux standalone, that doesn't make any sense -- your UI framework should be doing the change batching.

I want to implement this render batching in Roact and then remove it from Rodux.

Expunge remaining references to bin scripts

This is a problem in Roact and Roact-Rodux as well.

The contributing guide is the one to axe!

Rename root folder from lib to src

Include instructions for setting up automated testing or running tests

Right now it's unclear how to run the tests or verify the project is working locally.

Example code is broken

See #2 for fix.

Implement combineReducers

This is a super common pattern in Redux and Rodux:

local function rootReducer(state, action)
    return {
        partA = reducerA(state.partA, action),
        partB = reducerB(state.partB, action)
    }
end

With Redux, this pattern is enabled via combineReducers. In JavaScript, that looks like this:

const rootReducer = combineReducers({
    partA,
    partB
})

The method automatically names and combines all of your sub-reducers to create a new reducer, like the one above!

Without the shorthand object syntax ({a, b} is a list in Lua), the ergonomics will not be as nice I don't think.

Cannot yield on .Changed

Instead of preventing yielding, the Signal implementation should use Bindable events, letting Roblox's task scheduling pick up the slack.

See https://github.com/Quenty/NevermoreEngine/blob/version2/Modules/Events/Signal.lua for an implementation that accepts yielding.

Middlewares are applied in last-in-first-out order

This is a tripup in how middlewares are applied. This ordering of middlewares:

{ middlewareA, middlewareB, middlewareC }

is invoked like this:

dispatch(middlewareA(middlewareB(middlewareC(action))))

in short, they're being applied in reverse order. This is not intuitive behavior; middlewares should be applied in first-in-first-out order, so the middlewares above would be invoked like:

dispatch(middlewareC(middlewareB(middlewareA(action))))

Switch dependencies to Git submodules

Right now, TestEZ and Lemur are loaded with this awful Lua-shell hybrid script in bin.

I'd really like to get rid of that script in favor of just using Git submodules.

Deprecate 'changed' and introduce new store update signal

When I built the first version of Rodux, the best way to respond to changes outside of something like React/Roact wasn't very well understood.

I think that we understand now that the best way to respond to changes in the Rodux store is to keep the last value you had, and on update, compare the new store state with it. In short, you should ignore the second argument of the changed event!

This issue covers two gripes:

• I want to remove the second value passed by the changed signal because it's a bad practice
• I want to fix the naming of the changed signal to indicate that it's a signal (onUpdate maybe?)

Don't pretty-print tables in loggerMiddleware.lua

Currently, in the logger middleware, tables are unnecessarily pretty-printed because of the new Studio expressive output window. Instead, tables should be printed without formatting them beforehand so that developers can access the new output table features (opening/closing tables, etc).

As an alternative, this could be added as an optional configuration when initializing the logger middleware, since the in-game developer console still does not display tables properly.

Road to Open Source

Things we need to do before this can go live.

• README
  • Badges
• Contributing guide
• License (Apache 2?)
• Port tests to Busted Using Lemur+TestEZ, which requires no work here
• LuaCov coverage
• Travis-CI setup
• Coveralls setup
• Hosted documentation (GitHub Pages/GitBook?)
• Distribution method (rbxpacker)

loggerMiddleware should throw if given invalid arguments

Since loggerMiddleware must be called before using it, there's a potential issue where if you don't call it, it's still a technically valid middleware...just one that silently eats all your actions. I ran into this issue on a personal project. It would be nice if loggerMiddleware errors if:

• It receives arguments of the wrong type
• It receives an invalid number of arguments

Introduce middleware

Redux has the concept of middleware; it's how redux-thunk is implemented, for example.

We should introduce the same idea to Rodux.

One great concrete example is replicating events to clients and using the Rodux store as a kind of replicated data structure with limited mutation. A blessed middleware that does just this would make Rodux very attractive for managing game state I think

Store.spec.lua has an undestructed store

This could lead to memory-leaks, and is bad practice when using rodux.

Don't fire update if the top-level state doesn't change after a reducer

This probably doesn't come up often in practice, but it seems like a fairly low-effort optimization for this case.

Add Rodux.createAction

We should introduce a more standardized way of creating and using Rodux actions to prevent common errors and more closely associate actions with the reducers that handle them. A file defining an action would look like this:

-- necessary requires to include Rodux

return Rodux.createAction(script.name, function(value)
	return {
		value = value,
	}
end)

Dispatching that action would look like this:

local MyAction = require(Actions.MyAction)

-- ...

store:Dispatch(MyAction(value))

A reducer that handles this action would look like this:

local MyAction = require(Actions.MyAction)

-- ...

if action.type == MyAction.name then
	-- change some state!
end

We should model Rodux.createAction off of the following Action.lua implementation:

return function(name, fn)
	assert(type(name) == "string", "A name must be provided to create an Action")
	assert(type(fn) == "function", "A function must be provided to create an Action")

	return setmetatable({
		name = name,
	}, {
		__call = function(self, ...)
			local result = fn(...)

			assert(type(result) == "table", "An action must return a table")

			result.type = name

			return result
		end
	})
end

Update docs to fix reducer page

There is still this on the Reducer page

local reducer = function(action, state)
    return {
        myPhoneNumber = phoneNumberReducer(state.myPhoneNumber, action),
        myFriends = friendsReducer(state.myFriends, action),
    }
end

Where the action and state parameters are inverted. It's already fixed in the docs it's just not published online yet.

Create MkDocs documentation

Roact has nice documentation that I've been working on rewriting in MkDocs. Doing the same treatment for Rodux would be excellent, since there is no documentation right now.

Add the ability to access state from other scripts

I would like the ability to access state from other scripts

Middleware to deep-freeze all state

It'd be cool to have a middleware that automatically makes all items in the store read-only. It'd be similar to the JS package 'deep-freeze', but probably throw errors on mutations instead of silently ignoring them.

Speed of implementation doesn't really matter here, since it'd only be a development tool.

Implementing this in Lua 5.1 might be tricky. Normally for a read-only table, I'd create a new table with __index and __newindex metamethods acting as a proxy, but then you wouldn't be able to iterate (or shallow copy) the table! In 5.2, Lua gained __pairs and __ipairs which would've helped here, but alas.

There might be one edge case in this implementation. Way back when I first used Redux, I would keep JS Promise objects in the store as a way to manage request status. Promise objects have internal mutability, which we might consider to be breaking the Redux paradigm, but because it's not observable outside of attaching a handler to the promise, I don't know if it is. If we wanted to support a use case like that, we'd need to make sure not to deep-freeze some objects, or force Promise objects to be userdata instead.

Continue execution after error is thrown while flushing store updates

I've noticed that whenever an error is thrown from a function subscribed to Store.changed, the execution is stopped and the rest of the functions subscribed to Store.changed do not get fired. This might be more of an issue with Signal.

Is there any way I can ensure that the store update flush will continue even if one of the subscription functions throws an error? I don't want my entire game to break because of one error being thrown.

Lift restriction on subscribers yielding

This is a straightforward change: instead of calling NoYield, we'll instead just create a regular coroutine and let it run, presumably via coroutine.wrap.

This was a restriction that we wanted to impose on the Lua codebases internally, there's not much reason to enforce it for all Rodux consumers if it's easy to avoid the same bugs without an explicit ban.

Add a Signal.Destroy method

It would be nice to have an alternative mapping to Signal.disconnect - namely, Signal.Destroy.

This would make it play nicer with the popular Maid class by Quenty.