[USD Price] SWR-ise the USD fetching logic #539
Conversation
|
CLA Assistant Lite All Contributors have signed the CLA. |
| () => (strategy && quoteParams ? getGpUsdcPriceResolveOnlyLastCall({ strategy, quoteParams }) : null) | ||
| ) | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
was considering using useMemo being returned directly but I like having the previous price in state remaining vs null if returning useMemo
could be convinced otherwise. the useMemo approach is way cleaner since i dont need useState and can return the error directly from SWR
|
Could you please provide the PR preview link? |
|
|
Hey @W3stside , is it OK that even if I select ETH-WETH or WXDAI-XDAI token pairs, quote requests for WETH-USDC, WXDAI-USDXC are still running in the console for Rinkeby and GC networks? |
|
Also, I noticed, that USD estimation for ETH token can disappear when swap tokens in the From/TO fields |
|
Not sure the caching is working as expected, at least not how I would expect. Screen.Recording.2022-05-16.at.15.05.54.movBTW, what is the cache expiring time? |
Well here you're looking at price quotes. We're talking USD estimation, which isn't being called AFAICT. also any new swap = different swap params = invalidated cache, or did u mean sth else? Not sure the cache time, think it's 2s (2000) which is default |
There was a problem hiding this comment.
Nice initiative.
My only problem with this PR is where you are adding caches.
In principle, the cache should be added in quering of the data. Adding it there feels like mixing logic with cache strategies. The logics for the strategy quering should be fast. Why not just wrapping the API calls with a simple custom hook that uses SWR
| }, [account, baseAmountRaw, sellTokenAddress, sellTokenDecimals, stablecoin, supportedChain]) | ||
|
|
||
| // SWR cache cow usd requests | ||
| const { data: priceResponse, error: errorResponse } = useSWR<CancelableResult<string | null> | null>( |
There was a problem hiding this comment.
how often this cached result will be revalidated? when does it become stale?
There was a problem hiding this comment.
it's currently revalidated using default options, as shown:
i added custom options that are in a comment below
|
USD estimation for ETH token work fine now, but the issue reported in #539 (comment) is still here |
Ah sorry, my mistake. |
|
Filtering for coingecko requests though, they all seem to be failing due to CORS issues |
Seemed to be a temporary fluke locally. All is well now |
|
added SWR options to constants: const SWR_OPTIONS = {
refreshInterval: ms`30s`,
dedupingInterval: ms`10s`,
// don't revalidate data on focus, can cause too many re-renders
// see https://koba04.medium.com/revalidating-options-of-swr-4d9f08bee813
revalidateOnFocus: false,
}
@anxolin @alfetopito thoughts? |
Looks good. |
|
ok im merging this |
| } else { | ||
| price = new Price({ | ||
| baseAmount, | ||
| quoteAmount: stringToCurrency(quote, currency), |
There was a problem hiding this comment.
here is the relevant change that was NOT in 1.14.2 and only added to prod in 1.15.0
Summary
Attempts to SWR-ise the USD querying logic
As a bonus - stops 2 faulty calls to quote happening on initial load - app was querying "USDC" in price quote