[4.7.0] proper type for second generic parameter to assign?

[wKovacs64]

Description

May not be a bug, might just be a misunderstanding. Prior to v4.7.0, I had omitted the second generic parameter of the assign action creator and TypeScirpt was OK with it, but now it complains that event does not have a data property (which makes sense, because it now defaults to EventObject which does not have said property).

Example:

interface Context {
  count: number;
}

interface Schema {
  states: {
    inactive: {};
    active: {};
  };
}

interface ToggleEvent {
  type: 'TOGGLE';
}

const machine = Machine<Context, Schema, ToggleEvent>(
  {
    id: 'machine',
    initial: 'inactive',
    context: {
      count: 0,
    },
    states: {
      inactive: {
        on: { TOGGLE: 'active' },
      },
      active: {
        invoke: {
          id: 'setCountService',
          src: () => Promise.resolve(42),
          onDone: {
            actions: 'setCount',
          },
        },
        on: { TOGGLE: 'inactive' },
      },
    },
  },
  {
    actions: {
      // what should the second generic parameter to `assign` be?
      setCount: assign<Context /* , ___here___ */>({
        count: (_, event) => event.data,
      }),
    },
  },
);

Expected Result

No errors.

Actual Result

Property 'data' does not exist on type 'EventObject'.

Reproduction

https://codesandbox.io/s/xstate-issue-836-w3bjm

Additional context

I found AnyEventObject works, but I am uncertain if that's correct. Is there something narrower that makes more sense to use here?

[davidkpiano]

The second parameter should be which events would trigger this action, or (in your case) just ToggleEvent.

[wKovacs64]

In this example, it's not a ToggleEvent because the action is called from an onDone handler of an invoked service. So event.data is the result of the resolved Promise in the service src property. It's in the CodeSandbox, but I updated the issue to include the entire machine.

Maybe I'm just way more off base than I thought? 😅

[davidkpiano]

Ah, then provide it the event that you expect.

In 4.7, event typings were fixed because it was a mistake to assume that an event can contain any payload where it might not exist.

[wKovacs64]

Ah, then provide it the event that you expect.

That doesn't seem to work, as TS complains if the event passed here does not match the event that was passed into the machine config:

interface SetCountServiceResolvedEvent {
  type: 'done.invoke.setCountService';
  data: number;
}

// or maybe just `DoneInvokeEvent<number>` ?

Argument of type '{ actions: { setCount: AssignAction<Context, SetCountServiceResolvedEvent>; }; }' is not assignable to parameter of type 'Partial<MachineOptions<Context, ToggleEvent>>'.
  Types of property 'actions' are incompatible.
    Type '{ setCount: AssignAction<Context, SetCountServiceResolvedEvent>; }' is not assignable to type 'Record<string, ActionFunction<Context, ToggleEvent> | ActionObject<Context, ToggleEvent>>'.
      Property 'setCount' is incompatible with index signature.
        Type 'AssignAction<Context, SetCountServiceResolvedEvent>' is not assignable to type 'ActionFunction<Context, ToggleEvent> | ActionObject<Context, ToggleEvent>'.
          Type 'AssignAction<Context, SetCountServiceResolvedEvent>' is not assignable to type 'ActionObject<Context, ToggleEvent>'.
            Types of property 'exec' are incompatible.
              Type 'ActionFunction<Context, SetCountServiceResolvedEvent> | undefined' is not assignable to type 'ActionFunction<Context, ToggleEvent> | undefined'.
                Type 'ActionFunction<Context, SetCountServiceResolvedEvent>' is not assignable to type 'ActionFunction<Context, ToggleEvent>'.
                  Types of parameters 'event' and 'event' are incompatible.
                    Type 'ToggleEvent' is not assignable to type 'SetCountServiceResolvedEvent'.

So I guess it needs to match what was passed into the machine initially, which should be a union of all possible event types that could take place within the machine (both incoming and "internal")? Which would mean I need a type guard (etc.) to ensure the event is the right type inside the action.

