-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[core] Add waitFor
helper
#3190
Conversation
🦋 Changeset detectedLatest commit: 281f1fe The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
CodeSee Review Map:Review in an interactive map View more CodeSee Maps Legend |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 281f1fe:
|
@@ -1090,7 +1090,7 @@ export class Interpreter< | |||
}) | |||
.start(); | |||
|
|||
return actor; | |||
return actor as any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this change needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests were failing without it locally
sub?.unsubscribe(); | ||
}; | ||
|
||
const sub = actorRef.subscribe({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about actors that do not emit their current state at subscription time? or are actors supposed to always do that? Note that today an actor made out of an observable might not emit immediately because the emit time is controlled by the observable itself
Should we run predicate with .getSnapshot()
first? With that I'm a little bit worried about handling non-started interpreters as they would instantiate their .initialState
and that is not a pure operation right now 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, let's have the behavior as-is - if actors (like observables) do not emit state upon subscription, this utility should respect that.
That avoids the impure initial state problem, too 👍
…ter (#3200) * Fixed `waitFor` hanging till timeout with final state matching the predicate * Fixed `waitFor` hanging till timeout with final state not matching the predicate * Fixed spammy sends in the waitFor test file
[core] Add `waitFor` helper
This PR adds the opt-in
waitFor(...)
helper function, that asynchronously waits for the emitted value of an actor to satisfy a predicate before returning that value.See discussion here: https://discord.com/channels/795785288994652170/955085268677963776
Basic usage:
cc. @mattpocock @gavination
TODO: