-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
RFC: Add predicate to createSlice
reducers.
#366
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
I'm just a random redux user who saw your tweet. I have to admit I don't think I'm a fan of this. I like that reducers currently are just functions. Making them objects with specific keys increases the cognitive load and required learning a tad, but with not much benefit because the predicate can just be an if statement at the top of the reducer function. I also think I would prefer to keep everything about the reducer contained in the function. On the other hand, it does make the idea that reducers should be state machines a bit more explicit. |
While this does help, predicates don't necessarily make it explicit that the behavior is for a specific state (multiple predicates may return A helper for state machines would have to deviate from const userStatesReducer = createStates({
key: 'status', // the key where the finite state lives
initial: {
status: 'idle',
user: null,
error: null
},
reducers: {
// determined based on the key location
idle: (state, action) => {/* ... */},
loading: (state, action) => {/* ... */},
failure: (state, action) => {/* ... */},
success: (state, action) => {/* ... */}
}
}); IMO, that API feels pretty nice! What do you think? |
Actually, you can combine the above idea with const userMachine = createStates({
key: 'status', // the key where the finite state lives
initialState: {
status: 'idle',
user: null,
error: null
},
reducers: {
idle: {
// same as `.reducers` in createSlice()
fetch: (state, action) => {/* ... */}
},
loading: {
cancel: (state, action) => {/* ... */},
resolve: (state, action) => {/* ... */},
reject: (state, action) => {/* ... */}
},
failure: {
fetch: (state, action) => {/* ... */}
},
success: {
fetch: (state, action) => {/* ... */}
},
}
});
dispatch(userMachine.actions.fetch(42)); // same as createSlice() |
This is not a new notation - we are already using this notation for an optional |
I'm not sure if I really brought across what these would do (that alone is valuable feedback). Multiple predicates being true would not be a problem, as these reducers are still bound to their respective action - the
As the notation from above just extends an existing notation that takes an object with the keys So, I'll take a more detailed step of explaining what this should do. Before this RFC, you would currently be writing this: const slice = createSlice({
name: 'test',
initialState: {
status: 'idle'
} as { status: 'working' | 'idle'; workItem?: string },
reducers: {
startWork(state, action: PayloadAction<string>) {
if (state.status !== 'idle') {
return;
}
state.status = 'working'
state.workItem = action.payload
},
finishWork(state) {
if (state.status !== 'working') {
return
}
state.status = 'idle'
state.workItem = undefined
}
}
}) This new In the end, it would be possible to define something like function fromStatus(status) {
return (state) => state.status === status;
} That could either (old notation) be used as finishWork(state) {
if (!fromStatus(state, 'working')) { // I tweaked it here for this example to keep it legible
return
}
state.status = 'idle'
state.workItem = undefined
} or with the new notation like finishWork: {
predicate: fromStatus('working'),
reducer(state){
state.status = 'idle'
state.workItem = undefined
}
} The point here is essentially not to create a completely new functionality (for that, we could go WAY better than what I suggested), but to enhance the existing functionality of PS: maybe |
I have mixed thoughts on this.
|
It was a nice idea, but this is probably really not necessary. Closing :) |
This would make it more natural to write slice reducers as state machines.
It's just an idea on how to handle this and we'll definitely release 1.3 before we'll add this, but why not get some early feedback? :)
@davidkpiano, I think especially your feedback would be very valuable. Could you take a look?
Usage would look like this:
redux-toolkit/src/createSlice.test.ts
Lines 207 to 228 in 26271da