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

feat(core): expose UnknownActorRef #4929

Merged
merged 2 commits into from
Jun 21, 2024

Conversation

boneskull
Copy link
Contributor

@boneskull boneskull commented Jun 8, 2024

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.

Copy link

changeset-bot bot commented Jun 8, 2024

🦋 Changeset detected

Latest commit: 54195ec

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

boneskull commented Jun 8, 2024

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

@davidkpiano davidkpiano requested a review from Andarist June 9, 2024 21:52
@davidkpiano
Copy link
Member

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

boneskull commented Jun 10, 2024

@davidkpiano I don't quite understand why the 3rd any in AnyActorRef was removed in dbeafeb (#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
Copy link
Contributor Author

See #4931 for the "other problem" I mentioned above.

@davidkpiano
Copy link
Member

There may be some overlap with #4932

@boneskull
Copy link
Contributor Author

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

@davidkpiano
Copy link
Member

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

@boneskull
Copy link
Contributor Author

looking

@boneskull boneskull changed the title fix(core): make AnyActorRef compatible with getSnapshot() feat(core): expose SomeActorRef Jun 14, 2024
@boneskull boneskull force-pushed the boneskull/fix-any-actor-ref branch from 9c0d042 to d3e2a7b Compare June 14, 2024 19:09
@boneskull
Copy link
Contributor Author

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

@boneskull boneskull force-pushed the boneskull/fix-any-actor-ref branch from d3e2a7b to 8c1226b Compare June 15, 2024 19:55
@boneskull boneskull changed the title feat(core): expose SomeActorRef feat(core): expose UnknownActorRef Jun 15, 2024
@boneskull boneskull force-pushed the boneskull/fix-any-actor-ref branch from 8c1226b to cdbaab6 Compare June 15, 2024 19:59
`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:

```ts
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.
@boneskull
Copy link
Contributor Author

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

@davidkpiano davidkpiano merged commit 417f35a into statelyai:main Jun 21, 2024
1 check passed
@github-actions github-actions bot mentioned this pull request Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants