Skip to content

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: 5ba73cb

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>>
{
  ....
}

Copy link
Author

Choose a reason for hiding this comment

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

@Andarist Any idea how to resolve this without a major refactor? Is it required in order to merge the PR?

@Uniqen Uniqen requested a review from Andarist June 23, 2025 20:01
@Uniqen
Copy link
Author

Uniqen commented Aug 14, 2025

@Andarist @davidkpiano
Hi! I just wanted to check in on this PR to see if it’s relevant to you or if there’s anything I can do to help move it forward? I’m happy to make changes if needed.

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