Xstate: Transition in strict node does not report errors when event is not handled by StateNode

Created on 14 Jun 2018  ·  4Comments  ·  Source: davidkpiano/xstate

Bug (at least I think so)

There seems to be no way to determine if an event can be handled by the current state node.
For example:

const { Machine } = require('xstate');

const machine = Machine({
    initial: 'idle',
    strict: true,
    states: {
        idle: {
            on: {
                START: 'processing'
            }
        },
        processing: {
            on: {
                DONE: 'idle'
            }
        },
    }
});

let state = machine.initialState;

function transition(event) {
    const originalValue = state.value;
    state = machine.transition(state, event);
    console.log(`State changed from '${originalValue}' to '${state.value}'`);
}

transition('DONE');

Expected result:

// => Error: Machine '(machine)' does not accept event 'DONE'
// or:
// => Error: State node 'idle' does not accept event 'DONE'

Actual result:

// Actual:
// => State changed from 'idle' to 'idle'

Link to reproduction:

https://codepen.io/anon/pen/qKXbEZ

Discussion

I believe it's a bug, or at least there should be a way to avoid transitions. There's handles function that I can potentially use with machine.getStateNode, eg:

const stateNode = machine.getStateNode(state);
const isSupported = stateNode.handles(event);

However, it might not be as easy to implement for nested machines and I believe it should be checked in transition when running in strict mode.

Am I missing something ?

enhancement

Most helpful comment

To me, it makes sense that for the states

states: {
    idle: {
        on: {
            START: 'processing'
        }
    },
    processing: {
        on: {
            START: null, // or START: '' ?
            DONE: 'idle'
        }
    },
}

then machine.transition('idle', 'DONE') would return an object that indicated:

  • that the current state is (still) idle (current behaviour)
  • that none of the active states consumed the event (the new functionality)

Conersely, machine.transition('processing', 'START') would return an object that indicated:

  • that the current state is (still) processing (current behaviour)
  • that the event was consumed.

In my opinion, the fact that a state transition occurred or not is not worth exposing. If it's important, then you should add an action on the state entry, and react to that signal instead. Knowing if an event was consumed or not is the important part.

All 4 comments

This seems related to a discussion we've been having in the Gitter (cc. @mogsie @mpolichette).

I think what we want to do is _add a property_ to the State instance that answers the question:

Did this event result in a state transition?

With the results:

  • ✅ state transitioned to a different state
  • ✅ state transitioned to a different child state
  • ✅ state explicitly transitioned to self (internal or external)

    • Note: the state value doesn't change in this instance, but it still counts

  • ❌ state did not transition; event was not handled by any state node
  • :x: state did not transition; event was handled but next state target is undefined or null

Initially I was thinking of adding a changed: true property to the State instance, but we can bikeshed that.

What I _don't_ think we should do (at least by default) is throw an error if an event is unhandled. Happy to discuss this, though.

I agree, except maybe for the last "state did not transition, event was handled but next state target is undefined or null" — To me it's an indication of the statechart explicitly understanding the event, but choosing not to do anything. I'll have to think about it a bit, maybe come up with some more use cases, I guess :-)

Edit: Oh, and I agree: We should definitely not throw an error if the event was ignored :-)

To me, it makes sense that for the states

states: {
    idle: {
        on: {
            START: 'processing'
        }
    },
    processing: {
        on: {
            START: null, // or START: '' ?
            DONE: 'idle'
        }
    },
}

then machine.transition('idle', 'DONE') would return an object that indicated:

  • that the current state is (still) idle (current behaviour)
  • that none of the active states consumed the event (the new functionality)

Conersely, machine.transition('processing', 'START') would return an object that indicated:

  • that the current state is (still) processing (current behaviour)
  • that the event was consumed.

In my opinion, the fact that a state transition occurred or not is not worth exposing. If it's important, then you should add an action on the state entry, and react to that signal instead. Knowing if an event was consumed or not is the important part.

the fact that a state transition occurred or not is not worth exposing

This is now under state.changed (boolean | undefined).

There seems to be no way to determine if an event can be handled by the current state node

This is now under state.nextEvents and can be used like:

const currentState = machine.transition(...);

// Does it handle the FOO event?
currentState.nextEvents.includes('FOO');
// => true | false
Was this page helpful?
0 / 5 - 0 ratings

Related issues

rodinhart picture rodinhart  ·  3Comments

carloslfu picture carloslfu  ·  3Comments

greggman picture greggman  ·  3Comments

laurentpierson picture laurentpierson  ·  3Comments

carlbarrdahl picture carlbarrdahl  ·  3Comments