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

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

Open
andrei-picus-tink opened this issue Sep 15, 2022 · 3 comments
Open
Labels
bug Something isn't working

Comments

@andrei-picus-tink
Copy link

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

@andrei-picus-tink andrei-picus-tink added the bug Something isn't working label Sep 15, 2022
@Andarist
Copy link
Member

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:

  • we either allow AnyStateMachine and you won't get any type-checking between parent and the child in this situation
  • or we keep using the provided service[name].data and you will have to adjust the declared service[name].data and apply this patch
-        done: { type: "final", data: (c) => c.foo }
+        done: { type: "final", data: (c) => c }

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
Copy link
Author

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
Copy link

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

@davidkpiano davidkpiano transferred this issue from statelyai/xstate Jan 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants