statelyai / xstate

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

Is sending event from within an action considered anti-pattern? #434

Closed coodoo closed 5 years ago

coodoo commented 5 years ago

See code sample below, three questions:

  1. Is sending event from within an action an anti-pattern?

  2. If yes, any recommended approach to trigger a state switch from within an action?

  3. If not, is there better way to pass in the send function, or make it available from within the action? Maybe by embedding it inside the context?

states: {
  a1: {...},
  a2: {
    on: { 
      'CLICK': {
        actions: assign((ctx, e) => {
          // using useMachine() outside and pass in `send` function via the click event
          const { send } = e
          // send another event from with-in the action
          send({type: 'SCROLL', value: 123})
        })  
      },
      'SCROLL': 'a1'
    }
  },
}
davidkpiano commented 5 years ago

Is sending event from within an action an anti-pattern?

Yes - actions should be fire-and-forget side-effects.

If yes, any recommended approach to trigger a state switch from within an action?

Do you have a real-life example of what you're trying to do? Why does it have to be from within an action in your case?

coodoo commented 5 years ago

Yes, I do have a real use case for that, in short I'm trying to implement optimistic update where I need to update the context multiple times, as show in the code sample below (as you can see I'm kind of using send as dispatch in a redux manner).

and here's my updated two questions:

1) Why actions need to be fire and forget? What if it's not?

2) What's the downside of sending multiple events as demoed below? Is there better approach to do that?

// let's say we are in a `create new item` window
// user just filled the form and hit `send` button
// and while we are persisting that new item via an API, 
// we also want to switch from `newItem` state back to `master` state
// here's how it goes:
actions: assign((ctx, e) => {
  const { payload, send } = e
  const newItem = { ...payload, id: randomId() }

  // drive fsm into 'master' state, along with the newly-created item
  // so that we can display this new item on screen, 
  // even though it's still on the wire back to the server
  send({
    type: 'GOTO_MASTER',
    payload: newItem,
  })

  // invoke API
  fetch(...)

  // and if the invokation was successful and data persisted
  // we want to display some notifications on the screen
  // we may also wanto to persist the result returned from the API in the context
  .then( result => {
    send({
      type: 'NEW_ITEM_SUCCESS',
      result
    })
  })

  // of course if the invokation failed, we need to show an alert
  // which brings us into another state
  .catch( error => {
    send({
      type: 'NEW_ITEM_FAIL',
      error,
    })
  })

})
davidkpiano commented 5 years ago

Side-effects that communicate back with the machines are invoked services (e.g., callbacks, Promises, etc.) Because it's an optimistic UI, it becomes a little tricky, as you are in some 'newItem.updating' as well as some 'view.master' state at the same time, so if you want to explicitly model this in the same state machine, you'd need parallel states.

However, using a service might be easier:

