statelyai / xstate

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

feat(core): expose UnknownActorRef #4929

Closed boneskull closed 5 months ago

boneskull commented 5 months ago

AnyActorRef is problematic for some use cases, because the result of the AnyActorRef['getSnapshot'] function is of type any.

However, changing AnyActorRef to use a narrow type in ActorRef's first type argument, TSnapshot, breaks conditional types which perform inference based on TSnapshot.

This change introduces UnknownActorRef, which is like AnyActorRef, but is not intended to be inferred from in future conditional types. A consumer can use UnknownActorRef like so:

const actor: UnknownActorRef = getSomeActor();

// inferred as `Snapshot<unknown>`
const snapshot = actor.getSnapshot();

// do things w/ snapshot

AnyActorRef would return any from actor.getSnapshot(), which make it unsuitable for this use-case in a strictly-typed environment.

changeset-bot[bot] commented 5 months ago

🦋 Changeset detected

Latest commit: 54195ec52cf2290c43aa8b88cf2158468818353d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ------ | ----- | | xstate | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

boneskull commented 5 months ago

regardless; if this (or the other suggestion) makes sense, I'll go in and make the change and/or fix the type tests.

davidkpiano commented 5 months ago

Thank you for this! It seems like TS is complaining in some of the test files (you can run yarn typecheck to check) 🕵️

cc. @Andarist

boneskull commented 5 months ago

@davidkpiano I don't quite understand why the 3rd any in AnyActorRef was removed in dbeafeb25eed63ee1d2b027f10bc63d9937ab073 (#4905). That change seems to break some stuff anyhow. I'll create a simple reproduction for that.

But I'm wary of attempting to change anything else without further review/feedback

boneskull commented 5 months ago

See https://github.com/statelyai/xstate/issues/4931 for the "other problem" I mentioned above.

davidkpiano commented 5 months ago

There may be some overlap with #4932

boneskull commented 5 months ago

@davidkpiano Yeah. I'll wait until #4932 is merged then retry on top of it.

davidkpiano commented 5 months ago

@boneskull It's merged; please merge main and try again?

boneskull commented 5 months ago

looking

boneskull commented 5 months ago

@davidkpiano OK, I rebased, but changed my strategy. Please read updated issue description

boneskull commented 5 months ago

@davidkpiano I've added a changeset for this. I'm calling it "patch" since it doesn't affect any functionality. Happy to change it