statelyai / xstate

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

Bug: Unable to type `sendBack` in `fromCallback` #4666

Open offirgolan opened 5 months ago

offirgolan commented 5 months ago

XState version

XState version 5

Description

Currently, the fromCallback function takes two generics, the event type for receive and the input type but the sendBack generic type isn't exposed.

Currently, the only way to get around this is to do the following:

import type { InvokeCallback } from 'xstate/dist/declarations/src/actors/callback';

fromCallback((({ receive, sendBack, input }) => {
   // ...
}) as InvokeCallback<ReceiveEvent, SendBackEvent, InputEvent>);

Expected result

fromCallback should have a 3rd generic parameter for the sendBack event type.

Actual result

fromCallback does not have a 3rd generic parameter for the sendBack event type.

Reproduction

N/A

Additional context

No response

Andarist commented 5 months ago

You can either rely on the inference or use the CallbackActorLogic and type your sendBack inside the function's body:

import { fromCallback, CallbackActorLogic } from "xstate"; // types: 5.5.1

type ReceiveEvent = {
  type: "RECEIVED";
};

type SendBackEvent = {
  type: "SENT_BACK";
};

type SomeInput = {
  some: "input";
};

const callbackLogic1 = fromCallback(
  ({
    receive,
    sendBack,
    input,
  }: {
    receive: (cb: (ev: ReceiveEvent) => void) => void;
    sendBack: (ev: SendBackEvent) => void;
    input: SomeInput;
  }) => {},
);

const callbackLogic2: CallbackActorLogic<ReceiveEvent, SomeInput> =
  fromCallback(({ receive, sendBack: _sendBack, input }) => {
    const sendBack: (ev: SendBackEvent) => void = _sendBack;
  });
offirgolan commented 5 months ago

@Andarist these all feel like workarounds when 2/3 of the types can already be typed via the generics. Ideally, a 3rd generic parameter can be exposed so we can properly type the sendBack method.

Andarist commented 5 months ago

I'll give this a thought but our types are usually quite complex. To the point that I recommend sticking to inference over explicit type arguments.

offirgolan commented 5 months ago

Thanks, I appreciate it.

I recommend sticking to inference over explicit type arguments.

Looking at the type for fromCallback, it wont infer the sendBack event type since its not part of the generics and its type is being set to AnyEventObject.

Andarist commented 5 months ago

This is kinda na inference-based solution:

const callbackLogic1 = fromCallback(
  ({
    receive,
    sendBack,
    input,
  }: {
    receive: (cb: (ev: ReceiveEvent) => void) => void;
    sendBack: (ev: SendBackEvent) => void;
    input: SomeInput;
  }) => {},
);

It doesn't encode SendBackEvent in the callbackLogic1's type - that's true. We don't have any "slot" for it there. The whole sendBack is unsound today because we made it bivariant. This way you can annotate it like n the example above but you lose some safety lines.

Type-wise it's better to pass parent down as part of the input and have that typed, like here

offirgolan commented 5 months ago

@Andarist what is being inferred in the example you provided? Regardless, it's very verbose.

Type-wise it's better to pass parent down as part of the input and have that typed, like here

Doesn't this defeat the whole point of using sendBack? Why would it be better type wise to pass the parent to the input?

Andarist commented 5 months ago

@Andarist what is being inferred in the example you provided?

The callbackLogic1's type is inferred as CallbackActorLogic<ReceiveEvent, SomeInput>.

Doesn't this defeat the whole point of using sendBack?

Yes, in a way. We are still exploring the TS capabilities here and trying to balance different things.

Why would it be better type wise to pass the parent to the input?

Because now when you try to pass an incorrect parent to this actor TS should yell at you. An actor using sendBack can be passed to just any actor and it will be able to send types of events that might not be handled by its parent.

phcoliveira commented 2 months ago

I did a different hack.

import {fromCallback} from 'xstate';
import type {AnyEventObject, EventObject, NonReducibleUnknown} from 'xstate';
import type {InvokeCallback} from 'xstate/dist/declarations/src/actors/callback';

export function enhancedFromCallback<
    TReceive extends EventObject = AnyEventObject,
    TSend extends EventObject = AnyEventObject,
    TInput = NonReducibleUnknown
>(invokeCallback: InvokeCallback<TReceive, TSend, TInput>) {
    return fromCallback(invokeCallback);
}

It seems to work fine.

import {onValue} from 'firebase/database';
import type {Query} from 'firebase/database';

import {enhancedFromCallback, NoEventError} from '$lib';
import type {DatabaseKey} from '$lib';

export type TReceive =
    | {type: 'database.subscribe'; data: {query: Query}}
    | {type: 'database.unsubscribe'};

export type TSend =
    | {type: 'database.value'; data: {key: DatabaseKey; value: unknown}}
    | {type: 'database.empty'};

export const databaseSubscription = enhancedFromCallback<TReceive, TSend>(({sendBack, receive}) => {
    let unsubscribe: ReturnType<typeof onValue> | undefined;

    receive((event) => {
        switch (event.type) {
            case 'database.subscribe': {
                unsubscribe?.();

                unsubscribe = onValue(event.data.query, (snapshot) => {
                    if (snapshot.exists()) {
                        sendBack({
                            type: 'database.value',
                            data: {key: snapshot.key!, value: snapshot.val()},
                        });
                    } else {
                        sendBack({type: 'database.empty'});
                    }
                });

                break;
            }
            case 'database.unsubscribe': {
                unsubscribe?.();
                break;
            }
            default: {
                // Sentry
                // @ts-expect-error. It should never happen, as no other event should be sent.
                throw new NoEventError(event.type);
            }
        }
    });

    return () => unsubscribe?.();
});
phcoliveira commented 2 months ago

This makes think about how a child actor's events are perceived by the parent actor. I believe they are not "perceived" at all. Not in the sense of knowing its defined interface.

I find strange that the events sent by a child are handled like this:

const someCallback = fromCallback();

const parent = createMachine({
  id: 'parent',
  states: {
    active: {
      on: {
        'event.from.child': {
          actions: ['someAction']
        },
      },
      invoke: {
        id: 'child',
        src: someCallback
      }
    }
});

Because of this, the parent machine has to type itself the event event.from.child, as if it should know about it to redefine it.

I believe the events sent from a child could be handled just like the events onDone and onError, used for promises.

const someCallback = fromCallback();

const parent = createMachine({
  id: 'parent',
  states: {
    active: {
      on: {
        'parent.is.taking.over': {
          target: 'inactive'
        },
      },
      invoke: {
        id: 'child',
        src: someCallback
        // onDone: for promises, properly typed
        // onError: for promises
        on: {
          'event.from.child': { // type inferred from someCallback
            actions: sendTo('parent', ({event}) => ({
              ...event // work with properties if needed
              type: 'parent.is.taking.over'
            })
          },
        },
      }
    }
});

In this way, I believe the "contract" between parent and child is clearer.

andrecrimb commented 1 month ago

Thanks for your solution @phcoliveira 🥇. @Andarist is your team planing to cover this issue anytime soon?