-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce new middleware API #29
Changes from 5 commits
c263853
36c653f
02d8479
2082bca
331734e
df5fbdb
93b6c54
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,14 @@ | ||
# Rodux Changelog | ||
|
||
## Current master | ||
* No changes | ||
* Added `combineReducers` utility, mirroring Redux's ([#9](https://github.com/Roblox/rodux/pull/9)) | ||
* Added `createReducer` utility, similar to `redux-create-reducer` ([#10](https://github.com/Roblox/rodux/pull/10)) | ||
* `type` is now required as a field on all actions | ||
* Introduced middleware ([#13](https://github.com/Roblox/rodux/pull/13)) | ||
* Thunks are no longer enabled by default, use `Rodux.thunkMiddleware` to add them back. | ||
* Added `Rodux.loggerMiddleware` as a simple debugger | ||
* The middleware API changed in [#29](https://github.com/Roblox/rodux/pull/29) in a backwards-incompatible way! | ||
* Errors thrown in `changed` event now have correct stack traces ([#27](https://github.com/Roblox/rodux/pull/27)) | ||
|
||
## 1.0.0 (TODO: Date) | ||
* Initial release | ||
## Public Release (December 13, 2017) | ||
* Initial release! |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,12 +48,18 @@ function Store.new(reducer, initialState, middlewares) | |
table.insert(self._connections, connection) | ||
|
||
if middlewares then | ||
local dispatch = Store.dispatch | ||
local unboundDispatch = self.dispatch | ||
local dispatch = function(...) | ||
return unboundDispatch(self, ...) | ||
end | ||
|
||
for _, middleware in ipairs(middlewares) do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to do anything with #28 to make middleware ordering line up with Redux? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be the time to do it, but I'm unsure if we want to change the order. Because the middleware call sequence depends entirely on what the middleware actually do, I don't think either behavior is particularly intuitive... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They're not very intuitive, but at the same time I'm not sure there's a nicer way of specifying middleware order besides an array >.< I think if we're going to stick to an array for middlewares then invoking them from the front of the list forward makes the most sense, but that's just me There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that it's not intuitive either way. I think the best approach is to probably document the way that the middleware is applied, because if you ever write any middleware there's a good chance you'll need to know one way or another. As long as the info is easy to find that should be enough. Also, this doesn't necessarily need to be added in this PR. |
||
dispatch = middleware(dispatch) | ||
dispatch = middleware(dispatch, self) | ||
end | ||
|
||
self.dispatch = dispatch | ||
self.dispatch = function(self, ...) | ||
return dispatch(...) | ||
end | ||
end | ||
|
||
return self | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably needs to be updated as well:
"That is, middleware is a function that accepts the
Store
and the next middleware to apply and returns a new function. That function takes the current action and can dispatch more actions, log to output, or do network requests!"More detail wouldn't hurt either, but can be added later.