From cee3390b7185c4a020e1d8a9890986dbe6e59d0e Mon Sep 17 00:00:00 2001 From: Tina <59578595+tinaszheng@users.noreply.github.com> Date: Wed, 8 Nov 2023 15:14:50 -0500 Subject: [PATCH] fix: skip all quote / pricing requests if window is not visible (#7554) * skip all quote / pricing requests if window is not visible * add unit tests * add ts-ignore comment --- src/hooks/useDebouncedTrade.test.ts | 79 ---------- src/hooks/useDebouncedTrade.ts | 4 +- src/state/routing/usePreviewTrade.ts | 4 +- src/state/routing/useRoutingAPITrade.test.ts | 143 +++++++++++++++++++ src/state/routing/useRoutingAPITrade.ts | 5 +- 5 files changed, 151 insertions(+), 84 deletions(-) delete mode 100644 src/hooks/useDebouncedTrade.test.ts create mode 100644 src/state/routing/useRoutingAPITrade.test.ts diff --git a/src/hooks/useDebouncedTrade.test.ts b/src/hooks/useDebouncedTrade.test.ts deleted file mode 100644 index 4f6c594a20..0000000000 --- a/src/hooks/useDebouncedTrade.test.ts +++ /dev/null @@ -1,79 +0,0 @@ -import { renderHook } from '@testing-library/react' -import { CurrencyAmount, TradeType } from '@uniswap/sdk-core' -import { DAI, USDC_MAINNET } from 'constants/tokens' -import { RouterPreference, TradeState } from 'state/routing/types' -import { usePreviewTrade } from 'state/routing/usePreviewTrade' -import { useRouterPreference } from 'state/user/hooks' -import { mocked } from 'test-utils/mocked' - -import { useRoutingAPITrade } from '../state/routing/useRoutingAPITrade' -import useAutoRouterSupported from './useAutoRouterSupported' -import useDebounce from './useDebounce' -import { useDebouncedTrade } from './useDebouncedTrade' -import useIsWindowVisible from './useIsWindowVisible' - -const USDCAmount = CurrencyAmount.fromRawAmount(USDC_MAINNET, '10000') -const DAIAmount = CurrencyAmount.fromRawAmount(DAI, '10000') - -jest.mock('./useAutoRouterSupported') -jest.mock('./useDebounce') -jest.mock('./useIsWindowVisible') -jest.mock('state/routing/useRoutingAPITrade') -jest.mock('state/routing/usePreviewTrade') -jest.mock('state/user/hooks') - -// helpers to set mock expectations -const expectRouterMock = (state: TradeState) => { - mocked(useRoutingAPITrade).mockReturnValue({ state, trade: undefined }) - mocked(usePreviewTrade).mockReturnValue({ state, trade: undefined }) -} - -beforeEach(() => { - // ignore debounced value - mocked(useDebounce).mockImplementation((value) => value) - - mocked(useIsWindowVisible).mockReturnValue(true) - mocked(useAutoRouterSupported).mockReturnValue(true) - mocked(useRouterPreference).mockReturnValue([RouterPreference.API, () => undefined]) -}) - -describe('#useBestV3Trade ExactIn', () => { - it('does not compute routing api trade when window is not focused', async () => { - mocked(useIsWindowVisible).mockReturnValue(false) - expectRouterMock(TradeState.NO_ROUTE_FOUND) - - const { result } = renderHook(() => useDebouncedTrade(TradeType.EXACT_INPUT, USDCAmount, DAI)) - - expect(useRoutingAPITrade).toHaveBeenCalledWith( - /* skipFetch = */ true, - TradeType.EXACT_INPUT, - USDCAmount, - DAI, - RouterPreference.API, - /* account = */ undefined, - /* inputTax = */ undefined, - /* outputTax = */ undefined - ) - expect(result.current).toEqual({ state: TradeState.NO_ROUTE_FOUND, trade: undefined }) - }) -}) - -describe('#useDebouncedTrade ExactOut', () => { - it('does not compute routing api trade when window is not focused', () => { - mocked(useIsWindowVisible).mockReturnValue(false) - expectRouterMock(TradeState.NO_ROUTE_FOUND) - - const { result } = renderHook(() => useDebouncedTrade(TradeType.EXACT_OUTPUT, DAIAmount, USDC_MAINNET)) - expect(useRoutingAPITrade).toHaveBeenCalledWith( - /* skipFetch = */ true, - TradeType.EXACT_OUTPUT, - DAIAmount, - USDC_MAINNET, - RouterPreference.API, - /* account = */ undefined, - /* inputTax = */ undefined, - /* outputTax = */ undefined - ) - expect(result.current).toEqual({ state: TradeState.NO_ROUTE_FOUND, trade: undefined }) - }) -}) diff --git a/src/hooks/useDebouncedTrade.ts b/src/hooks/useDebouncedTrade.ts index 46ae92ba82..ab90ac434f 100644 --- a/src/hooks/useDebouncedTrade.ts +++ b/src/hooks/useDebouncedTrade.ts @@ -10,7 +10,6 @@ import { useRouterPreference } from 'state/user/hooks' import useAutoRouterSupported from './useAutoRouterSupported' import useDebounce from './useDebounce' -import useIsWindowVisible from './useIsWindowVisible' // Prevents excessive quote requests between keystrokes. const DEBOUNCE_TIME = 350 @@ -71,7 +70,6 @@ export function useDebouncedTrade( } { const { chainId } = useWeb3React() const autoRouterSupported = useAutoRouterSupported() - const isWindowVisible = useIsWindowVisible() const inputs = useMemo<[CurrencyAmount | undefined, Currency | undefined]>( () => [amountSpecified, otherCurrency], @@ -94,7 +92,7 @@ export function useDebouncedTrade( const [routerPreference] = useRouterPreference() - const skipBothFetches = !autoRouterSupported || !isWindowVisible || isWrap + const skipBothFetches = !autoRouterSupported || isWrap const skipRoutingFetch = skipBothFetches || isDebouncing const skipPreviewTradeFetch = skipBothFetches || isPreviewTradeDebouncing diff --git a/src/state/routing/usePreviewTrade.ts b/src/state/routing/usePreviewTrade.ts index cee90197f9..2b58fddcb4 100644 --- a/src/state/routing/usePreviewTrade.ts +++ b/src/state/routing/usePreviewTrade.ts @@ -2,6 +2,7 @@ import { skipToken } from '@reduxjs/toolkit/query/react' import { ChainId, Currency, CurrencyAmount, Percent, TradeType } from '@uniswap/sdk-core' import { ZERO_PERCENT } from 'constants/misc' import { useQuickRouteMainnetEnabled } from 'featureFlags/flags/quickRouteMainnet' +import useIsWindowVisible from 'hooks/useIsWindowVisible' import { useMemo } from 'react' import { useGetQuickRouteQuery, useGetQuickRouteQueryState } from './quickRouteSlice' @@ -78,9 +79,10 @@ export function usePreviewTrade( inputTax, outputTax, }) + const isWindowVisible = useIsWindowVisible() const { isError, data: tradeResult, error, currentData } = useGetQuickRouteQueryState(queryArgs) - useGetQuickRouteQuery(skipFetch ? skipToken : queryArgs, { + useGetQuickRouteQuery(skipFetch || !isWindowVisible ? skipToken : queryArgs, { // If latest quote from cache was fetched > 2m ago, instantly repoll for another instead of waiting for next poll period refetchOnMountOrArgChange: 2 * 60, }) diff --git a/src/state/routing/useRoutingAPITrade.test.ts b/src/state/routing/useRoutingAPITrade.test.ts new file mode 100644 index 0000000000..95151b8754 --- /dev/null +++ b/src/state/routing/useRoutingAPITrade.test.ts @@ -0,0 +1,143 @@ +import { skipToken } from '@reduxjs/toolkit/query/react' +import { renderHook } from '@testing-library/react' +import { CurrencyAmount, TradeType } from '@uniswap/sdk-core' +import { AVERAGE_L1_BLOCK_TIME } from 'constants/chainInfo' +import { ZERO_PERCENT } from 'constants/misc' +import { USDC_MAINNET } from 'constants/tokens' +import { useUniswapXDefaultEnabled } from 'featureFlags/flags/uniswapXDefault' +import { useUniswapXEthOutputEnabled } from 'featureFlags/flags/uniswapXEthOutput' +import { useUniswapXExactOutputEnabled } from 'featureFlags/flags/uniswapXExactOutput' +import { useUniswapXSyntheticQuoteEnabled } from 'featureFlags/flags/uniswapXUseSyntheticQuote' +import { useFeesEnabled } from 'featureFlags/flags/useFees' +import useIsWindowVisible from 'hooks/useIsWindowVisible' +import ms from 'ms' +import { GetQuoteArgs, INTERNAL_ROUTER_PREFERENCE_PRICE, RouterPreference } from 'state/routing/types' +import { useRouterPreference, useUserDisabledUniswapX, useUserOptedOutOfUniswapX } from 'state/user/hooks' +import { ETH_MAINNET } from 'test-utils/constants' +import { mocked } from 'test-utils/mocked' + +import { useGetQuoteQuery, useGetQuoteQueryState } from './slice' +import { useRoutingAPITrade } from './useRoutingAPITrade' +import { currencyAddressForSwapQuote } from './utils' + +const USDCAmount = CurrencyAmount.fromRawAmount(USDC_MAINNET, '10000') + +jest.mock('hooks/useIsWindowVisible') +jest.mock('state/routing/usePreviewTrade') +jest.mock('./slice', () => { + return { + useGetQuoteQuery: jest.fn(), + useGetQuoteQueryState: jest.fn(), + } +}) +jest.mock('state/user/hooks') +jest.mock('featureFlags/flags/uniswapXUseSyntheticQuote') +jest.mock('featureFlags/flags/uniswapXEthOutput') +jest.mock('featureFlags/flags/uniswapXExactOutput') +jest.mock('featureFlags/flags/uniswapXDefault') +jest.mock('featureFlags/flags/useFees') + +beforeEach(() => { + mocked(useIsWindowVisible).mockReturnValue(true) + mocked(useRouterPreference).mockReturnValue([RouterPreference.API, () => undefined]) + mocked(useUniswapXSyntheticQuoteEnabled).mockReturnValue(false) + mocked(useUserDisabledUniswapX).mockReturnValue(false) + mocked(useUserOptedOutOfUniswapX).mockReturnValue(false) + mocked(useUniswapXEthOutputEnabled).mockReturnValue(false) + mocked(useUniswapXExactOutputEnabled).mockReturnValue(false) + mocked(useUniswapXDefaultEnabled).mockReturnValue(false) + mocked(useFeesEnabled).mockReturnValue(true) + // @ts-ignore we dont use the response from this hook in useRoutingAPITrade so fine to mock as undefined + mocked(useGetQuoteQuery).mockReturnValue(undefined) + mocked(useGetQuoteQueryState).mockReturnValue({ + refetch: jest.fn(), + isError: false, + data: undefined, + error: false, + currentData: undefined, + }) +}) + +const MOCK_ARGS: GetQuoteArgs = { + account: undefined, + amount: USDCAmount.quotient.toString(), + tokenInAddress: currencyAddressForSwapQuote(USDCAmount.currency), + tokenInChainId: USDCAmount.currency.chainId, + tokenInDecimals: USDCAmount.currency.wrapped.decimals, + tokenInSymbol: USDCAmount.currency.wrapped.symbol, + tokenOutAddress: currencyAddressForSwapQuote(ETH_MAINNET), + tokenOutChainId: ETH_MAINNET.wrapped.chainId, + tokenOutDecimals: ETH_MAINNET.wrapped.decimals, + tokenOutSymbol: ETH_MAINNET.wrapped.symbol, + routerPreference: RouterPreference.API, + tradeType: TradeType.EXACT_INPUT, + needsWrapIfUniswapX: USDCAmount.currency.isNative, + uniswapXForceSyntheticQuotes: false, + userDisabledUniswapX: false, + userOptedOutOfUniswapX: false, + uniswapXEthOutputEnabled: false, + uniswapXExactOutputEnabled: false, + isUniswapXDefaultEnabled: false, + sendPortionEnabled: true, + inputTax: ZERO_PERCENT, + outputTax: ZERO_PERCENT, +} + +describe('#useRoutingAPITrade ExactIn', () => { + it('does not call routing api when window is not focused for quote requests', () => { + mocked(useIsWindowVisible).mockReturnValue(false) + + const { result } = renderHook(() => + useRoutingAPITrade(false, TradeType.EXACT_INPUT, USDCAmount, ETH_MAINNET, RouterPreference.API) + ) + + expect(useGetQuoteQuery).toHaveBeenCalledWith(skipToken, { + pollingInterval: AVERAGE_L1_BLOCK_TIME, + refetchOnMountOrArgChange: 2 * 60, + }) + expect(result.current?.trade).toEqual(undefined) + }) + + it('does call routing api when window is focused for quote requests', () => { + mocked(useIsWindowVisible).mockReturnValue(true) + + renderHook(() => useRoutingAPITrade(false, TradeType.EXACT_INPUT, USDCAmount, ETH_MAINNET, RouterPreference.API)) + + expect(useGetQuoteQuery).toHaveBeenCalledWith(MOCK_ARGS, { + pollingInterval: AVERAGE_L1_BLOCK_TIME, + refetchOnMountOrArgChange: 2 * 60, + }) + }) +}) + +describe('#useRoutingAPITrade pricing', () => { + it('does not call routing api when window is not focused for price requests', () => { + mocked(useIsWindowVisible).mockReturnValue(false) + + const { result } = renderHook(() => + useRoutingAPITrade(false, TradeType.EXACT_INPUT, USDCAmount, ETH_MAINNET, INTERNAL_ROUTER_PREFERENCE_PRICE) + ) + + expect(useGetQuoteQuery).toHaveBeenCalledWith(skipToken, { + pollingInterval: ms(`1m`), + refetchOnMountOrArgChange: 2 * 60, + }) + expect(result.current?.trade).toEqual(undefined) + }) + + it('does call routing api when window is focused for pricing requests', () => { + mocked(useIsWindowVisible).mockReturnValue(true) + + renderHook(() => + useRoutingAPITrade(false, TradeType.EXACT_INPUT, USDCAmount, ETH_MAINNET, INTERNAL_ROUTER_PREFERENCE_PRICE) + ) + + expect(useGetQuoteQuery).toHaveBeenCalledWith( + { ...MOCK_ARGS, sendPortionEnabled: false, routerPreference: INTERNAL_ROUTER_PREFERENCE_PRICE }, + { + pollingInterval: ms(`1m`), + refetchOnMountOrArgChange: 2 * 60, + } + ) + }) +}) diff --git a/src/state/routing/useRoutingAPITrade.ts b/src/state/routing/useRoutingAPITrade.ts index 38a29be163..442ec4de89 100644 --- a/src/state/routing/useRoutingAPITrade.ts +++ b/src/state/routing/useRoutingAPITrade.ts @@ -2,6 +2,7 @@ import { skipToken } from '@reduxjs/toolkit/query/react' import { Currency, CurrencyAmount, Percent, TradeType } from '@uniswap/sdk-core' import { AVERAGE_L1_BLOCK_TIME } from 'constants/chainInfo' import { ZERO_PERCENT } from 'constants/misc' +import useIsWindowVisible from 'hooks/useIsWindowVisible' import { useRoutingAPIArguments } from 'lib/hooks/routing/useRoutingAPIArguments' import ms from 'ms' import { useMemo } from 'react' @@ -92,9 +93,11 @@ export function useRoutingAPITrade( inputTax, outputTax, }) + // skip all pricing and quote requests if the window is not focused + const isWindowVisible = useIsWindowVisible() const { isError, data: tradeResult, error, currentData } = useGetQuoteQueryState(queryArgs) - useGetQuoteQuery(skipFetch ? skipToken : queryArgs, { + useGetQuoteQuery(skipFetch || !isWindowVisible ? skipToken : queryArgs, { // Price-fetching is informational and costly, so it's done less frequently. pollingInterval: routerPreference === INTERNAL_ROUTER_PREFERENCE_PRICE ? ms(`1m`) : AVERAGE_L1_BLOCK_TIME, // If latest quote from cache was fetched > 2m ago, instantly repoll for another instead of waiting for next poll period