fix(bridge): hide stale price impact while a new quote is loading

apps/cowswap-frontend/src/common/pure/PriceImpactIndicator/index.tsx

@@ -32,7 +32,7 @@ export function PriceImpactIndicator({ priceImpactParams, isBridging = false }:
 
   return (
     <span>
-      {priceImpact ? (
+      {priceImpact && !priceImpactLoading ? (
         <PriceImpactWrapper priceImpact$={priceImpact} isBridging$={isBridging}>
           {' '}
           <HoverTooltip

apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.test.ts

@@ -4,6 +4,7 @@ import { CurrencyAmount, Token } from '@cowprotocol/currency'
 import { act, renderHook } from '@testing-library/react'
 
 import { useDerivedTradeState } from 'modules/trade'
+import { useTradeQuote } from 'modules/tradeQuote'
 import { useTradeUsdAmounts } from 'modules/usdAmount'
 
 import { useFiatValuePriceImpact } from './useFiatValuePriceImpact'
@@ -17,6 +18,10 @@ jest.mock('modules/trade', () => ({
   useDerivedTradeState: jest.fn(),
 }))
 
+jest.mock('modules/tradeQuote', () => ({
+  useTradeQuote: jest.fn(),
+}))
+
 jest.mock('modules/usdAmount', () => ({
   useTradeUsdAmounts: jest.fn(),
 }))
@@ -32,11 +37,26 @@ jest.mock('./logger', () => ({
 
 const mockedUseDerivedTradeState = useDerivedTradeState as jest.MockedFunction<typeof useDerivedTradeState>
 const mockedUseTradeUsdAmounts = useTradeUsdAmounts as jest.MockedFunction<typeof useTradeUsdAmounts>
+const mockedUseTradeQuote = useTradeQuote as jest.MockedFunction<typeof useTradeQuote>
 
 function createToken(symbol: string, address: string): Token {
   return new Token(ChainId.SEPOLIA, address, 18, symbol, symbol)
 }
 
+// `fetchStartTimestamp` bumps on every genuine quote request and is what the hook keys its
+// safety-valve timeout reset off of, so tests set it explicitly per quote.
+function tradeQuoteState(params: {
+  isLoading: boolean
+  hasParamsChanged: boolean
+  fetchStartTimestamp: number
+}): ReturnType<typeof useTradeQuote> {
+  return {
+    isLoading: params.isLoading,
+    hasParamsChanged: params.hasParamsChanged,
+    fetchParams: { fetchStartTimestamp: params.fetchStartTimestamp },
+  } as unknown as ReturnType<typeof useTradeQuote>
+}
+
 describe('useFiatValuePriceImpact', () => {
   const inputToken = createToken('ETH', '0x0000000000000000000000000000000000000001')
   const outputToken = createToken('COW', '0x0000000000000000000000000000000000000002')
@@ -52,6 +72,10 @@ describe('useFiatValuePriceImpact', () => {
       inputCurrencyAmount: CurrencyAmount.fromRawAmount(inputToken, 1),
       outputCurrencyAmount: CurrencyAmount.fromRawAmount(outputToken, 1),
     } as ReturnType<typeof useDerivedTradeState>)
+
+    mockedUseTradeQuote.mockReturnValue(
+      tradeQuoteState({ isLoading: false, hasParamsChanged: false, fetchStartTimestamp: 1 }),
+    )
   })
 
   afterEach(() => {
@@ -158,4 +182,75 @@ 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(
+      tradeQuoteState({ isLoading: true, hasParamsChanged: true, fetchStartTimestamp: 2 }),
+    )
+    rerender()
+
+    expect(result.current).toEqual({ priceImpact: undefined, isLoading: true })
+
+    act(() => {
+      jest.advanceTimersByTime(15_000)
+    })
+
+    expect(result.current).toEqual({ priceImpact: undefined, isLoading: false })
+  })
+
+  it('re-arms the timeout when a second same-pair quote starts while params are already changed', () => {
+    mockedUseTradeUsdAmounts.mockReturnValue({
+      inputAmount: { value: null, isLoading: false },
+      outputAmount: { value: null, isLoading: false },
+    })
+
+    // First changed-params quote for the pair is in flight
+    mockedUseTradeQuote.mockReturnValue(
+      tradeQuoteState({ isLoading: true, hasParamsChanged: true, fetchStartTimestamp: 1 }),
+    )
+
+    const { result, rerender } = renderHook(() => useFiatValuePriceImpact())
+
+    expect(result.current).toEqual({ priceImpact: undefined, isLoading: true })
+
+    act(() => {
+      jest.advanceTimersByTime(15_000)
+    })
+
+    // Safety valve fired
+    expect(result.current).toEqual({ priceImpact: undefined, isLoading: false })
+
+    // A SECOND changed-params quote for the same pair starts: `hasParamsChanged` stays true
+    // (true -> true), only the per-fetch timestamp advances. This must still re-arm the timeout
+    // and keep the stale value suppressed, which keying off the boolean alone failed to do.
+    mockedUseTradeQuote.mockReturnValue(
+      tradeQuoteState({ isLoading: true, hasParamsChanged: true, fetchStartTimestamp: 2 }),
+    )
+    rerender()
+
+    expect(result.current).toEqual({ priceImpact: undefined, isLoading: true })
+
+    act(() => {
+      jest.advanceTimersByTime(15_000)
+    })
+
+    expect(result.current).toEqual({ priceImpact: undefined, isLoading: false })
+  })
 })

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,23 @@ 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, fetchParams } = useTradeQuote()
+
+  // Bumps on every genuine quote request (see `doQuotePolling`). Used to re-arm the timeout below.
+  const quoteFetchStartTimestamp = fetchParams?.fetchStartTimestamp
+
+  // 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 request begins
+  // (`quoteFetchStartTimestamp`, which bumps per fetch). Keying it off the `quoteParamsChanged`
+  // boolean instead left `hasLoadingTimedOut` stuck true once it had timed out: a second changed-
+  // params quote for the same pair keeps the flag `true`, so the effect never re-ran and the stale
+  // value rendered immediately. The per-fetch timestamp re-arms on every genuinely new quote, while
+  // plain loading flicker (no new fetch) still lets a stuck quote time out.
   useEffect(() => {
     logPriceImpact.debug(`Price impact timeout reset`)
     setHasLoadingTimedOut(false)
@@ -45,18 +60,26 @@ export function useFiatValuePriceImpact(): { priceImpact: Percent | undefined; i
     }, PRICE_IMPACT_LOADING_TIMEOUT)
 
     return () => clearTimeout(timeoutId)
-  }, [isTradeSetUp, inputToken, outputToken])
+  }, [isTradeSetUp, inputToken, outputToken, quoteFetchStartTimestamp])
 
   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])
 }