feat: Add subscribeOnce and waitUntil methods#8575
feat: Add subscribeOnce and waitUntil methods#8575FrederikBolding wants to merge 7 commits intomainfrom
subscribeOnce and waitUntil methods#8575Conversation
| const promise = messenger.waitUntil('Fixture:message'); | ||
| messenger.publish('Fixture:message', 'foo'); | ||
|
|
||
| expect(await promise).toBe('foo'); |
There was a problem hiding this comment.
When not using a selector this just returns the first parameter of the event. The other option is returning the entire array, but that feels a bit complicated 🤔
There was a problem hiding this comment.
Hmm, why does it feel complicated? If it's easy to support, I say we give consumers the whole payload.
| selector: SelectorFunction<Event, EventType, SelectorReturnValue>, | ||
| condition?: (value: SelectorReturnValue) => boolean, |
There was a problem hiding this comment.
What if you want to specify a condition, but not a selector? Like this for example:
// Subscribe to `KeyringController:unlock` and wait if condition is not met
await messenger.waitUntil(
'KeyringController:unlock',
() => !engineContext.KeyringController.isUnlocked()
);Maybe it's easier to have an options object, so you could do something like this:
await messenger.waitUntil(
'KeyringController:unlock',
{
condition: () => !engineContext.KeyringController.isUnlocked()
}
);There was a problem hiding this comment.
I'm confused by this question, if KeyringController.isUnlocked is true you wouldn't want to wait for unlock in the first place. The condition is meant as a condition on the value itself and is only checked when an event fires, e.g.
await messenger.waitUntil(
'TransactionController:transactionSuccess',
(tx) => tx.hash === 'foo'
);There was a problem hiding this comment.
That being said, it would be nice if you could pass a condition without a selector. Potentially worth doing an options bag regardless.
There was a problem hiding this comment.
Then I was misunderstanding what the condition is supposed to do 😅
There was a problem hiding this comment.
I was under the impression that these would be equivalent:
await messenger.waitUntil(
'KeyringController:unlock',
{
condition: () => !engineContext.KeyringController.isUnlocked()
}
);and
if (!engineContext.KeyringController.isUnlocked()) {
await messenger.waitUntil('KeyringController:unlock');
}There was a problem hiding this comment.
I don't see an obvious way to solve for both that and the TransactionController example from above 🤔
There was a problem hiding this comment.
Yeah, we'd need to introduce a third property. But in hindsight, I'm not sure if it's necessary as the if statement seems clean enough.
There was a problem hiding this comment.
Certainly better than what we have today
| | ((value: SelectorReturnValue) => boolean); | ||
| }, | ||
| ): Promise<SelectorReturnValue | ExtractEventPayload<Event, EventType>[0]> { | ||
| return new Promise((resolve) => { |
There was a problem hiding this comment.
How we expect waitUntil to be used? In Slack you gave this example:
getUnlockPromise: () => {
if (engineContext.KeyringController.isUnlocked()) {
return Promise.resolve();
}
return new Promise<void>((resolve) => {
controllerMessenger.subscribeOnceIf(
'KeyringController:unlock',
resolve,
() => true,
);
});
},Is the idea that with this method you could shorten this to the following?
getUnlockPromise: () => {
return await controllerMessenger.waitUntil(
'KeyringController:unlock',
{
condition: () => {
return engineContext.KeyringController.isUnlocked();
}
}
);
},Or is it that we would expect people to write this instead?
getUnlockPromise: () => {
return await controllerMessenger.waitUntil(
'KeyringController:stateChange',
{
selector: (state) => state.isUnlocked,
}
);
},There was a problem hiding this comment.
Either way would we want to check the condition beforehand to make sure is not true before subscribing? (I guess the same goes for subscribeOnce as well.)
Explanation
Add
subscribeOnceandwaitUntilutility methods toMessenger. These are useful for subscribing to and waiting for a certain event to fire without wanting to listen for more than one invocation.Additionally the methods support a
conditionparameter which can be used as a predicate for whether the event should be consumed in the first place.A similar approach is already in-use on mobile, this is a "native" replacement.
References
https://consensyssoftware.atlassian.net/browse/WPC-985
Checklist