That's fine if that's how it's supposed to be - just wanting to make sure I'm not working around a problem.

[davidkpiano]

Can you update the CodeSandbox with what you have now?

[wKovacs64]

Updated the example CodeSandbox, but here is the real code:

https://codesandbox.io/s/xstate-issue-836-pwl-hr1zj?module=/src/utils/use-update-poller.ts

The same problem manifests itself in two different machines:

• src/utils/use-update-poller.ts (line 91)
• src/components/pwned-info.tsx (line 95)

I must be typing something incorrectly? Changing the third generic parameter on Machine to any works, but that's obviously not ideal nor should it be necessary.

[davidkpiano]

Workaround:

      setErrorMessage: assign<
        UpdatePollerContext,
        UpdatePollerEvent
      >({
        ...initialContext,
        error: (_, event) => (event as UpdatePollerCheckFailureEvent).data.message,
      }),

[davidkpiano]

In general, since an action like this is specific to onError, it's best to not define serialize it/define it in { actions: ... } and instead define it inline (in { onError: { actions: ... } }).

[wKovacs64]

I actually had that exact workaround in place but figured it was cheating. Is the workaround necessary due to limitations in the XState types, or because I'm doing something incorrectly?

In general, since an action like this is specific to onError, it's best to not define serialize it/define it in { actions: ... } and instead define it inline (in { onError: { actions: ... } }).

I put it there due to the tip in the Actions guide in the docs:

It is not recommended to keep the machine config like this in production code, as this makes it difficult to debug, serialize, test, and accurately visualize actions. Always prefer refactoring inline action implementations in the actions property of the machine options, like the previous example.

🤷‍♂

However, moving them inline does eliminate the problem (and does not require the workaround). So... fine with me! 🙂

[wKovacs64]

On a related note, should the third generic parameter to Machine (TEvent) include the internal events that are specific to internal actions? Or only event types that can be sent into the machine?

(removing the internal events appears to have no adverse effect after inlining the actions)

[davidkpiano]

should the third generic parameter to Machine (TEvent) include the internal events that are specific to internal actions? Or only event types that can be sent into the machine?

Only event types that can be sent to the machine. The other internal actions will be added (union) internally.

[wKovacs64]

Only event types that can be sent to the machine. The other internal actions will be added (union) internally.

OK, cool.

So overall, is it your opinion that everything is fine and we can close this issue? I can PR some documentation updates to reflect the findings here, if you'd like.

[davidkpiano]

I think typing can be improved, because assign<...> should be able to take in a specific subset of TEvent instead of forcing all TEvent events inside.

[CMCDragonkai]

I was facing this problem as well: https://github.com/davidkpiano/xstate/issues/863#issuecomment-718561133

The type errors aren't very clear for fixing this. Hopefully this can be spelled out in the documentation.

Now I have to do this:

actions: assign<BackupRestoreContext, {type: 'DELETE_RESTORE'}>({ backedUp: false }),

Seems quite verbose.

[Andarist]

@CMCDragonkai Please take a look at those 2 answers: https://github.com/davidkpiano/xstate/discussions/1591#discussioncomment-111875 https://github.com/davidkpiano/xstate/discussions/1591#discussioncomment-111875

[davidkpiano]

Also, see this PR which will simplify things: https://github.com/davidkpiano/xstate/pull/1439#issue-480529519

[Andarist]

It's currently impossible for us to infer the per-action event types at the type level. We can't process the state machine transitions at the type level to learn about the possible event types for given actions.

You can use our typegen to get better types in those kinds of situations: https://xstate.js.org/docs/guides/typescript.html#typegen

Without typegen the proper way of typing this is to do this:

  {
    actions: {
      // what should the second generic parameter to `assign` be?
      setCount: assign<Context, ToggleEvent>({
        count: (_, event) => event.data,
      }),
    },
  },

In this example, the ToggleEvent represents all of the events that this particular machine accepts. It should also be possible to just omi the generic type arguments here and let them be inferred from the createMachine call.