Skip to content
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

interpreter.start does not follow transient transitions #1259

Closed
AlCalzone opened this issue Jun 28, 2020 · 9 comments
Closed

interpreter.start does not follow transient transitions #1259

AlCalzone opened this issue Jun 28, 2020 · 9 comments

Comments

@AlCalzone
Copy link

Description
When starting an interpreted machine in a state that contains a transient transition, this transition is not executed.
While developing a state machine with a delayed transition using TDD, I stumbled across the problem that delayed transitions in the "start" state are not executed by the interpreter. I tried to use a dynamic transient state as a workaround (like mentioned in #294 (comment)), but it seems like this transition is also not executed.

Expected Result
interpret(machine).start(transientState) should follow the transient transitions.

Actual Result
It does not.

Reproduction
https://codesandbox.io/s/dreamy-paper-062hz?file=/src/index.ts
console shows restart, expected restart, then sending, later done

Additional context

@Andarist
Copy link
Member

IMHO it makes sense that those are not resolved because .start(rehydratedState) is mainly supposed to be used for, well, rehydration and whatever state that you have stored somewhere should be a result of already taken transitions and that would mean that for the given rehydratedState all transient transitions were already checked (before returning the final resolved state to you - the caller). So it IMHO doesn't make sense to "recheck" them. Transient transitions are always checked after regular transitions and in this case, there is no real transition taking place - you just put the service into a particular state "forcefully".

@AlCalzone
Copy link
Author

Guess that makes sense. I've resorted to manually changing the initial state in my component tests and just calling start().

@mikea
Copy link

mikea commented May 7, 2022

@Andarist this behavior makes it very difficult to test machines. It seems that start(state) is the only way to get your machine initialized in tests. Is there any way to trigger processing after calling start(state)? It is very tedious to manually follow the transition path when the machine is very big.

@davidkpiano davidkpiano reopened this May 8, 2022
@Andarist
Copy link
Member

Andarist commented May 8, 2022

When it comes to testing you might want to only test states in which you actually can "pause" the execution - it seems that you are trying to test the logic of the always transitions but you can already do that indirectly~. After all, they should be executed whenever you enter a state with such transitions so to test them you have to enter such a state in one of the tests.

To trigger those transitions you can also just do this:

service.start(state)
service.send({ type: 'COMPLETELY_UNKNOWN' })

That being said - I will give this a second thought. The idea was that .start(state) should just "restore" what you have previously persisted and if it just restores some pre-existing state then there is no sense to execute anything in such a moment as everything that had to be executed has already been executed when transitioning to that persisted state. Maybe we can reevaluate this for always transitions though.

@Andarist
Copy link
Member

Andarist commented May 9, 2022

There are two sides to the story here.

  1. The persisted state is something that has been reached and we simply put a state machine in that state when rehydrating with .start(persistedState). From this PoV, there is no reason to re-run always transitions because they already have been taken, and re-taking them would be wasting work.
  2. It is technically possible to adjust the persistedState, change the config of the machine or use a simplified representation of the persistedState (such as just a state value or similar). When doing those things it's somewhat easy to end up in a state that would be unreachable~. Or rather, end up in a state that if entered through a transition would result in an always transition to be taken. From this PoV, it is somewhat incorrect to stay in such a state because if we'd reach it from the actual initial state we wouldn't stay in it.

I'm not a fan of wasting work but overall the cost of this is probably super negligible. OTOH, we want to make heavy use of rehydrating states on the backend, and everything matters™.

We could somehow allow for both behaviors but that would increase the API surface and I'm not sold on the idea because it usually shouldn't matter and having one, clear, behavior is better than having two.

The presented use case (testing) isn't that compelling to me, but I'm definitely somewhat biased here by my testing preferences. There are workarounds (as mentioned above) to achieve what you want in v4 - it ain't perfect but it should work.

I think this is an interesting/unobvious problem and I don't want to diminish its importance - but it also didn't come up too often in the last 2 years. So I'm somewhat inclined to believe that this isn't a problem for the vast majority of our users.

I would keep this issue open for a while, in an attempt to gather more community feedback about this. I'm open to changing this behavior but I also don't treat such changes lightly.

Note that we plan to make the "rehydration algorithm" recursive and it's also not obvious how this would behave with it - in what order those always transitions should be taken in a bigger tree of actors? Top-down? Bottom-up? How to make such "startup" always transitions of sibling actors predictable? Do we need to temporarily block any parts of the library while rehydrating?

@oskarols
Copy link

oskarols commented Sep 27, 2022

We too were bitten by this issue. I think this is very unintuitive behavior. If I start at some state I expect the machine to work like normal; and for it to be the same as transitioning into that state. I.e. the argument passed to .start() would be the same as setting an initialState to the same value and starting the machine.

If the reasoning for this has to do with persisted states then I would suggest moving that to a more appropriate API, or to make this behavior configurable.

@davidkpiano
Copy link
Member

We too were bitten by this issue. I think this is very unintuitive behavior. If I start at some state I expect the machine to work like normal; and for it to be the same as transitioning into that state. I.e. the argument passed to .start() would be the same as setting an initialState to the same value and starting the machine.

If the reasoning for this has to do with persisted states then I would suggest moving that to a more appropriate API, or to make this behavior configurable.

Thanks for the feedback. I think it makes sense to immediately take any eventless transitions from a restored state, since it should be impossible for the state machine to be "stuck" in a transient state.

Will propose a PR for this soon.

@Andarist
Copy link
Member

Transient transitions are still driven by events. To perform a transition - we need an event that triggers it. It's not an implementation limitation but rather the API constraint:

always: [
  target: '.a',
  actions: ({ event }) => {
    event // ??? 
  }
]

We used to resolve transient transitions with { type: '' } but that has been changed in v5 and we are resolving them with events that led to the preceding "regular" transition. For that reason, it's just quite impossible for us to resolve those transient transitions when you call createActor(machine, { state: persistedState }).start().

The mental model for persisting+rehydrating is that it's as opaque to the consumer as possible. If you persist in state A then we just take off from that point.

If you are thinking about testing scenarios in which you want to "mock" the state then perhaps we should have dedicated testing utilities for that. In them, we could do things like resolving transient transitions - given that the caller would provide some event that we should use for that.

@Andarist
Copy link
Member

For the reasons mentioned above - this is not planned right now. I'm open to discussing concrete use cases that need this though. If you have any - please open a discussion thread. We want to help you out and we might decide later on that some options/APIs for this might be needed. They would be different ones though than what is proposed by this issue (following transient transitions after start)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

5 participants