statelyai / xstate-tools

Public monorepo for XState tooling
183 stars 36 forks source link

Bug: typegen does not like invoke functions returning machines #298

Open andrei-picus-tink opened 2 years ago

andrei-picus-tink commented 2 years ago

Description

We use factories to create machines with parameters that are then invoked from other machines. Typegen is unhappy when the invoke function returns a machine from the factory, but is happy if we pass the machine directly.

We tried passing context from parent to child using data, but it doesn't seem to be typesafe — typos in data are not checked against the actual context of the invoked machine.

const createNestedMachine = (someData: any) => createMachine(...);

const parentMachine = createMachine({
  schema: {
    services: {} as { machine: { data: string } }
  },
  services: {
    machine: (context) => createNestedMachine(context.data)
  }
});

Expected result

Typegen is happy.

Actual result

It looks like typegen compares the context of the invoked machine against the services schema:

Types of property 'activities' are incompatible.
Type 'Record<string, ActivityConfig<string, any>> | undefined' is not assignable to type 'Record<string, ActivityConfig<{ foo?: string | undefined; }, AnyEventObject>> | undefined'.
  Type 'Record<string, ActivityConfig<string, any>>' is not assignable to type 'Record<string, ActivityConfig<{ foo?: string | undefined; }, AnyEventObject>>'.
    'string' index signatures are incompatible.
      Type 'ActivityConfig<string, any>' is not assignable to type 'ActivityConfig<{ foo?: string | undefined; }, AnyEventObject>'.
        Type '{ foo?: string | undefined; }' is not assignable to type 'string'.ts(2322)

The machine runs fine though (see the sandbox). If you remove the () => then the type errors go away. Alternatively, if we change the services schema to data: any that also fixes it.

Reproduction

https://codesandbox.io/s/thirsty-rosalind-gifq27?file=/src/App.tsx

Additional context

No response

Andarist commented 2 years ago

We plan to rethink how parent-child communication is done in v5 and hopefully, this will get fixed holistically then.

The problem here is that we don't have any special generic slot for the "output data" of machines. It seems that we've used the declared service[name].data as the expected type for the whole context of the invoked actor here: https://github.com/statelyai/xstate/blob/0991ca9f6c70c661545dc1f594ab64b141d37b5c/packages/core/src/types.ts#L343 but we "forgot" to do the same here: https://github.com/statelyai/xstate/blob/0991ca9f6c70c661545dc1f594ab64b141d37b5c/packages/core/src/types.ts#L838

I think that both of those places should behave the same. I'm not sure though how this should be adjusted. We have a dilemma here:

However, the data callback is not validated at all... So in the case of invoked machines whatever you declare in service[name].data is not type-safe anyway. So perhaps, for the time being, we should just allow AnyStateMachine. WDYT?

andrei-picus-tink commented 2 years ago

In a way passing the whole context up from the child gives us type safety, because the invoke property checks it. It forces the parent to conform to the child's type though, which can be problematic if the child starts with some undefined state that becomes defined in the final state 🤷

I would personally go with AnyStateMachine in XState, to let users choose what they want to communicate to parent machines, and not be forced by some arbitrary restriction.

We plan to rethink how parent-child communication is done in v5

Do you have anything to share on this? 😄

martijnarts commented 1 year ago

I ran into this as well. Might be good to add to the Typescript section in the documentation