diff --git a/apps/cowswap-frontend/src/common/pure/PriceImpactIndicator/index.tsx b/apps/cowswap-frontend/src/common/pure/PriceImpactIndicator/index.tsx index f0e40d719b2..9728855a51e 100644 --- a/apps/cowswap-frontend/src/common/pure/PriceImpactIndicator/index.tsx +++ b/apps/cowswap-frontend/src/common/pure/PriceImpactIndicator/index.tsx @@ -32,7 +32,7 @@ export function PriceImpactIndicator({ priceImpactParams, isBridging = false }: return ( - {priceImpact ? ( + {priceImpact && !priceImpactLoading ? ( {' '} ({ useDerivedTradeState: jest.fn(), })) +jest.mock('modules/tradeQuote', () => ({ + useTradeQuote: jest.fn(), +})) + jest.mock('modules/usdAmount', () => ({ useTradeUsdAmounts: jest.fn(), })) @@ -32,6 +37,7 @@ jest.mock('./logger', () => ({ const mockedUseDerivedTradeState = useDerivedTradeState as jest.MockedFunction const mockedUseTradeUsdAmounts = useTradeUsdAmounts as jest.MockedFunction +const mockedUseTradeQuote = useTradeQuote as jest.MockedFunction function createToken(symbol: string, address: string): Token { return new Token(ChainId.SEPOLIA, address, 18, symbol, symbol) @@ -52,6 +58,11 @@ describe('useFiatValuePriceImpact', () => { inputCurrencyAmount: CurrencyAmount.fromRawAmount(inputToken, 1), outputCurrencyAmount: CurrencyAmount.fromRawAmount(outputToken, 1), } as ReturnType) + + mockedUseTradeQuote.mockReturnValue({ + isLoading: false, + hasParamsChanged: false, + } as ReturnType) }) afterEach(() => { @@ -158,4 +169,37 @@ describe('useFiatValuePriceImpact', () => { expect(result.current).toEqual({ priceImpact: undefined, isLoading: false }) }) + + it('restarts the loading timeout when a new quote begins after timing out', () => { + mockedUseTradeUsdAmounts.mockReturnValue({ + inputAmount: { value: null, isLoading: true }, + outputAmount: { value: null, isLoading: true }, + }) + + const { result, rerender } = renderHook(() => useFiatValuePriceImpact()) + + expect(result.current).toEqual({ priceImpact: undefined, isLoading: true }) + + act(() => { + jest.advanceTimersByTime(15_000) + }) + + // Stale value stays suppressed only until the safety-valve timeout fires + expect(result.current).toEqual({ priceImpact: undefined, isLoading: false }) + + // A fresh quote for the same token pair must re-arm the timeout and suppress the stale value again + mockedUseTradeQuote.mockReturnValue({ + isLoading: true, + hasParamsChanged: true, + } as ReturnType) + rerender() + + expect(result.current).toEqual({ priceImpact: undefined, isLoading: true }) + + act(() => { + jest.advanceTimersByTime(15_000) + }) + + expect(result.current).toEqual({ priceImpact: undefined, isLoading: false }) + }) }) diff --git a/apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.ts b/apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.ts index 62fdc9197d5..f2bb79b364c 100644 --- a/apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.ts +++ b/apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.ts @@ -8,6 +8,7 @@ import { Fraction, Percent } from '@cowprotocol/currency' import ms from 'ms.macro' import { useDerivedTradeState } from 'modules/trade' +import { useTradeQuote } from 'modules/tradeQuote' import { useTradeUsdAmounts } from 'modules/usdAmount' import { useSafeMemo } from 'common/hooks/useSafeMemo' @@ -31,9 +32,19 @@ export function useFiatValuePriceImpact(): { priceImpact: Percent | undefined; i outputAmount: { value: fiatValueOutput, isLoading: outputIsLoading }, } = useTradeUsdAmounts(inputCurrencyAmount, outputCurrencyAmount, inputToken, outputToken) - const isLoading = inputIsLoading || outputIsLoading + const { isLoading: isQuoteLoading, hasParamsChanged: quoteParamsChanged } = useTradeQuote() + + // Trade-quote signals indicate the current output amount is stale (token just changed + // or a fresh quote is in flight). Compute price impact only once the quote catches up, + // otherwise we'd display a huge nonsense % derived from mismatched in/out amounts. + const isLoading = inputIsLoading || outputIsLoading || isQuoteLoading || quoteParamsChanged const [hasLoadingTimedOut, setHasLoadingTimedOut] = useState(false) + // Restart the safety-valve timeout on a token-pair change OR whenever a new quote begins + // (`quoteParamsChanged`). Keying it only off the token pair left `hasLoadingTimedOut` stuck + // true after the first 15s, so the stale-value suppression below never fired again for later + // same-pair repricing. Plain loading flicker (unchanged params) intentionally does not restart + // it, so a stuck quote still times out. useEffect(() => { logPriceImpact.debug(`Price impact timeout reset`) setHasLoadingTimedOut(false) @@ -45,18 +56,26 @@ export function useFiatValuePriceImpact(): { priceImpact: Percent | undefined; i }, PRICE_IMPACT_LOADING_TIMEOUT) return () => clearTimeout(timeoutId) - }, [isTradeSetUp, inputToken, outputToken]) + }, [isTradeSetUp, inputToken, outputToken, quoteParamsChanged]) return useSafeMemo(() => { // Don't calculate price impact if trade is not set up (both trade assets are not set) if (!isTradeSetUp) return null + const stillLoading = isLoading && !hasLoadingTimedOut + + // While a fresh quote is loading, don't expose the stale value at all — consumers + // hide the percentage when `priceImpact` is undefined, leaving just the spinner. + if (stillLoading) { + return { priceImpact: undefined, isLoading: true } + } + const priceImpact = computeFiatValuePriceImpact( fiatValueInput ? FractionUtils.fractionLikeToFraction(fiatValueInput) : null, fiatValueOutput ? FractionUtils.fractionLikeToFraction(fiatValueOutput) : null, ) - return { priceImpact, isLoading: isLoading && !hasLoadingTimedOut } + return { priceImpact, isLoading: false } }, [isTradeSetUp, fiatValueInput, fiatValueOutput, isLoading, hasLoadingTimedOut]) }