Conversation
executeImmediately flag to PollingControllerPollingController to call _executePoll if no polling is currently active for the key
PollingController to call _executePoll if no polling is currently active for the keyPollingController to immediately call _executePoll if no polling is currently active for the key on start
Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>
There was a problem hiding this comment.
I have only one real comment on a pattern I see used in one test file that isn't used in other test files. Other than that my comments are more observational than actionable. It does seem like we are mostly writing tests a similar way so that's good.
| data: sampleSepoliaTokensChainCache, | ||
| }, | ||
| }); | ||
| controller.stopPollingByPollingToken(pollingToken); |
There was a problem hiding this comment.
Nit: This test seems to do a lot. It's testing startPollingByNetworkClientId, but also stopPollingByPollingToken. I'm wondering if it's worth it to look at this test to see if it can be split up. I realize this is somewhat difficult, because in order to stop polling you have to start it somehow, so these methods seem linked. But I feel like this test would be hard to read for future maintainers. Nothing to do about this now, but something to think about as we increase how many controllers use this pattern.
| @@ -117,7 +126,7 @@ describe('PollingController', () => { | |||
| }); | |||
| }); | |||
| describe('poll', () => { | |||
There was a problem hiding this comment.
Nit: Typically, we use specific names for describes — that is, we name them after the methods that we are testing. Instead, this file seems to name describes more loosely. It seems like this would be confusing for anyone maintaining these tests, particularly poll here, as there is no method called "poll" (leaving the question of how these differ from start and stop). It would be nice to clean this up at some point.
Explanation
Currently when PollingController polling is started via
startPollingByNetworkClientId(), the first poll execution doesn't occur untilintervalLengthtime has passed. Some controllers such as theCurrencyRateControllerwant to execute their polling logic immediately on polling start that way it is available to the UI as soon as possible.This PR adds aThis PR updatesexecuteImmediatelyflag to thePollingController. When set totrue, it causes_executePoll()to immediately be called whenstartPollingByNetworkClientId()is called, subsequently on eachintervalLengthtime period. This value defaults to false.#poll()to call_executePoll()if no polling is currently active for the keyReferences
PollingControllerinCurrencyRateControllerand key by nativeCurrency #1805Changelog
@metamask/polling-controller_executePoll()is called immediately on start if no polling interval is already active for the networkClientId + options combinationChecklist