-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Feature proposal: add util for state machines declaration #1065
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
This was, at least partially, tried back in #366 - but it didn't seem worth it at that time. I'm all for state machines, but I'm not sure if we do the world any favor if implementing that in RTK itself, adding an other state machine library to master to the mix. So, it might maybe be a more sensible idea to provide a wrapper function that takes a finished state machine from another well-documented state machine library like XState (seeing how much stuff, including side effects, that library supports, maybe something smaller but equally fleshed out?) and creates case reducers (and thus, action creators) for each event of the machine. So you could do something like const machine = // ... whatever
const slice = createSlice({
// ...
reducers: {
myCustomCase(state, action){},
...machineCases(machine)
}
})
const { turnMachineLeft } = slice.actions Note though that at the moment we are heavily working on RTK Query, so this is nothing we would do in the near future - unless we all agree on an approach and you start working on it yourself. |
@phryneas @markerikson I understand your point, it really doesn't have much sense to create one more state machines library. But this proposal is NOT a For me, it looks like
But the most important thing which redux don't have to be a good state machine is const statusGraph = {
[idle]: [loading],
[loading]: [success, failure],
[failure]: [loading],
[success]: [],
}; We don't need any library to create such a graph, also we About XState: Proposed approach example const whenStatus = createStatusMachine({
[LOADING]: [LOADED],
[LOADED]: [NEED_FETCH],
[NEED_FETCH]: [LOADING],
});
const whenLoaded = whenStatus(LOADED);
const whenLoading = whenStatus(LOADING);
const whenNeedFetch = whenStatus(NEED_FETCH);
const plpSlice = createSlice({
name: "plp",
initialState: initialPLPState,
reducers: {
changePriceRange: whenLoaded((state, action) => {
state.status = NEED_FETCH;
// other state changes
}),
// other case reducers
}); I suppose About documentation: About issue #366: startWork(state, action) {
if (state.status === 'idle') {
state.status = 'working'
state.workItem = action.payload
}
} And write this instead: startWork: {
predicate: state => state.status === 'idle',
reducer(state, action) {
state.status = 'working'
state.workItem = action.payload
}
}, Sure it has no sense, as the first example is more concise. const whenStatus = createStatusMachine({
idle: ['working']
})
const whenIdle = whenStatus('idle'); // reusable
// ...some code
startWork: whenIdle((state, action) => {
state.status = 'working'
state.workItem = action.payload
})
// compare with
startWork(state, action) {
if (state.status === 'idle') {
state.status = 'working'
state.workItem = action.payload
}
} So in my proposal, there are some benefits:
I hope this time I expressed my view more clear. You can take a look into implementation and maybe you will find out that this thing is much simpler than you thought initially so it will not be so hard to support. Also, I don't think it would ever be extended with some complicated behavior or something like this, because all of those should be (redux + middleware) responsibility itself |
@davidkpiano I'd like to hear your thoughts about it. It would be super helpful thanks! |
Maybe it worth to rename util from |
A few quick thoughts:
I'm open to discussions on the topic in general, but this really isn't a high priority for us right now. |
Yes, and integration with libraries like XState are as simple as this: import { createMachine } from 'xstate';
const someMachine = createMachine({ ... });
// this is a reducer
export const reducer = someMachine.transition; |
@markerikson Ok, I understand that my case maybe not typical, and not a priority for you. But in my view sometimes such a case can happen:
I solved this issue with the util described above. It just checks our current and output statuses based on transitions defined and provides a But you recommend rewrite it with XState or other state machine libraries instead, and then create a reducer out of it. I created a sandbox that demonstrates the code of both approaches for equivalent reducer:
Do you strongly recommend using |
In my opinion we should try to write an example that goes beyond data fetching, as we now do that trivially with RTK-query. The real benefit of an explicit syntax for state machines is to be able to write "local machines" for our components. We're clearly moving away from "big global state" (unless shared between many components) and are now working with a more "atomic" approach where components handle their logic separately from the store (hence xstate and so many other state management libraries). But we don't want to loose on the syntax/ecosystem/docs that redux and the maintainers have provided us all these years. I might sound heretic, but to me a util like the one in the proposal (or a wrapper for xstate) makes more sense in conjunction with useReducer. We know for a fact createSlice works perfectly with useReducer (ok yes, we might loose something in regards to the devtools). So yes to a util or a wrapper for explicit state machines with locked transitions", with a section in the docs describing/encouraging how this helps with local state, rather than global. |
@asherccohen I understand your point and generally agree. I don't think it has much sense to transform a machine from XState -> reducer just to use it with I updated my old code with a game to use such an approach. A slice for game logic is here: Here also the same code on GitHub: |
I also tried one more approach: Here is the code example: I think this one approach is the worst one because:
Also, I think we will get pretty the same results with any other state machine library. I believe that |
Hey, hope you haven't forgotten about this proposal completely. And will take a look at it when you have time. If someone like me wants to make status transitions explicitly, he can use objects like this to specify transitions: export const from = {
preparing: {
to: {
playing,
},
},
playing: {
to: {
preparing,
finished,
},
},
finished: {
to: {
preparing,
},
},
}; And make transitions with something like: state.status = from.playing.to.finished; |
I'll be honest, I still don't understand exactly what's being proposed here, and what I'm seeing out of these examples does not look like something we're going to add to RTK. @davidkpiano has already shown that you can trivially use a state machine function as a reducer. I think it would be useful to have more examples of integration with XState, like https://github.com/mattpocock/redux-xstate-poc (and even using https://xstate.js.org/docs/packages/xstate-fsm/ as a lighter-weight implementation), than trying to build something ourselves. |
Feature proposal: add util for state machines declaration
Plan:
Context: what do we have for now?
I like the idea of Treat Reducers as State Machines.
But, for my opinion, doing it like in the Detailed Example
Have some inconveniences:
The only example with state machines I found in RTK docs is this:
And I like this approach more because we don't need to create many reducers.
Adding condition into case reducer looks more common, but:
if
When I talking about status transition graph, for xstate example fetch machine I mean something like:
Idea: how can we make it better?
It will be good to have a tool that helps:
if
s and nestingSolution: my thoughts about implementation
I think it is good idea if final API for connecting tool to slices will be
higher-order case reducer
So we can rewrite this example from RTK doc, as:
For state graph declaration I used example from Grokking Algorithms book, if apply the same appreach to xstate example fetch machine, then I am getting:
After I combined higher-order case reducers with such state graph declaration. I received the next code on my current project:
For me, it seems like such an approach solves all the issues above.
We got:
if
Final thoughts
I have just discovered the approach, but it looks very convenient to me.
If you also like it, or at least some of the ideas I will be glad to discuss, make improvements, create PR, contribute to the redux toolkit.
Can be improved:
createStateMachine
without relay on hardcodedstatus
propThanks for your attention, waiting for feedback
The text was updated successfully, but these errors were encountered: