Issue 179: `onChange` not called for `immediate` states

[ehuelsmann]

This PR solves issue #179; it makes sure onChange() is called for every state the machine transitions to.

There is an additional side effect to this change: it doesn't call onChange() when send() is called with an unsupported event name. That is, if a state only has a transition triggering on the "done" event, onChange() would be called even when executing send("fail"). This commit does not eliminate the case where the machine transitions from and to one and the same state, although with this change, detecting that situation and eliminating the invocations should be trivial.

[ehuelsmann]

@matthewp hi. Hope you like this contribution. If not, let me know how you'd like me to adjust it (or that you want it to go away).

[matthewp]

Thanks for sending this! I think this would qualify as a breaking change, so I want to keep the PR open for a bit. I think I'm onboard but would want other people to weigh in as well. Do we want a change event for an immediate action?

I'm curious, does anyone know how XState behaves here?

[ehuelsmann]

Anything I can do to help expedite this change?

[matthewp]

@ehuelsmann Sorry for the delay. Can you answer the question about XState? Does it trigger its onchange equivalent for immediate states?

[ehuelsmann]

@matthewp just checked with the following example:

import { createMachine, interpret } from 'xstate';

const skipMachine = createMachine({
  id: 'skip',
    initial: 'one',
    predictableActionArguments: true,
  states: {
    one: {
      on: { CLICK: 'two' }
    },
    two: {
        always: [ { target: 'three' } ]
    },
    three: {
      type: 'final'
    }
  }
});

const service = interpret(skipMachine).onTransition((state) => {
  console.log(state.value);
});

service.start();
service.send({ type: 'CLICK' });

service.stop();

where:

$ node index.mjs 
one
three

[matthewp]

With that being the case I think we should probably not merge this.

[matthewp]

Closing to remain consistent with XState, thank you for taking the time regardless.

[ehuelsmann]

@matthewp apparently, xslate wasn't happy with their decision not to call onTransition, so they created a new callback with exactly the behaviour this PR is asking for. They merged this in 2021-or-so for v5: https://github.com/statelyai/xstate/issues/1767

[matthewp]

Ok! Let's reopen to reconsider.

[ehuelsmann]

Apart from updating this to eliminate the conflicts, is there anything I can do here?

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: f10af7975783a1a6647a8ba78ea58752a18a5b45

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

| Name | Type | | ------------- | ----- | | robot3 | Major | | haunted-robot | Major | | lit-robot | Major | | preact-robot | Major | | react-robot | Major | | robot-hooks | Major |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[ehuelsmann]

@matthewp conflicts resolved that were caused by the new project structure. Hope that helps to consider this PR.

[matthewp]

Ok,let's do it. Can you add a changeset? npm run changeset. This would be a major.

[ehuelsmann]

@matthewp Figured out the problem. Changeset added. I'll remove all the problem reports from the PR; they clutterthe essence.

[ehuelsmann]

@matthewp hi. May I kindly request your feedback on this PR? I'd like to know if there is anything I can do to advance it

[matthewp]

@ehuelsmann Sorry for the delay. If you could just update the changeset with a description of the change I'll get this released.

[ehuelsmann]

Rebased on top of "main".

[ehuelsmann]

@matthewp , please let me know if there's anything more I can do to help here.

[ehuelsmann]

Thanks for the merge!