feat: query transactions on networks independently #5432
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5432 +/- ##
==========================================
+ Coverage 86.40% 86.44% +0.03%
==========================================
Files 761 761
Lines 31359 31430 +71
Branches 5389 5409 +20
==========================================
+ Hits 27097 27169 +72
+ Misses 4031 4030 -1
Partials 231 231
... and 13 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
This reverts commit bae46e3.
| }, {}) | ||
| for (const promise of promises) { | ||
| const { networkId, result } = await promise | ||
| yield { [networkId]: result } |
There was a problem hiding this comment.
If I understand correctly, this would start all the requests in parallel and then serially wait for requests to complete one after the other right? If so, if we have a slow running request at the first one, it would block all subsequent requests right? And if one of them fails, wouldn't all subsequent requests be ignored even if they succeed? Wonder if we can do a non generator based solution where we just invoke some action that can update tx feed for a single n/w
There was a problem hiding this comment.
All the requests are started in parallel within the map and you are correct that a slow request would block the subsequent requests from being processed; however, they will not be removed from the activeRequests state. During the next polling cycle, the slower request will be skipped, if it's listed as having an active request, and the other networks will be processed accordingly.
handleResult currently updates the transaction feed for a single network; however, it's called from two hooks and has some heavy lifting interacting fetchedResult state. Dispatching it from queryTransactionsFeed might be a bit complicated when we have to deal with empty pages and possible refetches.
There was a problem hiding this comment.
@satish-ravi I think this is now Fixed in #5528 which uses an alternative approach.
|
Closing in favor of #5528. |
### Description Alternative implementation of #5432 which addresses the comments raised in #5432 (comment) passing state and setState to the `queryTransactionsFeed` function. ### Test plan - [x] Tested locally on iOS - [x] Tested locally on Android - [x] Unit tests updated ### Related issues - Fixes ACT-1186 ### Backwards compatibility Yes ### Network scalability If a new NetworkId and/or Network are added in the future, the changes in this PR will: - [x] Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)
Description
PR for independent network queries;
useFetchTransactionsinsrc/transactions/feed/queryHelper.tshas a lot going on to manage the transaction state. In the future, something like @tanstack/react-query might serve us better, but is outside of the scope of this PR.Video
Screen.Recording.2024-05-17.at.3.19.30.PM.mov
Screen.Recording.2024-05-17.at.3.14.28.PM.mov
Test plan
prerequisite: add your wallet address to the overrides for
base_and_polygon_targetingin Statsig.Related issues
Backwards compatibility
Yes
Network scalability
If a new NetworkId and/or Network are added in the future, the changes in this PR will: