(Feature) Safe owners not loaded#1710
Conversation
…to feature/safe-owners-not-loaded
|
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
fernandomg
left a comment
fernandomg
left a comment
There was a problem hiding this comment.
Can you fix the exhaustive-deps warning?
src/logic/safe/hooks/useLoadSafe.tsx
Outdated
| dispatch(fetchTransactions(safeAddress)) | ||
| return dispatch(addViewedSafe(safeAddress)) | ||
| dispatch(addViewedSafe(safeAddress)) | ||
| setSafeLoaded(true) |
There was a problem hiding this comment.
@Agupane, help me understand this.
How I see it is:
dispatch(fetchLatestsMasterContractVersion())
// here we're waiting for `fetchLatestsMasterContractVersion` to resolve
.then(() => {
// after the previous promise resolution we dispatch two different actions
dispatch(fetchSafe(safeAddress)) // !!! - I understand this as the most important
return dispatch(fetchSafeTokens(safeAddress))
})
// we wait for `fetchSafeTokens` action to resolve
.then(() => {
/*
...
*/
setSafeLoaded(true) // !!! - basically, we're waiting for `fetchSafeTokens` to resolve to switch the flag
return
})How I see it (and how I understand it), we may face a race condition if fetchSafe takes too long to resolve.
Isn't it?
There was a problem hiding this comment.
As far as I investigated yes but it's not always simple to get that status
There was a problem hiding this comment.
I updated the code to be sure that fetchSafe is executed in order to update the flag, seems that the problem is with multiple redux actions executing at the same time, at least at know, is the best solution I found
…into feature/safe-owners-not-loaded
fixed thanks |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
|
Tried to load a few different safes and check the "read only" mode doesn't stay for long Looks good to me. I'll recheck this with the regression |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
5 participants
Closes #1685 by:
useLoadSafeto know if the safe was loaded or not and using that to avoid scheduling safe update before it is loaded, that caused a race condition betweenuseSafeScheduledUpdatesanduseLoadSafethat "removed" the owners