statelyai / xstate-tools

Public monorepo for XState tooling
183 stars 40 forks source link

Bug: typegen resolves "exit" actions to 'xstate.init' #89

Closed SimeonC closed 2 years ago

SimeonC commented 2 years ago

Description

Running typegen on this code, the exit event is incorrectly typed to 'xstate.init' as in this screenshot.

CleanShot 2022-03-02 at 11 11 31

Expected result

Exit events should be at least typed to "all events" in the machine schema: { events }.

In this particular case it would have been nice to have it typed to FOCUS event, but that's probably impractical.

Actual result

Exit actions are typed to 'xstate.init' instead of ''

Reproduction

NA - see additional context for example machine and output

Additional context

const test =
  /** @xstate-layout N4IgpgJg5mDOIC5gF8A0IB2B7CdGgAoBbAQwGMALASwzAEoA6AMyoCdYAXfEABy1iocqWDNwAeiACwAmdAE9EATgCMDAAwa1kgMw7pa6ZMWTkaEMXLVadbnwFCR4xMoAc85y-WaX06QFYAdgA2X0VFaVNTIA */
  createMachine(
    {
      tsTypes: {} as import('./machine.typegen').Typegen0,
      schema: { events: {} as { type: 'SOME_ACTION' | 'B'; value: string } },
      id: '(machine)',
      states: {
        first: {
          exit: 'exitAction',
          on: { SOME_ACTION: 'second' }
        },
        second: {}
      }
    },
    {
      actions: {
        exitAction: (ctx, event) => {
          const t = event.value;
        }
      }
    }
  );
export interface Typegen0 {
  '@@xstate/typegen': true;
  eventsCausingActions: {
    exitAction: 'xstate.init';
  };
  internalEvents: {
    'xstate.init': { type: 'xstate.init' };
  };
  invokeSrcNameMap: {};
  missingImplementations: {
    actions: never;
    services: never;
    guards: never;
    delays: never;
  };
  eventsCausingServices: {};
  eventsCausingGuards: {};
  eventsCausingDelays: {};
  matchesStates: 'first' | 'second';
  tags: never;
}
Andarist commented 2 years ago

Where this expectation comes from?

Exit actions are typed to 'xstate.init' instead of ''

Note that I'm working on introducing a special xstate.stop event that will end up being provided to all exit actions (apart from the fact that exit actions should also be "callable" with events that can lead to exiting a particular state). This special event is required because exit actions can be always called when a machine is running - for instance it can be canceled/stopped by a parent machine etc.

SimeonC commented 2 years ago

Ah, sorry, I simplified the example too much. I was referring to the case of an event causing the state to be exited, not the general case of exiting the whole machine on stop. I've updated the example to be more clear (though I can't update the typegen from mobile).

I expect the exit action to be at least narrowable to the event. I've used this a lot for cases when all my events in the machine extend one base event, so I know that the event usually has the shared event properties. I didn't consider the exit machine case though so this might be an anti-pattern. I had a fair few events that could cause the state to be exited and it was easier to put the action on the exit rather than on each event definition.

Andarist commented 2 years ago

In the given example the declared schema.events doesn't match the on.SOME_ACTION :P but I get the gist.

IMHO the typegen should declare this:

  eventsCausingActions: {
    exitAction: 'SOME_ACTION' | 'xstate.stop';
  };

Where that xstate.stop is not yet implemented (but it's going to be soon). When a machine gets stopped then all exit handlers of the current state configuration are called, so basically every exit action will always be "callable" with xstate.stop. Any thoughts on that?

SimeonC commented 2 years ago

Now I'm back at my computer I've updated the example to be correct.

Yes, I was expecting that in the typegen as well. I also thought that doing that may be quite difficult with all the possible events that could cause you an exit (parent events, events in parallel machines etc) so I was hoping that it would at least be resolving to "any event" not never.

Andarist commented 2 years ago

I've transferred this issue to https://github.com/statelyai/xstate-tools - since the algorithm for computing those events is living here.

mattpocock commented 2 years ago

Agree with all of the above, the algorithm should take this into account. This has been one of its weaknesses since it lived in xstate-codegen.

mattpocock commented 2 years ago

Added in #114, but need to co-ordinate it with XState's release of xstate.stop

with-heart commented 2 years ago

I think this can be closed. Merged in statelyai/xstate#3126 and I see it working in local typegen