Skip to content

Actor select #5299

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Actor select #5299

wants to merge 5 commits into from

Conversation

Uniqen
Copy link

@Uniqen Uniqen commented Jun 15, 2025

The select function in @xstate/store is very useful. This PR adds a similar select function to the actor. This is to make the transition from @xstate/store to an xstate machine easier. I've used the same naming of types as in @xstate/store to make it consistent.

I used the tests in @xstate/store as inspiration to what to test.

I hope you find the PR useful! Thanks for an awesome library!

Copy link

changeset-bot bot commented Jun 15, 2025

🦋 Changeset detected

Latest commit: e1f2808

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

This PR includes changesets to release 5 packages
Name Type
xstate Minor
@xstate/react Major
@xstate/solid Major
@xstate/svelte Major
@xstate/vue Major

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

@davidkpiano davidkpiano requested a review from Andarist June 16, 2025 14:49
@@ -1911,6 +1911,12 @@ export interface Subscription {
unsubscribe(): void;
}

export type Selection<TSelected> = Readable<TSelected>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unused, do we need this alias? it probably could be dropped

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good catch! Removing it.

@@ -1990,6 +1996,10 @@ export interface ActorRef<
emitted: TEmitted & (TType extends '*' ? unknown : { type: TType })
) => void
) => Subscription;
select<TSelected, TSnapshot>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TSnapshot is already available in the scope here (it's an existing type parameter of ActorRef), so u dont need it here - only TSelected is needed

Copy link
Author

@Uniqen Uniqen Jun 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get compilation error when I remove TSnapshot and then run pnpm typecheck, see GitHub Actions. Not sure on how to resolve it. Reverting to use TSnapshot for now.

I looked into adding a generic for TSnapshot on Actor, see below. But that ended up in a lot of changes not really in the scope of this PR. @Andarist any suggestions?

export class Actor<TLogic extends AnyActorLogic, TSnapshot extends SnapshotFrom<TLogic>>
  implements
    ActorRef<SnapshotFrom<TLogic>, EventFromLogic<TLogic>, EmittedFrom<TLogic>>
{
  ....
}

@Uniqen Uniqen requested a review from Andarist June 23, 2025 20:01
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.

3 participants