-
Notifications
You must be signed in to change notification settings - Fork 50
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
Chore/sc 519/upgrade swr to 1 0 #244
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/dux-core-unit/governance-portal-v2/A5eNxX5ZKF8zR3mmmQRiLdTqYTh3 |
This pull request has been linked to Shortcut Story #519: Upgrade SWR to 1.0. |
Codecov Report
@@ Coverage Diff @@
## develop #244 +/- ##
===========================================
+ Coverage 35.34% 36.53% +1.19%
===========================================
Files 58 57 -1
Lines 1610 1593 -17
Branches 520 502 -18
===========================================
+ Hits 569 582 +13
+ Misses 1034 1004 -30
Partials 7 7
Continue to review full report at Codecov.
|
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.
So it seems on the delegates page, if you freshload, it does not load the MKR delegated, but if you switch focus ( change to other app or tab) it does refresh the balances
@@ -1,4 +1,4 @@ | |||
### Link to Clubhouse story | |||
### Link to Shortcut ticket: |
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.
@@ -8,7 +8,9 @@ type DelegateAddressMapResponse = { | |||
}; | |||
|
|||
export const useDelegateAddressMap = (): DelegateAddressMapResponse => { | |||
const { data: delegatesApiResponse, error } = useSWR(`/api/delegates?network=${getNetwork()}`); | |||
const { data: delegatesApiResponse, error } = useSWR(`/api/delegates?network=${getNetwork()}`, null, { | |||
refreshInterval: 0 |
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.
it means no refresh, right?
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.
Exactly
@@ -68,6 +76,8 @@ export const DelegateModal = ({ isOpen, onDismiss, delegate }: Props): JSX.Eleme | |||
const lockTxCreator = () => maker.service('voteDelegate').lock(voteDelegateAddress, mkrToDeposit); | |||
const txId = await trackTransaction(lockTxCreator, 'Depositing MKR', { | |||
mined: txId => { | |||
mutateTotalStaked(); | |||
mutateMkrStaked(); |
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.
If you close the modal does these mutates get triggered?
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.
Not on modal close. When we broadcast the tx, we track it through dai.js's transaction manager (through trackTranscation
). That tracking function provides hooks for when the tx status changes. So as soon as we detect that the tx was mined, that's when those functions trigger.
`/api/executive/analyze-spell/${proposalAddress}?network=${getNetwork()}` | ||
); | ||
|
||
return { | ||
data, | ||
loading: !error && !data, | ||
error | ||
error, | ||
mutate |
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.
nice
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.
Nice catch. Should be fixed now 🔨 |
Link to Clubhouse story
https://app.shortcut.com/dux-makerdao/story/519/upgrade-swr-to-1-0
What does this PR do?
Upgrades to SWR 1.0
Reduces default polling interval
Cuts down on unnecessary calls
Adds mutations (revalidation) where necessary
Steps for testing:
Check all the pages of the app. Send tests transactions. Look for anything funky.
Any additional helpful information?:
Reducing infura calls will be handled in a separate PR
Add a GIF: