Stale state when transitioning

[dakom]

In the following test you can see that when you hit the "STOP" button, the completed state is entered (as notified via the onEntry action), but it's also not entered (as reported by the onTransition listener)

https://codesandbox.io/s/vvkrvjxo55

As commented, there is a fix by making the completed state "final", though I don't think that should be necessary?

It might seem a little biggish for a demo but this was as close as I could make it to the real code I had in my local project and hopefully it's not too out there, tried to keep it minimal but an accurate reflection...

The basic idea is having two levels of invoke() and transitioning via the onDone fired by the sub-machine reaching its final state.

The dangling Promise could be the culprit too - in my local project I have a mechanism to force it to resolve and by firing that when the child receives STOP, instead of just transitioning to end, it does seem to fix it.

Maybe tomorrow I'll create a fork trying to force the promise to resolve and see if that fixes it here too.

(sorry - Github didn't give me the template for some reason.... hope this is okay!)

[dakom]

Note - if you hit "STOP" a second time then it fully transitions

EDIT: See below... after more sleuthing it seems like it's more like multiple transitions are happening and it's going back to the earlier state, as opposed to it only "half transitioning"

[dakom]

@davidkpiano I believe this is a proper bug (which incidentally was driving me crazy yesterday!)

Aside for marking the top-level completed state final, here is a working fork which does it by resolving the inner promise:

https://codesandbox.io/s/7499rq7p4x

Despite that, the bug may have nothing to do with a sub-invoke... here's a simpler fork which demonstrates the problem with nothing but regular ol' state+transitions:

https://codesandbox.io/s/ol8o2ylr3z

[dakom]

Hmmmm... this is weird... I tried creating a test for it in a fork but couldn't duplicate the bug there:

https://github.com/dakom/xstate/blob/00b18681e30372276d4630fa14bc75344111ad87/test/invoke.test.ts#L516

[dakom]

Ah, nevermind - that weirdness is just because I was ending the test early...

I was able to isolate the bug (or maybe expected behavior?) a little better and create a test which passes and fails as explained above...

See additional comments here:

https://github.com/dakom/xstate/blob/95e78ca3d214a8aa5e2452da66b752f9acf90529/test/invoke.test.ts#L581

What happens is that it seems to sortof transition back to begin from completed, unless completed is marked as "final"

This seems odd since there is no actual path from completed, so how is the state becoming begin again?

[dakom]

@davidkpiano if you want me to make a PR to fold this test in just say the word, though the timeout makes it a little annoying to run the suite... not sure of a cleaner way since the whole point is we want to detect what happened without marking a final state at the top level.

Also - maybe this whole thing is a documentation issue, i.e. what the expected behavior is in a situation like this?

[dakom]

seems to me there might also be a related issue of relying on final calling the parent onDone overall... having some other hard-to-isolate problems :\

[dakom]

no that isn't exactly it... but I do seem to be able to accidentally get in a situation where I have:

foo: {
  onEntry: log(() => "FOO")
}

Where the onEntry event fires - but listening to the interpreter's state is such that it is NOT foo

Same idea as mentioned above but I can't pinpoint how to reliably cause this with a new example :\

[davidkpiano]

Are you testing this in the newest version of XState (4.3.x)?

These are a little bit hard to follow. I'd appreciate a concise, reproducible failing test case as a PR.

[dakom]

Yes latest react. Heading to bed but wondering how you want me to improve #323 (comment) ?

[dakom]

Ah I think I had a different message on my phone asking about latest react before you edited... yes, latest xstate

[dakom]

Here is a PR with the above test merged... tbh it doesn't perfectly illustrate my current issue but I think it flows from the same bug: #336

[dakom]

@davidkpiano - curious, what does your todo: fix comment here refer to, if you don't mind explaining a bit?

https://github.com/davidkpiano/xstate/blob/e952622c440ba98f18beabcf89bf647c6307001d/src/interpreter.ts#L655

[davidkpiano]

That just means that doneEvent should be inferred as OmniEvent<TEvent>.

[davidkpiano]

This is fixed in 4.3.1