const appMachine = Machine({
  id: 'app',
  initial: 'master',
  invoke: {
    id: 'itemsService',
    src: () => (cb, onReceive) => {
      onReceive(event => {
        if (event.type === 'NEW_ITEM') {
          fetch(...)
            .then(result => cb({ type: 'NEW_ITEM_SUCCESS', result }))
            .catch(error => cb({ type: 'NEW_ITEM_FAIL', error }));
        }
      }
    }
  }
  states: {
    master: { ... },
    newItem: {
      on: {
        SUBMIT_NEW_ITEM: {
          // go back to master
          target: 'master',
          // send intent to service
          actions: send((_, e) => ({
            type: 'NEW_ITEM', 
            data: e.data
          }, { to: 'itemsService' })
        }
      }
    }
  }
});

The difference between this and an "action that can send back events" is that 'itemsService' is a single instance of a managed service, and it lets you implement more advanced logic like throttling, keeping track of the items, adding more features (updating and deleting items), teardown (abort in-flight Promises), etc.

It's basically a mini API / "microservice" that you can even swap out for testing.

davidkpiano commented 5 years ago

The spawning Actors proposal #428 can also be used for this, once it's in:

const createItem = (data) => Machine({
  id: 'item',
  context: data,
  invoke: {
    src: (ctx) => fetch(...),
    onDone: 'created',
    onError: 'error'
  },
  states: {
    // ...
  }
});

const appMachine = Machine({
  // ...
  on: {
    SUBMIT_NEW_ITEM: { actions: spawn(createItem) },
    NEW_ITEM_SUCCESS: { ... },
    NEW_ITEM_FAIL: { ... }
  }
});
coodoo commented 5 years ago

Invoked services are indeed all I needed to implement optimistic update, thanks for the examples!! 👍

pdcarroll commented 4 years ago

@davidkpiano This pattern is exactly what I'm trying to implement; however, I'm struggling to figure out how to use the send function the way you wrote in your example:

          actions: send((_, e) => ({
            type: 'NEW_ITEM', 
            data: e.data
          }, { to: 'itemsService' })

This sample doesn't appear to be valid JS because you're implicitly returning two separate values. Is there another way to use the send function in this manner (i.e., explicitly specifying the receiver and including the event data in the action)?

Side-effects that communicate back with the machines are invoked services (e.g., callbacks, Promises, etc.) Because it's an optimistic UI, it becomes a little tricky, as you are in some 'newItem.updating' as well as some 'view.master' state at the same time, so if you want to explicitly model this in the same state machine, you'd need parallel states.

However, using a service might be easier:

const appMachine = Machine({
  id: 'app',
  initial: 'master',
  invoke: {
    id: 'itemsService',
    src: () => (cb, onReceive) => {
      onReceive(event => {
        if (event.type === 'NEW_ITEM') {
          fetch(...)
            .then(result => cb({ type: 'NEW_ITEM_SUCCESS', result }))
            .catch(error => cb({ type: 'NEW_ITEM_FAIL', error }));
        }
      }
    }
  }
  states: {
    master: { ... },
    newItem: {
      on: {
        SUBMIT_NEW_ITEM: {
          // go back to master
          target: 'master',
          // send intent to service
          actions: send((_, e) => ({
            type: 'NEW_ITEM', 
            data: e.data
          }, { to: 'itemsService' })
        }
      }
    }
  }
});

The difference between this and an "action that can send back events" is that 'itemsService' is a single instance of a managed service, and it lets you implement more advanced logic like throttling, keeping track of the items, adding more features (updating and deleting items), teardown (abort in-flight Promises), etc.

It's basically a mini API / "microservice" that you can even swap out for testing.

davidkpiano commented 4 years ago

@pdcarroll Please put your code in a CodeSandbox. Easier to diagnose the issue there.

pdcarroll commented 4 years ago

@pdcarroll Please put your code in a CodeSandbox. Easier to diagnose the issue there.

Sure, here's a sandbox with a modified version of the sample you posted above: https://codesandbox.io/s/laughing-margulis-zd4yh

I included your sample implementation of send (line 29), but commented it out because it's invalid syntax. The version of send that I wrote is valid (and works), but it doesn't capture the event data. My question is, is there a way to send the action to the invoked service, including the event data, as you attempted to illustrate in your version of send?

Andarist commented 4 years ago

This sample doesn't appear to be valid JS because you're implicitly returning two separate values. Is there another way to use the send function in this manner (i.e., explicitly specifying the receiver and including the event data in the action)?

That's because you have made a syntax mistake, it should be:

          actions: send(
            (_, e) => ({
              type: "NEW_ITEM",
              data: e.data
            }),
            { to: "itemsService" }
          )

https://codesandbox.io/s/delicate-https-ujfyh

pdcarroll commented 4 years ago

@Andarist The invalid syntax was not my code, it was from the sample posted as a response to the original issue. That's why I asked for clarification. The change you made is exactly what I was looking for. Thank you!

smarko82 commented 3 years ago

I am having same questions of thread author, and I wasn't able to have a satisfying reply. In my situation, I have entry actions that draw some graphical objects for each state: it's something like:

    actions: {
        setInitialHomeDetailAndNavViews: (context: IMainPageFSMContext) => {
            const homeViewCtrl = new MenuViewController();

            context.homeViewCtrl.updateHomeNavView(homeViewCtrl);
        },

But I need to capture events of this viewcontrollers tp translate into events for my machine. I should do something like:

    actions: {
        setInitialHomeDetailAndNavViews: (context: IMainPageFSMContext) => {
            const homeViewCtrl = new MenuViewController();

            homeViewCtrl.onChangeFocus = () => send('MENU_ITEM_CHANGED'))

            context.homeViewCtrl.updateHomeNavView(homeViewCtrl);
        },

But, of course, I cannot use send action creator in this way. How can I send an event in the most efficient way in my situation?

davidkpiano commented 3 years ago

I am having same questions of thread author, and I wasn't able to have a satisfying reply. In my situation, I have entry actions that draw some graphical objects for each state: it's something like:

    actions: {
        setInitialHomeDetailAndNavViews: (context: IMainPageFSMContext) => {
            const homeViewCtrl = new MenuViewController();

            context.homeViewCtrl.updateHomeNavView(homeViewCtrl);
        },

But I need to capture events of this viewcontrollers tp translate into events for my machine. I should do something like:

    actions: {
        setInitialHomeDetailAndNavViews: (context: IMainPageFSMContext) => {
            const homeViewCtrl = new MenuViewController();

            homeViewCtrl.onChangeFocus = () => send('MENU_ITEM_CHANGED'))

            context.homeViewCtrl.updateHomeNavView(homeViewCtrl);
        },

But, of course, I cannot use send action creator in this way. How can I send an event in the most efficient way in my situation?

This would make a good discussion, but you would want to invoke instead of using an action.

Remember: actions are fire-and-forget, and you're not wanting to do any "forgetting" here.

smarko82 commented 3 years ago

I am having same questions of thread author, and I wasn't able to have a satisfying reply. In my situation, I have entry actions that draw some graphical objects for each state: it's something like:

    actions: {
        setInitialHomeDetailAndNavViews: (context: IMainPageFSMContext) => {
            const homeViewCtrl = new MenuViewController();

            context.homeViewCtrl.updateHomeNavView(homeViewCtrl);
        },

But I need to capture events of this viewcontrollers tp translate into events for my machine. I should do something like:

    actions: {
        setInitialHomeDetailAndNavViews: (context: IMainPageFSMContext) => {
            const homeViewCtrl = new MenuViewController();

            homeViewCtrl.onChangeFocus = () => send('MENU_ITEM_CHANGED'))

            context.homeViewCtrl.updateHomeNavView(homeViewCtrl);
        },

But, of course, I cannot use send action creator in this way. How can I send an event in the most efficient way in my situation?

This would make a good discussion, but you would want to invoke instead of using an action.

Remember: actions are fire-and-forget, and you're not wanting to do any "forgetting" here.

I opened new discussion https://github.com/davidkpiano/xstate/discussions/1835 If I cannot use actions to draw components, my architecture is really broken :) but I am happy to learn.

cdoublev commented 2 years ago

I hope you will forgive me for commenting on this long-closed issue.

Is sending event from within an action an anti-pattern?

Yes - actions should be fire-and-forget side-effects.

I find send(event) not sending an event (but creating an action that will send an event) a bit misleading. In a context (the processing of an action) where sending an event should not be allowed, it is even more confusing to me.

"Fire-and-forget" effects, which execute a synchronous side-effect with no events sent back to the statechart, or send an event synchronously back to the statechart

English is not my native language and I cannot say whether the italicized part means you can use send(event) or raise(event) but you cannot use service.send(event) or why I cannot use the later. And the difference between send() or raise() is still blurry to me, even with the provided exemple for rise(), which makes me feel dumb.

Andarist commented 2 years ago

I find send(event) not sending an event (but creating an action that will send an event) a bit misleading.

We understand that it might be a little bit misleading at first but take a look at how this is used within createMachine calls and consider that createMachine doesn't actually execute any side-effects. It's just a blueprint for a given behavior, it's like a class/template and you only create "instances" later on when actually instantiating concrete machines with interpret (or similar). Given that - this is sort of a requirement. We could try to think about different APIs for it, for instance we could require thunks:

entry: (ctx, ev, { self }) => self.send({ type: 'MY_EVENT' })

However, this would be way less statically analyzable and thus it would come with some serious tradeoffs for our tooling.

In a context (the processing of an action) where sending an event should not be allowed, it is even more confusing to me.

Could you elaborate? Not sure sure if I understand your intention correctly here

but you cannot use service.send(event) or why I cannot use the later.

You shouldn't use it because there is a good chance that it's not what you want. Please refer to the class/instance analogy that I've presented above. When using service.send directly from within actions and stuff you risk introducing different scheduling semantics etc because we no longer know that this event has a specific "sender".

And the difference between send() or raise() is still blurry to me, even with the provided exemple for rise(), which makes me feel dumb.

The difference is purely in the priority of both actions. Consider this:

entry: [send({ type: 'SENT_EVENT' }), raise({ type: 'RAISED_EVENT' })]

In here, the RAISED_EVENT will always be delivered to the machine first despite being "defined" later as an action. Both are queued but in different internal queues and the raisedQueue has to always be empty before we proceed to take any item from the sentQueue.

Note that we consider making this a clear distinction in the upcoming v5 and we might change this slightly:

We hope that introducing a change like this would make it more clear as to how one can use both of those.

cdoublev commented 2 years ago

Thank you for the answer!

Actually, I never expected send(event) would send an event because I had read the documentation which is perfectly clear on what it does. I just find that it hinders understanding for someone new to XState. schedule(event) seems clearer to me. But I guess it doesn't really matter anymore once the fire-and-forget (side effect free) concept is integrated.

In a context (the processing of an action) where sending an event should not be allowed, it is even more confusing to me.

Could you elaborate? Not sure sure if I understand your intention correctly here

It is also a feedback. From the description of effects quoted in my previous comment, the name chosen for send(), my level of English which is far from perfect, etc, I was not sure what is allowed or not in an action.

I wish I had an example of the difference between raise() and send() that I can visualize. I have an idea of ​​the difference in their implementation but haven't yet figured out the practical differences in using one or the other.

hlovdal commented 8 months ago

Some more examples of using invoke/services in this blog: https://stately.ai/blog/2021-05-13-why-i-love-invoked-callbacks.