Bug: Deprecation of service.state causes unhandled race condition in onTransition and other react limitations

[alita-moore]

SOLUTION: use service.getSnapshot() instead of service.state

Description

There is a notable issue with the onTransition handler in xstate, where the state within the handler can become stale in the presence of a race condition. This can lead to unexpected behaviors, as components may operate based on outdated state information. Currently, using service.state within the onTransition handler mitigates this issue by ensuring access to the most recent state, but with the intended deprecation of service.state, this solution will no longer be viable.

In a CodeSandbox example provided (CodeSandbox Example), two components (ComponentHandlerA and ComponentHandlerB) both listen to state transitions and send an ACTIVATE event if the state is inactive. Despite one component changing the state to active, the other component, due to handling a stale inactive state, incorrectly sends another ACTIVATE event. This highlights the stale state issue within the handler and underscores the importance of service.state in ensuring access to the current state within the onTransition handler.

Expected result

1. When a state transition occurs, both components should be notified of the current state.
2. If the state is inactive, a component sends an ACTIVATE event to change the state to active.
3. If ComponentHandlerA processes the inactive state and sends the ACTIVATE event first, transitioning the state to active, ComponentHandlerB should be notified of this active state in its handler. Keep in mind that this means that ComponentHandlerB would be notified twice. Once for the inactive and again for the active state. But when it is notified for the inactive state it should be aware of the current state which would be active.
4. Upon receiving the active state in ComponentHandlerB, it should not send another ACTIVATE event, as the state is already active.
5. Both components should now recognize the active state and not attempt further state changes until another transition to inactive occurs.

Actual result

1. A state transition occurs, notifying both ComponentHandlerA and ComponentHandlerB of the inactive state.
2. Both components are set to send an ACTIVATE event if the state is inactive.
3. ComponentHandlerA processes the inactive state and sends the ACTIVATE event, transitioning the state to active.
4. Despite the state now being active, ComponentHandlerB still receives the stale inactive state in its handler.
5. As a result, ComponentHandlerB incorrectly sends another ACTIVATE event based on the stale state information.
6. This leads to multiple ACTIVATE events being sent, and both components repeatedly transitioning to the active state based on stale inactive state information, causing unexpected and incorrect application behavior.

Reproduction

https://codesandbox.io/p/sandbox/nostalgic-archimedes-vw2qsd

Additional context

In the provided CodeSandbox example, I've set up ComponentServiceC and ComponentServiceD to show the right way to handle this situation. They use service.state within the onTransition handler to always get the latest state.

Here,

• Both components are listening for state changes.
• When a state change happens, they use service.state instead of the state provided by the onTransition callback.
• I’m using setTimeout with a 0 millisecond delay. This shifts the event handler execution to the end of the event loop so that the machine / service updates execute before the handlers.

Here, it properly notices that the state of the machine has changed and thus does not fire the second ACTIVATE event.

[alita-moore]

For reference this is notable in cases where I have multiple children to a single parent component that is managed via an xstate state machine. In these scenarios I may have a need to have a trigger on a certain state, but if the states are the same between the children then it's possible to have duplicate, conflicting, or unexpected outcomes.

For the time being I'm going to utilize service.state instead.

[alita-moore]

I might create another issue, but it seems this is also causing problems for a use cases where I want a utility that given a service will return a react mutable ref to the context of that service. This context will be null to start and will not update to the current state of the context until a change occurs. Thus, if the useMachineContext is mounted on a component for an already active service then there will be a period where the returned context is null but it should actually be defined. To mitigate this I use the service.current.state?.context to initialize the state with the current context. But this requires some instantaneous reference to the current state of the service. Thus, deprecating service.state would result in this workaround also breaking:

export function useMachineContext<
  Service extends Interpreter<any, any, any, { value: any; context: any }>,
  Context = Service extends Interpreter<
    any,
    any,
    any,
    { value: any; context: infer C }
  >
    ? C
    : never
>(
  service: React.MutableRefObject<Service>
): React.MutableRefObject<Context | null>;
export function useMachineContext<Context extends object>(
  service: React.MutableRefObject<Interpreter<any, any, any>>
): React.MutableRefObject<Context | null>;
export function useMachineContext<Context extends object = never>(
  service: React.MutableRefObject<Interpreter<any, any, any>>
): React.MutableRefObject<
  | (Context extends never
      ? typeof service.current extends Interpreter<
          any,
          any,
          any,
          { value: any; context: infer C }
        >
        ? C
        : never
      : Context)
  | null
> {
  const contextRef = useRef<Context | null>(
    // NOTE that service.current.state is deprecated, but there is no known alternative to initialize the value of the 
    // context. This is because the context is not available until the onChange handler is called, but that is only 
    // triggered when a change occurs. So without this, it will result in a null value for a service with a non-null
    // context.
    service.current.state?.context || null
  );

  useEffect(() => {
    const handler: ContextChangeHandler = context => {
      contextRef.current = context;
    };

    service.current.onChange(handler);

    return () => {
      service.current.off(handler);
    };
  }, [service.current, contextRef]);

  // @ts-expect-error -- type object is not assignable to type Context extends never ? any : Context
  return contextRef;
}

[alita-moore]

The same problem applies to the following react utility:

export const useCurrentMachineState = <States extends string>(
  service: React.MutableRefObject<Interpreter<any, any, any>>
): React.MutableRefObject<States | null> => {
  // Note that the following also uses the deprecated service.state property for the same reason as above.
  const stateRef = useRef<States | null>((service.current.state?.value as States) || null);

  useEffect(() => {
    const handler: TransitionHandler = state => {
      stateRef.current = state.value as States;
    };

    service.current.onTransition(handler);

    return () => {
      service.current.off(handler);
    };
  }, [service.current, stateRef]);

  return stateRef;
};

Without the service.state it would not be possible (that I know of) to avoid an initial null value for the state, despite the machine having a non null state value.

[davidkpiano]

@alita-moore Have you tried using service.getSnapshot() instead of service.state?

[alita-moore]

@davidkpiano that seems to work! My bad for missing it. Thanks for the heads up.