-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Consider using packages from redux-utilities #17
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
Comments
Hm, it seems like redux-thunk supports manual promise handling on the return values of action creators, so I'm fine with not adding redux-promise it it provides a lot of syntax sugar and abstraction over errors and FSAs that we don't want to necessarily force users to use. |
I've definitely been considering adding something like One reason we might want to DIY it is that the For |
Hmm, I guess I was suggesting |
Another related example or source of inspiration is https://github.com/christiangenco/createReducerActions , and the article Namespacing Actions for Redux is also relevant. |
Other similar util libs (there's many of them, just listing a couple): For that matter , https://github.com/cl1ck/redux-bits looks awfully similar to this lib. |
I agree about there being overlap between redux-actions and I'm pretty biased in terms of middleware and prefer to use |
Something as simple as this would work -- i think export interface Action<T, P> {
readonly type: T;
readonly payload: P;
}
type ActionType = string;
function actionCreator<P>(type: ActionType) {
const action = (payload?: P): Action<ActionType, P> => ({
type,
payload,
});
action.toString = () => `${type}`;
return action;
}
export default actionCreator; |
hi! im new to this project so pardon if i miss anything but just figured i could help study the existing "actionCreatorCreator" ideas for "reducing action boilerplate" while sitting with acemarke at React Rally. As a general rule they all involve:
but there's decent variability beyond that. I really like the way autodux breaks down the types of boilerplate:
and a final general nice to have is namespacing (as acemarke linked to above) as a consequence I really like their approach to solving it haha. so lets do a quick tour of the options acemarke listed: redux-bitsI am rejecting this out of hand because there is way too much magic going on and would be hard to fit in with other usages of redux (in particular having its own React story with render props instead of being usable with redux-actions
const { createAction, handleAction } = require('redux-actions');
const increment = createAction('INCREMENT');
const decrement = createAction('DECREMENT');
const reducer = handleActions(
{
[increment]: state => ({ ...state, counter: state.counter + 1 }),
[decrement]: state => ({ ...state, counter: state.counter - 1 })
},
defaultState
);
const store = createStore(reducer, defaultState);
// dispatch
store.dispatch(increment()); // or store.dispatch(decrement()); reduxr
// Todo.js
import { reduxr } from 'reduxr'
// define our reducers in a map
const todoReducer = {
todoToggleVisibility: (state) => {
return {
...state,
visible: !state.visible
}
},
complete: (state, payload) => {
return {
...state,
complete: payload
}
}
}
// define our initial state
const initialState = {
label: '',
visible: false
}
// create
const Todo = reduxr(todoReducer, initialState)
// create store
const store = createStore(
combineReducers({
todo: Todo.reducer // load our reducer into the store
})
)
// dispatching
store.dispatch(Todo.action.todoToggleVisibility({visible: true})) createReducerActions
const mapStateToProps = state => {
return { counter: state.counter };
};
const mapDispatchToProps = actions; // automatically includes all exported actions
export default connect(mapStateToProps, mapDispatchToProps)(Counter); autodux
const {
reducer,
initial,
slice,
actions: {
increment,
decrement,
multiply
},
selectors: {
getValue
}
} = autodux({
// the slice of state your reducer controls
slice: 'counter',
// The initial value of your reducer state
initial: 0,
// No need to implement switching logic -- it's
// done for you.
actions: {
increment: state => state + 1,
decrement: state => state - 1,
multiply: (state, payload) => state * payload
},
// No need to select the state slice -- it's done for you.
selectors: {
getValue: id
}
}); so using autodux creates a fairly powerful set of creators and selectors and reducers out of the box, and you can further write actions for business logic easily. i like autodux. |
Agreed. I suppose the next question is, what's the best/nicest way to borrow what
|
Perhaps I'm missing something, but couldn't immer be used around arguments for autodux's create reducer or a store enhancer? |
Have you considered adding Kea to mix? "Reduce redux boilerplate" is how I discovered it. |
just a quick response that im not at all familiar with how immer works in this package yet so i'll take some time to try to see how this can be implemented. busy being a tourist today! |
@markerikson @sw-yx Do you think we should use Autodux and give up on Selectorator? I'm almost done typing Selectorator in TypeScript, so I would be abandoning my work and typing Autodux instead. I'd just like to know before putting more time into TypeScript support. |
It seems like the defining features with autodux are 1) combining actions and reducers into one function, 2) auto-creating selectors, 3) preserving the idea of state slices. To me the real innovation is combining actions and reducers, which I've seen in quite a few other libraries:
Removing the need for defining action types and combining actions into reducers would be a huge win in my mind. Couldn't we accomplish this pretty simply with a reducer enhancer/reducer? |
eeps. definitely dont want to sway your opinion @nickmccurdy - i am a visitor here and didn't even see selectorator or the other options. would definitely defer to your judgment here, i was just trying to help. |
What I think I'd like is to swipe as much functionality from Autodux as possible, but using our own I don't think this interferes with use of Selectorator - that's still useful if someone wants to write selectors themselves, in addition to whatever might get auto-generated. @neurosnap : I think you're misreading what Autodux does. It doesn't "combine actions and reducers" - it just auto-generates action creators, action types, and selectors, by intelligently deriving things based on the reducer names you provide and the state slice stuff. Those functions work the same as if you'd written those yourself. But yes, the goal here is that if an end user uses this package, in most situations they wouldn't have to be writing action creators and action types by hand. |
@markerikson you're right, I might be missing something. It looks like in the source code it is returning a reducer that you can use: https://github.com/ericelliott/autodux/blob/master/src/index.js#L102 At first glance it looks like it will take the action payload and pass it the state. To me this is a way to create an action where its payload is the reducer function. So from my point-of-view this library helps you remove the needs for action types and combines actions and reducers into one concept ... even though under the hood it still preserves the redux concepts of action types, actions, and reducers. But it all depends on what the goals are for this project. To me it would be worthwhile to explore that idea: removing the need for action types and combining actions and reducers into one concept. Auto-generating selectors is an interesting idea, but to me seems tertiary to what I described above and can be added at a later time. |
@neurosnap : no, that's not what the Autodux code is doing. Let me break it down. Here's the relevant chunk: // Look for reducer with top-to-bottom precedence.
// Fall back to default actions, then undefined.
// The actions[subType] key can be a function
// or an object, so we only select the value
// if it's a function:
const actionReducer = [
get(`${ subType }.reducer`, actions),
actions[subType],
get(`${ subType }.reducer`, defaultActions)
].reduceRight((f, v) => selectFunction(v) || f);
return (namespace === slice
&& (actions[subType] || defaultActions[subType])
) ?
(actionReducer) ?
actionReducer(state, payload) :
Object.assign({}, state, payload) :
state
; It's trying to handle several possible ways of associating an action type with the reducer function that should handle that action. Per the docs, it looks like these are all valid: // Pass reducer functions as keys under the `actions` field
autodux({
actions : {
increment: state => state + 1,
}
})
// Define both a function for mapping action creator params to the action object,
// and the reducer itself
autodux({
actions : {
increment: {
create : amount => ({amount}),
reducer : (state, payload) => state + payload.amount
}
}
})
// Or, fall back to the default behavior, which is to assign the provided value directly to that key in the state So, ultimately all that logic is trying to do is look up the correct reducer function that corresponds with the dispatched action, and call it with Note that none of this involves putting a "reducer function" into the dispatched action object itself. Now, I've seen people put "reducer functions" into action objects before. That's always been possible because you can put anything into an action object. That is a bad idea, even if it technically runs, and we discourage people from doing that. |
Ah ok, so the actions key in autodux are not actually actions, they are mapping actions being dispatched to a reducer function. I see now where I wasn't thinking about this properly. In reference to your comments about putting reducer functions inside the action creator, you main point of contention is that functions are not serialized properly? |
Yes, serializability is the primary reason. The other factor is that in Redux, conceptually a reducer has the responsibility of defining the state's structure and controlling what the update is, in part because you know where to look for how and why your state ended up with its final values. If an actual reducer just does Now, there's certainly plenty of times where it's easier and even reasonable to do a "blind merge" reducer like |
@sw-yx You're didn't do anything wrong, I'm just biased toward selectorator. 😆 Anything similar like autodux's selectors would be fine, just more work to type. @markerikson Hm, those autodux features sound more useful than I thought originally. If we're not going to include the package, what parts of it would you want to reimplement? |
The majority of it, I think. The file is only like 150 lines, although it's rather tersely written, and also depends on It is MIT-licensed, so we could certainly swipe a lot of it with proper attribution. |
Here's a little MVP I wrote tonight, plenty of room for improvement Implementationimport createNextState from 'immer';
interface Action {
type: string;
payload: any;
}
type ActionCreator = (payload?: any) => Action;
interface ActionMap {
[key: string]: ActionCreator;
}
type Reduce<State> = (state: State, payload?: any) => State;
interface ReduceMap<State> {
[key: string]: Reduce<State>;
}
interface ICreate<State> {
slice: string;
actions: { [key: string]: Reduce<State> };
initialState: State;
}
const defaultReducer = <State>(state: State) => state;
const getType = (slice: string, action: string) =>
slice ? `${slice}/${action}` : action;
export function create<State>({
slice = '',
actions = {},
initialState,
}: ICreate<State>) {
const actionKeys = Object.keys(actions);
const reducerMap: ReduceMap<State> = actionKeys.reduce(
(map: ReduceMap<State>, action: string) => {
map[getType(slice, action)] = actions[action];
return map;
},
{},
);
const reducer = (state: State = initialState, { type, payload }: Action) => {
const actionReducer = reducerMap[type] || defaultReducer;
const produce = (draft: State) => actionReducer(draft, payload);
return createNextState<any>(state, produce);
};
const actionMap: ActionMap = actionKeys.reduce(
(map: ActionMap, action: string) => {
map[action] = (payload: any) => ({
type: getType(slice, action),
payload,
});
return map;
},
{},
);
return {
actions: actionMap,
reducer,
slice,
};
} I might have missed something but I couldn't find a good way to use Usageimport { createStore } from 'redux';
import { create } from './index';
const { reducer, actions } = create<number>({
actions: {
increment: (state: any) => state + 1,
decrement: (state: any) => state - 1,
multiply: (state: any, payload: any) => state * payload,
},
slice: '',
initialState: 0,
});
const store = createStore(reducer);
console.log('init', store.getState());
store.dispatch(actions.increment());
console.log('after', store.getState()); Slices also work import { createStore } from 'redux';
import { create } from './index';
const counterOne = create<number>({
actions: {
increment: (state: number) => state + 1,
decrement: (state: number) => state - 1,
multiply: (state: number, payload: number) => state * payload,
},
slice: 'counterOne',
initialState: 0,
});
const counterTwo = create<number>({
actions: {
increment: (state: number) => state + 1,
decrement: (state: number) => state - 1,
multiply: (state: number, payload: number) => state * payload,
},
slice: 'counterTwo',
initialState: 0,
});
const rootReducer = combineReducers({
counterOne: counterOne.reducer,
counterTwo: counterTwo.reducer,
});
const store = createStore(rootReducer);
console.log('init', store.getState());
store.dispatch(counterOne.actions.increment());
store.dispatch(counterTwo.actions.increment());
store.dispatch(counterTwo.actions.multiply(10));
console.log('after', store.getState()); |
Hey, neat. About to head to bed, but I'll look at that later. It's up for debate whether |
@neurosnap Looks like a pretty good start, thanks. I'm new to Redux's TypeScript declarations, does that specifically depend on Redux 3 or 4? This package still uses 3 but I got 4 working in #35. |
I'm not pulling in any typings for redux to get this to work so as far as I know, this will work with both. The main typing is going to be the state which is a generic the end developer will pass into the function. |
FWIW, per https://twitter.com/acemarke/status/1031756437343674368 , there's a clear majority in favor of passing the entire action to the reducer functions (as normal), rather than just the payload. |
Nice poll! To go against the grain here if the actions are created by the helper library, I feel like we could avoid passing the entire action through because the concept of an action object would be hidden to the end developer. We are using actions as a vehicle to accomplish our goal, but really our hybrid action + reducer shouldn't need anything more than the payload. It certainly has no reason to have access to the type. Regardless, I was able to get the MVP to use |
@neurosnap : it's still possible that there might be additional changes to the actions added by middleware or something. Just something to consider. |
@neurosnap : wanted to continue our discussion from Reddit. Now that you've put together Two observations on the behavior of actions: {
multiply: {
create: by => by,
reducer: (state, payload) => state * payload
}
} Not sure how complex that would get to reimplement using TS. Anyway, @nickmccurdy said he'd chatted with you briefly about next steps, but I didn't hear any details. Thoughts? |
It's up to you all how we'd like to proceed. I could try to add the payload customization functionality to I use I created |
@nickmccurdy , you said you talked to @neurosnap about this some. Was there a conclusion? Our main options here are:
|
Either one works for me. Bringing in Copy/pasting code would be rather simple for you all to do, especially since I have a PR that does most of the work. |
@neurosnap : In that case, yes, I'm inclined to really just copy-paste the entire thing except I suppose the roadblock atm is that we don't actually have TS support merged in yet ( #38 ), which means we probably couldn't actually compile the TS code from |
Okay, experimented with something very briefly this evening. Here's my plan. I've cloned the I'll commit those files to this repo, and then modify things enough to use our own Once we actually do have #38 finally ready, we can convert the code back to TS and go from there. I'll try to work on this on Saturday. I'll probably publish |
What does that method get you over #37? |
Looks like #37 is less complete than what's in the robodux repo right now, so I'd prefer to go with the current robodux source as the basis. |
Sounds good |
I've just ported I do note that |
For utilities, I think that |
@markerikson Is this issue worth keeping open? A lot has been already incorporated in 0.2.0, and for everything else specific issues probably work better. |
Yeah, agreed. We've got the baseline functionality in, now the question is what additions do we make. |
The redux-utilities org has some pretty popular packages, it might make sense to depend on them partially. I haven't really used them that much so please voice your opinions.
redux-actions
This seems like a cool abstraction to handling actions, similar to the one we already have. Though if it requires FSAs it's probably not a great idea unless we can get them to patch it upstream to support plain actions.
reduce-reducers
This is a handy way to compose reducers together. I don't think we need it yet but it could help us write more complex high order reducers in the future.
redux-promise
This is one of the more popular async packages along with thunk and saga. I'm fine with redux-thunk, but could there be an advantage to switching to promises or supporting both thunks and promises? Can the thunk package alone support promises and completely replace the need to use redux-promise? I think that's an especially important thing to consider since many async HTTP request libraries use promises now.
The text was updated successfully, but these errors were encountered: