Fixed an issue with states coming from model.createMachine resulting in contexts of type any after type refinements
#2436
Conversation
…g in contexts of type `any` after type refinements
.changeset/warm-spies-cry.md
Outdated
| 'xstate': patch | ||
| --- | ||
|
|
||
| Fixed an issue with states coming from `model.createMachine` resulting in contexts of type `any` after type refinements such as here: |
There was a problem hiding this comment.
I'm super tired already and I have a feeling that I've written some hard-to-understand gibberish here. If you have any ideas on how I could rephrase this I would appreciate the help.
There was a problem hiding this comment.
Maybe something like?
Fixed issue where, when using model.createMachine, state context was incorrectly inferred to be any after refinement with .matches(..), for example:
There was a problem hiding this comment.
Thank you very much for taking a stab at rewording this! Would you like to prepare this as a PR suggestion that I could just commit with a button click? The change would also be contributed to you that way which is nice :)
There was a problem hiding this comment.
| Fixed an issue with states coming from `model.createMachine` resulting in contexts of type `any` after type refinements such as here: | |
| Fixed an issue where, when using `model.createMachine`, state context was incorrectly inferred as `any` after refinement with `.matches(...)`, such as in here: |
Using @johtso's suggestion
| @@ -32,7 +32,7 @@ export interface Model< | |||
| createMachine: ( | |||
| config: MachineConfig<TContext, any, TEvent>, | |||
| implementations?: Partial<MachineOptions<TContext, TEvent>> | |||
| ) => StateMachine<TContext, any, TEvent, any>; | |||
| ) => StateMachine<TContext, any, TEvent>; | |||
There was a problem hiding this comment.
This resulted in the default type not being chosen here:
xstate/packages/core/src/State.ts
Line 93 in 141c91c
which in turn resulted in
any being selected here:xstate/packages/core/src/State.ts
Line 298 in 68692c1
The alternative fix here would be to use the default type explicitly here. It would be more explicit but it's probably not worth it at the moment.
There was a problem hiding this comment.
Thanks for digging into this and fixing it.
There was a problem hiding this comment.
No problem. That's my job 😍
packages/core/src/State.ts
Outdated
| ? TTypestate | ||
| : never | ||
| : never)['context'], | ||
| Equals<TTypestate, any> extends 1 |
There was a problem hiding this comment.
This is not strictly necessary because the other. simple, fix already fixes the reported issue.
This seems a little bit heavy (adding more local complexity here) but handles "more" scenarios. I feel like if the TTypestate is not configured then the most sensible approach is to prefer using TContext. However, this still doesn't handle unknown and never and if we do this then it would probably be the most sensible approach for all of those special types.
I think I'm leaning towards removing this part of the fix for the time being. Thoughts?
There was a problem hiding this comment.
Yeah, let's remove this and revisit later 👍
packages/core/src/State.ts
Outdated
| type Equals<A1 extends any, A2 extends any> = (<A>() => A extends A2 | ||
| ? 1 | ||
| : 0) extends <A>() => A extends A1 ? 1 : 0 | ||
| ? 1 | ||
| : 0; |
There was a problem hiding this comment.
Could this be true or false instead of 1 or 0?
There was a problem hiding this comment.
It could. It's just taken from here and this type collection is using the 1/0 pattern in all its types.
I'm still wondering if we should use this though, see the comment here: #2436 (comment)
fixes #2423
Note that this PR contains 2 separate fixes for the reported issue. Any of them could be removed and the reported issue would still be fixed. I will comment on each one "inline"