statelyai / xstate

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

enqueue.assign() is not typesafe #4810

Open ivankoleda opened 6 months ago

ivankoleda commented 6 months ago

XState version

XState version 5

Description

If we define types in setup function like this

setup({
    types: {} as {
      context: {
        foo: {
          bar: string;
        };
      };
    },
  })

And add a following action handler

 actions: [
  enqueueActions(({ enqueue }) => {
    enqueue.assign({
      // No TS error
      foo: undefined,
    });
  }),
],

There won't be a type error and accessing context.foo.bar will result in runtime error as context.foo can be undefined during runtime.

Doing the same with assign results in type error though:

actions: [
  assign({
    // TS error
    foo: () => undefined,
  })
]

Expected result

Typescript error

Actual result

No Typescript error

Reproduction

https://stackblitz.com/edit/vitejs-vite-jwifv4?file=src%2Fmain.ts

Additional context

No response

Andarist commented 6 months ago

You are not comparing the same flavor of assign here. If we want to compare the same flavors then we need to compare those two:

assign({
  foo: undefined,
}),
enqueueActions(({ enqueue }) => {
  enqueue.assign({
    foo: undefined,
  });
}),

And they both don't error here. The problem is that assign accepts a "partial assigned" - u are free to pass just some keys (and not everything the context has). The problem here is that TS (by default) always allows undefined value for optional properties (and Partial creates optional properties). One needs exactOptionalPropertyTypes to change this behavior and not many projects have that setting enabled.

Andarist commented 6 months ago

We can't address this as long as we allow property assigners derived from Partial<TContext>. So the Bug label here isn't quite fitting (cc @davidkpiano). It feels like a design limitation with the current API.

davidkpiano commented 6 months ago

I added the recommendation to use exactOptionalPropertyTypes in the TypeScript documentation https://stately.ai/docs/typescript#set-up-your-tsconfigjson-file

{
  "compilerOptions": {
    // ...
    "strictNullChecks": true,
    // or set `strict` to true, which includes `strictNullChecks`
    // "strict": true,
    "skipLibCheck": true,
    "exactOptionalPropertyTypes": true
  }
}
Andarist commented 6 months ago

@davidkpiano this likely should be removed from the docs for the time being since our types are actually not fully compatible with this flag, see https://github.com/statelyai/xstate/issues/4613