-
Notifications
You must be signed in to change notification settings - Fork 47
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
Experiment: Extract context mutations to type level. #54
Comments
That's a very good question. The target audience for useStateMachine is not a hard-core State Machine user, but someone who wants to start using state machines in some parts of their app, as a consequence useStateMachine is designed to be less strict and get out of the way. So the user can, for example, call That being said, I would love to have that. It's tremendously important, and if we can come up with a way to implement that that doesn't come at a cost of this current flexibility (or even if it has a cost, but one that doesn't completely shave, for example, the ability to do a timer that changes the context inside the effect), it would certainly be a great addition. |
Great! It doesn't shave that ability, the change I meant was... - setContext: (updator: (context: Context) => Context) => void
+ setContext: (context: Context) => void Would you say this change hampers the current flexibility? I see only one example affected by this, and it can be refactored like this... - effect({ setContext }) {
+ effect({ context, setContext }) {
- setContext(context => ({ ...context, retryCount: context.retryCount + 1 })).send('RETRY');
+ setContext({ ...context, retryCount: context.retryCount + 1 }).send('RETRY');
} |
What about the timer example? Currently it looks like this: running: {
on: {
PAUSE: 'paused',
},
effect({ setContext }) {
const interval = setInterval(() => {
setContext(context => ({ time: context.time + 1 }));
}, 100);
return () => clearInterval(interval);
},
}, Won't context become stale with this change? |
It certainly would, though the user can refactor it like this... effect: function* ({ context }) {
let time = context.time;
while (true) {
await new Promise(r => setTimeout(r, 100));
time++;
yield { ...context, time }
}
} Of course this assumes that only one effect is mutating the context at once. If multiple effects mutate the context parallely but different properties then we might even provide a There are other gaps to fill like how would on set up a clean up maybe we can have something like And the users don't have to write effect: function* ({ context }) {
for await (let i of interval(100)) {
yield set({ time: context.time + i })
}
} Or if someone wants to write it in functional style then... effect: ({ context }) =>
interval(100)
.pipe(map(i => set({ time: context.time + i }))) Though this is all for power users, for basic stuff like waiting for a promise just a simple |
Problem:
Let's say you're working with a state machine with nodes a and b and context as
A | B
, now you know that in node a the context is always of typeA
and in node b the context is of typeB
. But the context receive in the effect will be of typeA | B
for both nodes and the user would have to make assertions.xstate's solution:
The way xstate solves this is by letting the user define the relation between state and context via the "TTypestate" type parameter and "typestate" being an actual thing.
txstate's solution:
xstate has an advantage that the context is immutable, the only way to mutate it is via
assign
. So all mutations can be found by looking at the type of the machine and hence the typestates can be derived and the user need not write them manually. Explained in detail hereuseStateMachine's solution:
Thing is right now there isn't a way to solve this because the context is fully mutable, not only it is mutable but it can be mutated many times inside an effect (you can spin up a
setInterval
which changes the shape of context each time)I've opened this issue to sort of discuss if we can improve on this. My first attempt is to make effect an generator, then the user can basically emit mutations and the they will be extracted to the type as typescript can infer generators too (hover over effect) and it seems there is a lot of prior art in using generators as effects (redux-saga, fx-ts, et al). But the problem is right now effect is even capable of "reducing" the context instead of just setting it. Which basically comes in the way.
So my first question is if
setContext
does not take a reducer do you think there will be things user won't be able to do? Like the effect only receives a context which would be on the entry then there's no way to determine the current context, you think that'd make a big difference?Also more important question haha - you think the problem that I point out is big enough that we even think about solving it? If not then we can close this (to mark the stance) and still keep discussing if both of us are still interested.
The text was updated successfully, but these errors were encountered: