ChrisShank / xstate-vue2

Vue 2 composables for XState.
MIT License
19 stars 5 forks source link

`service.send` gets ignored past first call on initial setup #6

Closed martynchamberlin closed 1 year ago

martynchamberlin commented 1 year ago

Appreciate the work on this! One issue I'm running into. I am using script/setup in a project with the Vue 2.7.10 compatibility build, and if I do two service.send() calls back-to-back in the root of the Vue script (the equivalent of the setup function as shown in this repo's README), only the first one gets honored. Took me a minute to realize that the reason is because the listener only gets set in the mount event.

https://github.com/ChrisShank/xstate-vue2/blob/479b93786071a1136e5725dcf42a024c347fe560/src/useInterpret.ts#L81-L85

For this reason, it's a bit misleading to show usage of this directly in the setup portion of a function, since setup fires before the component mounts. I wonder if we could use an earlier hook in the component lifecycle, in useInterpret.ts?

ChrisShank commented 1 year ago

Thanks for filing an issue! Could you just share a small example of what you are describing? Just want to make sure I understand!

I’ve known there was a potential bug here since I ported this lib… the onMounted isnt necessary imo, but I wanted to keep the API/behavior the same so someone could easily move to @xstate/vue.

martynchamberlin commented 1 year ago

@ChrisShank Sure! I have a Vue component that looks like this:

<template>
  <div v-text="state.value.playing" />
</template>

<script lang="ts" setup>
import { sampleMachine } from './sample-machine';
import { useMachine } from 'xstate-vue2';

const { state, service } = useMachine(sampleMachine);

service.send('EVENT_1');
service.send('EVENT_2');
</script>

Both of those service.send() calls happen before the useInterpret's mount event, so only the last send() gets honored. (And even then, I think the only reason it works is because that send() changes the state of the core machine, so useMachine thinks it's the "initial" state.)

Does that make sense?

ChrisShank commented 1 year ago

Interesting… I’ve put up a PR with your suggestion, but I need to do some more testing. Based on my understanding, I’m not sure that the events would be “ignored” but I think it is true the current state of the machine would not be correct on the first render if events are sent in setup!

One more question, could you share a minimal version of sampleMachine too? If I can reproduce this behavior then it should be pretty quick to release a patch.

https://github.com/ChrisShank/xstate-vue2/pull/7/files

martynchamberlin commented 1 year ago

@ChrisShank Ah, I see what it is. You can't reproduce unless you have guards in place. With this state machine:

export const sampleMachine = createMachine({
  schema: {
    events: {} as
      | { type: 'EVENT_1' }
      | { type: 'EVENT_2' },
  },
  // eslint-disable-next-line @typescript-eslint/consistent-type-imports
  tsTypes: {} as import('./sample-machine.typegen').Typegen0,
  id: 'puzzleCore',
  predictableActionArguments: true,
  initial: 'playing',
  states: {
    playing: {
      initial: 'state1',
      states: {
        state1: {
          on: {
            EVENT_1: { target: 'state2', cond: 'guard1' },
          },
        },
        state2: {
          on: {
            EVENT_2: { target: 'state2', cond: 'guard2' },
          },
        },
      },
    },
  },
}, {
  guards: {
    guard1: () => true,
    guard2: () => false,
  },
});

...if you run these two transitions in the root of a script/setup component:

service.send('EVENT_1');
service.send('EVENT_2');

...you should still be in state2. The bug is that you won't be. This will be the case if you run them inside an onMounted event, but it's not the case if you run it in the root.

ChrisShank commented 1 year ago

@martynchamberlin thanks for clarifying! Is EVENT_2: { target: 'state2', cond: 'guard2' }, a typo? You would be transitioning to the same state?

Either way I confirmed the bug! https://stackblitz.com/edit/vite-zrtbpm?file=main.js

image

Next time Ill ask for a minimal reproduction earlier on.

ChrisShank commented 1 year ago

Published xstate-vue@0.3.1. Forking the reproducer and bumping the version looks like a fix. Thanks again!

https://stackblitz.com/edit/vite-skoepc?file=package.json