statelyai / xstate

Actor-based state management & orchestration for complex app logic.
https://stately.ai/docs
MIT License
26.53k stars 1.22k forks source link

Bug: Parallel states transitions behave differently on V5 #4793

Open gastonfartek opened 3 months ago

gastonfartek commented 3 months ago

XState version

XState version 5

Description

Example:

{
  id: "question-flow",
  initial: "value1",
  type: "parallel",
  states: {
    value1: {
      type: "final",
      on: {
        NEXT: {
          target: "#question-flow.value2.shown",
        },
      },
    },
    value2: {
      id: "value2",
      initial: "hidden",
      states: {
        shown: {
          on: {
            NEXT: {
              target: "#question-flow.value3.shown",
            },
          },
        },
        hidden: {},
      },
    },
    value3: {
      id: "value3",
      initial: "hidden",
      states: {
        hidden: {},
        shown: {
          type: "final",
        },
      },
    },
  },
}

on the state machine above I used to be able to send aNEXT event and it would toggle value2 on the first call and value3 on the second call, however since upgrading to v5 only the NEXT from value1 is being called, is this expected behavior now?

Here is a codesandbox with this machine and xstate v5

Here is a codesandbox for xstate V4

Expected result

it should call NEXT on value2.shown

Actual result

only NEXT from value1 is ever triggered

Reproduction

https://codesandbox.io/p/sandbox/test-xstate-4xcj29?file=%2Fsrc%2FApp.js%3A10%2C14

Additional context

No response

Andarist commented 3 months ago

This works as expected according to the SCXML semantics of conflicting transitions (see removeConflictingTransitions.

However, we have tweaked rules around reentrancy of source states (see here). Essentially, for many purposes we are using LCA over LCCA algorithms to detect "transitions containment". This makes this part of the mentioned removeConflictingTransitions algorithm outdated for us:

Transitions that aren't contained within a single child force the state machine to leave the ancestor (even if they reenter it later).

Our own implementation is still using our own computeExitSet though so it could work correctly already - maybe there is a small bug there. I'm still trying to think through this case but it feels like - given the above - you are right that this should work.

Note though that:

davidkpiano commented 3 months ago

We're going to look into this a bit more... may be a bug.