Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conditional actions #990

Closed
haywirez opened this issue Feb 9, 2020 · 9 comments · Fixed by #1076
Closed

Conditional actions #990

haywirez opened this issue Feb 9, 2020 · 9 comments · Fixed by #1076
Milestone

Comments

@haywirez
Copy link
Contributor

haywirez commented Feb 9, 2020

I think the possibility of conditional actions should be entertained, similar to guarded transitions:

 entry: [{ actions: 'xyz', cond: 'condition' }]
@semopz
Copy link

semopz commented Feb 10, 2020

I normally have "decision point" states with Transient Transitions. All the conditions go in that state which leaves the states with entry actions simpler.

so each action in your entry example will have a corresponding state (and associated entry action) + 1 decision point state with all the conditions.

I see entry as the synchronous version of invoke (not saying it is, but I think it serves my point)

@davidkpiano
Copy link
Member

SCXML precedent: https://www.w3.org/TR/scxml/#if

@MrNovado
Copy link

MrNovado commented Mar 1, 2020

From #1036 just for visibility and potential discussion.
The conditional action could also be represented as (context, event) => action | false, coupling action and conditional-check together, restricting the context and allowing for a simpler/optimized transition.

Or, to put it in perspective:

on: {
    "": [
       { target: "x", condAction: (context) => context?.x ? send({ type: "x", x: context.x }) : false  },
       { target: "y", condAction: (context) => context?.y ? sendParent({ type: "y", y: context.y }) : false  },
       { target: "z", condAction: (context) => context?.z ? assign({ z: context.z + 1 }) : false  },
       ...
    ]
}

For reasoning and examples check out the linked issue.

@davidkpiano
Copy link
Member

@MrNovado There is an undocumented (because it's experimental) pure() action creator that might do what you want:

import { pure } from 'xstate/lib/actions';

// ...
on: {
  '': {
    actions: pure(ctx => {
      return ctx.x ? send(/* ... */)
        : ctx.y ? sendParent(/* ... */)
        : ctx.z ? assign(/* ... */) : undefined;
      })
  }
}

Let me know if that satisfies your needs! pure() is just an action creator that lets you dynamically specify which action(s) you want to execute, by providing a "pure" function to pure(pureFn) (hence the name) that does not actually execute side-effects.

@MrNovado
Copy link

MrNovado commented Mar 2, 2020

@davidkpiano playing with it for a while, I realized conditional-actions, while cool and should definitely exist, is not the thing I actually wanted. I want a simpler way of declaring a transition.

Made another issue for it here #1038; hopefully, explained myself better this time.

pure looks great btw and I want the same'ish API for transitions, but with guard-resolution signaling.

@Andarist
Copy link
Member

A draft implementation is available here: #1076

@awreccan
Copy link

@Andarist @davidkpiano Thank you for implementing choose(). I'm curious - what are the benefits of using choose() over one action with if/else, other than syntactical clarity? I'm migrating some existing code into an xstate-based implementation and want to minimise cognitive overload for my team, so not having to refactor existing functions would help.

  // impl with one action
  // reuses the form of our existing function, just exit points replaced with `raise()`
  onError: {
    target: 'failure', // TODO or retry?
    actions: (_, { data: err }) => {
      // Runtime error
      if (!(err instanceof SomeError)) {
        throw err
      }

      let shouldRetry
      switch (err.type) {
        case FAILURE_REASON_A:
          return raise(FAILURE_REASON_A)

        case FAILURE_REASON_B:
        case FAILURE_REASON_C:
        case FAILURE_REASON_D:
          return raise('RETRY')

        default:
          return raise('FAILURE')
      }
    },
  },

  // impl with choose()
  onError: {
    target: 'failure',
    actions: choose([
      {
        // Runtime error
        cond: (_, { data: err }) => !(err instanceof SomeError),
        actions: (_, { data: err }) => throw err,
      },
      {
        cond: (_, { data: err }) => err.type === FAILURE_REASON_A,
        actions: raise(FAILURE_REASON_A),
      },
      {
        cond: (_, { data: err }) =>
          [FAILURE_REASON_B, FAILURE_REASON_C, FAILURE_REASON_D].includes(err.type),
        actions: raise('RETRY'),
      },
      {
        actions: raise('FAILURE'),
      },
    ]),
  },

By the way, I'm not sure if the action: (ctx,event)=>{} needs to be wrapped in pure() like so: action: pure((ctx,event)=>{}) - any pointers here? Since both (ctx,event)=>{} and raise() are action creators, I'm assuming it makes sense to write an action like (ctx,event)=>{ ... return raise() ... } without wrapping it in pure(). I'm probably confused by the pure() docs...

@Andarist
Copy link
Member

I'm curious - what are the benefits of using choose() over one action with if/else

choose allows for better visualization than if/else

I'm not sure if the action: (ctx,event)=>{} needs to be wrapped in pure() like so: action: pure((ctx,event)=>{}) - any pointers here?

(ctx,event)=>{} is just a custom function that gets called but it cannot execute/schedule any builtin actions like assign, send etc. For those, you need pure.

@andrecrimb
Copy link

Hello @Andarist 👋🏽
Typegen is great, but having the implementation of all actions in the machine body might turn the file a bit long, do we have any examples in terms of typing actions properly, when these are defined in another file?

eg:

actions.ts

const someCoolAction = pure<[how to type this properly]>(() => assign(....))

coolMachine.ts

import * as actions from "./actions"

export const coolMachine = createMachine(
....
    tsTypes: {} as import('./coolMachine.typegen').Typegen0,
....


actions: {
    someCoolAction: actions.someCoolAction,
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants