From e310f040341ac0b5c32d620d114a646d19cea17e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20G=C5=82ownia?= Date: Tue, 27 Sep 2022 20:48:53 +0200 Subject: [PATCH] chore: set default fetchPolicy, handle subscription errors like query errors, timeout unsubscribe (#1482) * chore: set default fetchPolicy, handle subscription errors like query errors,add unsubscribe timeout * chore: improve no data handling in fills, ordes and trades * chore: make reset delay optional, fix pagination and useOrderListData spec --- .../console-lite-grid/console-lite-grid.tsx | 16 ++++++-- .../portfolio/accounts/accounts.tsx | 2 +- .../components/portfolio/orders/orders.tsx | 3 +- .../portfolio/positions/positions-asset.tsx | 2 +- .../simple-market-list/simple-market-list.tsx | 2 +- libs/fills/src/lib/fills-manager.tsx | 3 +- libs/fills/src/lib/use-fills-list.ts | 20 ++++++---- libs/market-list/src/lib/markets-provider.ts | 1 + .../order-list-manager/order-list-manager.tsx | 3 +- .../use-order-list-data.spec.ts | 18 ++++----- .../order-list-manager/use-order-list-data.ts | 20 ++++++---- .../lib/components/order-list/order-list.tsx | 10 ++--- .../src/lib/generic-data-provider.spec.ts | 14 +++---- .../src/lib/generic-data-provider.ts | 40 ++++++++++++++----- libs/react-helpers/src/lib/pagination.ts | 13 ++++-- libs/trades/src/lib/trades-container.tsx | 23 ++++++----- 16 files changed, 118 insertions(+), 72 deletions(-) diff --git a/apps/console-lite/src/app/components/console-lite-grid/console-lite-grid.tsx b/apps/console-lite/src/app/components/console-lite-grid/console-lite-grid.tsx index e3c176ffd..8016ca4e3 100644 --- a/apps/console-lite/src/app/components/console-lite-grid/console-lite-grid.tsx +++ b/apps/console-lite/src/app/components/console-lite-grid/console-lite-grid.tsx @@ -15,19 +15,30 @@ import type { TabToNextCellParams, CellKeyDownEvent, FullWidthCellKeyDownEvent, + IGetRowsParams, + IDatasource, } from 'ag-grid-community'; import { NO_DATA_MESSAGE } from '../../constants'; import * as constants from '../simple-market-list/constants'; +export interface GetRowsParams + extends Omit { + successCallback(rowsThisBlock: T[], lastRow?: number): void; +} + +export interface Datasource extends IDatasource { + getRows(params: GetRowsParams): void; +} interface Props extends GridOptions { - data?: T[]; + rowData?: T[]; + datasource?: Datasource; handleRowClicked?: (event: { data: T }) => void; components?: Record; classNamesParam?: string | string[]; } const ConsoleLiteGrid = ( - { data, handleRowClicked, getRowId, classNamesParam, ...props }: Props, + { handleRowClicked, getRowId, classNamesParam, ...props }: Props, ref?: React.Ref ) => { const { isMobile, screenSize } = useScreenDimensions(); @@ -70,7 +81,6 @@ const ConsoleLiteGrid = ( return ( { noDataMessage={NO_DATA_MESSAGE} > - data={data as AccountObj[]} + rowData={data as AccountObj[]} columnDefs={columnDefs} defaultColDef={defaultColDef} components={{ PriceCell }} diff --git a/apps/console-lite/src/app/components/portfolio/orders/orders.tsx b/apps/console-lite/src/app/components/portfolio/orders/orders.tsx index a94887851..30a639c41 100644 --- a/apps/console-lite/src/app/components/portfolio/orders/orders.tsx +++ b/apps/console-lite/src/app/components/portfolio/orders/orders.tsx @@ -56,7 +56,8 @@ const OrdersManager = () => { > ref={gridRef} - rowModelType="infinite" + rowModelType={data?.length ? 'infinite' : 'clientSide'} + rowData={data?.length ? undefined : []} datasource={{ getRows }} onBodyScrollEnd={onBodyScrollEnd} onBodyScroll={onBodyScroll} diff --git a/apps/console-lite/src/app/components/portfolio/positions/positions-asset.tsx b/apps/console-lite/src/app/components/portfolio/positions/positions-asset.tsx index fc9dc194d..4ada5a6c6 100644 --- a/apps/console-lite/src/app/components/portfolio/positions/positions-asset.tsx +++ b/apps/console-lite/src/app/components/portfolio/positions/positions-asset.tsx @@ -47,7 +47,7 @@ const PositionsAsset = ({ partyId, assetSymbol }: Props) => { defaultColDef={defaultColDef} getRowId={getRowId} rowModelType={data?.length ? 'infinite' : 'clientSide'} - data={data?.length ? undefined : []} + rowData={data?.length ? undefined : []} datasource={{ getRows }} components={{ PriceFlashCell }} /> diff --git a/apps/console-lite/src/app/components/simple-market-list/simple-market-list.tsx b/apps/console-lite/src/app/components/simple-market-list/simple-market-list.tsx index b2e44d45e..91661fe74 100644 --- a/apps/console-lite/src/app/components/simple-market-list/simple-market-list.tsx +++ b/apps/console-lite/src/app/components/simple-market-list/simple-market-list.tsx @@ -67,7 +67,7 @@ const SimpleMarketList = () => { classNamesParam="mb-32 min-h-[300px]" columnDefs={columnDefs} - data={localData} + rowData={localData} defaultColDef={defaultColDef} handleRowClicked={handleRowClicked} /> diff --git a/libs/fills/src/lib/fills-manager.tsx b/libs/fills/src/lib/fills-manager.tsx index b1bdc0383..16aa0f837 100644 --- a/libs/fills/src/lib/fills-manager.tsx +++ b/libs/fills/src/lib/fills-manager.tsx @@ -33,8 +33,9 @@ export const FillsManager = ({ partyId }: FillsManagerProps) => { diff --git a/libs/fills/src/lib/use-fills-list.ts b/libs/fills/src/lib/use-fills-list.ts index 488037399..4dab0550a 100644 --- a/libs/fills/src/lib/use-fills-list.ts +++ b/libs/fills/src/lib/use-fills-list.ts @@ -47,17 +47,21 @@ export const useFillsList = ({ partyId, gridRef, scrolledToTop }: Props) => { if (!gridRef.current?.api) { return false; } - if (!scrolledToTop.current) { - const createdAt = dataRef.current?.[0]?.node.createdAt; - if (createdAt) { - newRows.current += delta.filter( - (trade) => trade.createdAt > createdAt - ).length; + if (dataRef.current?.length) { + if (!scrolledToTop.current) { + const createdAt = dataRef.current?.[0]?.node.createdAt; + if (createdAt) { + newRows.current += delta.filter( + (trade) => trade.createdAt > createdAt + ).length; + } } + dataRef.current = data; + gridRef.current.api.refreshInfiniteCache(); + return true; } dataRef.current = data; - gridRef.current.api.refreshInfiniteCache(); - return true; + return false; }, [gridRef, scrolledToTop] ); diff --git a/libs/market-list/src/lib/markets-provider.ts b/libs/market-list/src/lib/markets-provider.ts index 007e639c2..4a354be00 100644 --- a/libs/market-list/src/lib/markets-provider.ts +++ b/libs/market-list/src/lib/markets-provider.ts @@ -83,6 +83,7 @@ export const marketsProvider = makeDataProvider< >({ query: MARKET_LIST_QUERY, getData, + fetchPolicy: 'cache-first', }); export const activeMarketsProvider = makeDerivedDataProvider( diff --git a/libs/orders/src/lib/components/order-list-manager/order-list-manager.tsx b/libs/orders/src/lib/components/order-list-manager/order-list-manager.tsx index 4b3b40e14..74116ab4d 100644 --- a/libs/orders/src/lib/components/order-list-manager/order-list-manager.tsx +++ b/libs/orders/src/lib/components/order-list-manager/order-list-manager.tsx @@ -34,7 +34,8 @@ export const OrderListManager = ({ partyId }: OrderListManagerProps) => { { expect(mockRefreshAgGridApi).toHaveBeenCalled(); }); - it('methods for pagination should works', async () => { + it('methods for pagination should work', async () => { const successCallback = jest.fn(); mockData = [ { @@ -114,12 +114,10 @@ describe('useOrderListData Hook', () => { }, } as unknown as Orders_party_ordersConnection_edges, ]; - mockDataProviderData = { - ...mockDataProviderData, + Object.assign(mockDataProviderData, { data: mockData, loading: false, - totalCount: 4, - }; + }); const mockDelta = [ { node: { @@ -142,8 +140,6 @@ describe('useOrderListData Hook', () => { } ); - updateMock({ data: mockNextData, delta: mockDelta }); - const getRowsParams = { successCallback, failCallback: jest.fn(), @@ -152,12 +148,14 @@ describe('useOrderListData Hook', () => { } as unknown as IGetRowsParams; await waitFor(async () => { - await result.current.getRows(getRowsParams); + const promise = result.current.getRows(getRowsParams); + updateMock({ data: mockNextData, delta: mockDelta }); + await promise; }); expect(loadMock).toHaveBeenCalled(); expect(successCallback).toHaveBeenLastCalledWith( mockDelta.map((item) => item.node), - 4 + -1 ); }); }); diff --git a/libs/orders/src/lib/components/order-list-manager/use-order-list-data.ts b/libs/orders/src/lib/components/order-list-manager/use-order-list-data.ts index 746e5ec46..d56e09b45 100644 --- a/libs/orders/src/lib/components/order-list-manager/use-order-list-data.ts +++ b/libs/orders/src/lib/components/order-list-manager/use-order-list-data.ts @@ -50,17 +50,21 @@ export const useOrderListData = ({ if (!gridRef.current?.api) { return false; } - if (!scrolledToTop.current) { - const createdAt = dataRef.current?.[0]?.node.createdAt; - if (createdAt) { - newRows.current += delta.filter( - (trade) => trade.createdAt > createdAt - ).length; + if (dataRef.current?.length) { + if (!scrolledToTop.current) { + const createdAt = dataRef.current?.[0]?.node.createdAt; + if (createdAt) { + newRows.current += delta.filter( + (trade) => trade.createdAt > createdAt + ).length; + } } + dataRef.current = data; + gridRef.current.api.refreshInfiniteCache(); + return true; } dataRef.current = data; - gridRef.current.api.refreshInfiniteCache(); - return true; + return false; }, [gridRef, scrolledToTop] ); diff --git a/libs/orders/src/lib/components/order-list/order-list.tsx b/libs/orders/src/lib/components/order-list/order-list.tsx index 2a471f7ec..5aab707b7 100644 --- a/libs/orders/src/lib/components/order-list/order-list.tsx +++ b/libs/orders/src/lib/components/order-list/order-list.tsx @@ -24,11 +24,7 @@ import type { ICellRendererParams, ValueFormatterParams, } from 'ag-grid-community'; -import type { - AgGridReact, - AgGridReactProps, - AgReactUiProps, -} from 'ag-grid-react'; +import type { AgGridReact, AgGridReactProps } from 'ag-grid-react'; import { AgGridColumn } from 'ag-grid-react'; import { forwardRef, useState } from 'react'; import type { Orders_party_ordersConnection_edges_node } from '../'; @@ -40,7 +36,7 @@ import { OrderEditDialog } from './order-edit-dialog'; import type { OrderWithMarket } from '../'; import { OrderFeedback } from '../order-feedback'; -type OrderListProps = AgGridReactProps | AgReactUiProps; +type OrderListProps = AgGridReactProps; export const OrderList = forwardRef( (props, ref) => { @@ -104,7 +100,7 @@ type OrderListTableValueFormatterParams = Omit< data: OrderWithMarket | null; }; -type OrderListTableProps = (AgGridReactProps | AgReactUiProps) & { +type OrderListTableProps = AgGridReactProps & { cancel: (order: OrderWithMarket) => void; setEditOrder: (order: OrderWithMarket) => void; }; diff --git a/libs/react-helpers/src/lib/generic-data-provider.spec.ts b/libs/react-helpers/src/lib/generic-data-provider.spec.ts index 7970dc2b7..951b8d060 100644 --- a/libs/react-helpers/src/lib/generic-data-provider.spec.ts +++ b/libs/react-helpers/src/lib/generic-data-provider.spec.ts @@ -559,15 +559,15 @@ describe('derived data provider', () => { expect(callback.mock.calls[0][0].error).toBe(error); expect(callback.mock.calls[0][0].loading).toBe(false); subscription.reload(); - expect(callback.mock.calls.length).toBe(3); - expect(callback.mock.calls[2][0].loading).toBe(true); + expect(callback.mock.calls.length).toBe(2); + expect(callback.mock.calls[1][0].loading).toBe(true); await resolveQuery({ data: part1 }); - expect(callback.mock.calls.length).toBe(3); + expect(callback.mock.calls.length).toBe(2); await resolveQuery({ data: part2 }); - expect(callback.mock.calls.length).toBe(4); - expect(callback.mock.calls[3][0].data).toStrictEqual(data); - expect(callback.mock.calls[3][0].loading).toBe(false); - expect(callback.mock.calls[3][0].error).toBeUndefined(); + expect(callback.mock.calls.length).toBe(3); + expect(callback.mock.calls[2][0].data).toStrictEqual(data); + expect(callback.mock.calls[2][0].loading).toBe(false); + expect(callback.mock.calls[2][0].error).toBeUndefined(); subscription.unsubscribe(); }); }); diff --git a/libs/react-helpers/src/lib/generic-data-provider.ts b/libs/react-helpers/src/lib/generic-data-provider.ts index 776f06777..4b60f40c5 100644 --- a/libs/react-helpers/src/lib/generic-data-provider.ts +++ b/libs/react-helpers/src/lib/generic-data-provider.ts @@ -76,7 +76,7 @@ export interface Append { } interface GetData { - (queryData: QueryData): Data | null; + (queryData: QueryData, variables?: OperationVariables): Data | null; } interface GetPageInfo { @@ -88,7 +88,7 @@ interface GetTotalCount { } interface GetDelta { - (subscriptionData: SubscriptionData): Delta; + (subscriptionData: SubscriptionData, variables?: OperationVariables): Delta; } export function defaultAppend( @@ -144,6 +144,7 @@ interface DataProviderParams { first: number; }; fetchPolicy?: FetchPolicy; + resetDelay?: number; } /** @@ -162,6 +163,7 @@ function makeDataProviderInternal({ getDelta, pagination, fetchPolicy, + resetDelay, }: DataProviderParams): Subscribe< Data, Delta @@ -171,6 +173,7 @@ function makeDataProviderInternal({ // subscription is started before initial query, all deltas that will arrive before initial query response are put on queue const updateQueue: Delta[] = []; + let resetTimer: ReturnType; let variables: OperationVariables | undefined; let data: Data | null = null; let error: Error | undefined; @@ -242,7 +245,7 @@ function makeDataProviderInternal({ }, fetchPolicy: fetchPolicy || 'no-cache', }); - const insertionData = getData(res.data); + const insertionData = getData(res.data, variables); const insertionPageInfo = pagination.getPageInfo(res.data); ({ data, totalCount } = pagination.append( data, @@ -269,10 +272,10 @@ function makeDataProviderInternal({ variables: pagination ? { ...variables, pagination: { first: pagination.first } } : variables, - fetchPolicy, + fetchPolicy: fetchPolicy || 'no-cache', errorPolicy: 'ignore', }); - data = getData(res.data); + data = getData(res.data, variables); if (data && pagination) { if (!(data instanceof Array)) { throw new Error( @@ -332,6 +335,9 @@ function makeDataProviderInternal({ const initialize = async () => { if (subscription) { + if (resetTimer) { + clearTimeout(resetTimer); + } return; } loading = true; @@ -352,7 +358,7 @@ function makeDataProviderInternal({ if (!subscriptionData) { return; } - const delta = getDelta(subscriptionData); + const delta = getDelta(subscriptionData, variables); if (loading || !data) { updateQueue.push(delta); } else { @@ -364,17 +370,25 @@ function makeDataProviderInternal({ notifyAll({ delta, isUpdate: true }); } }, - () => reload() + (e) => { + error = e as Error; + if (subscription) { + subscription.unsubscribe(); + subscription = undefined; + } + notifyAll(); + } ); } await initialFetch(); }; const reset = () => { - if (subscription) { - subscription.unsubscribe(); - subscription = undefined; + if (!subscription) { + return; } + subscription.unsubscribe(); + subscription = undefined; data = null; error = undefined; loading = false; @@ -386,7 +400,11 @@ function makeDataProviderInternal({ const unsubscribe = (callback: UpdateCallback) => { callbacks.splice(callbacks.indexOf(callback), 1); if (callbacks.length === 0) { - reset(); + if (resetDelay) { + resetTimer = setTimeout(reset, resetDelay); + } else { + reset(); + } } }; diff --git a/libs/react-helpers/src/lib/pagination.ts b/libs/react-helpers/src/lib/pagination.ts index 0ecb403bf..008989e25 100644 --- a/libs/react-helpers/src/lib/pagination.ts +++ b/libs/react-helpers/src/lib/pagination.ts @@ -37,15 +37,22 @@ export const makeInfiniteScrollGetRows = startRow += newRows.current; endRow += newRows.current; try { - if (data.current && data.current.indexOf(null) < endRow) { - await load(); + if (data.current) { + const firstMissingRowIndex = data.current.indexOf(null); + if ( + endRow > data.current.length || + (firstMissingRowIndex !== -1 && firstMissingRowIndex < endRow) + ) { + await load(); + } } const rowsThisBlock = data.current ? data.current.slice(startRow, endRow).map((edge) => edge?.node) : []; successCallback( rowsThisBlock, - getLastRow(startRow, endRow, rowsThisBlock.length, totalCount.current) + getLastRow(startRow, endRow, rowsThisBlock.length, totalCount.current) - + newRows.current ); } catch (e) { failCallback(); diff --git a/libs/trades/src/lib/trades-container.tsx b/libs/trades/src/lib/trades-container.tsx index 6ffb72db3..df1de64b9 100644 --- a/libs/trades/src/lib/trades-container.tsx +++ b/libs/trades/src/lib/trades-container.tsx @@ -55,17 +55,21 @@ export const TradesContainer = ({ marketId }: TradesContainerProps) => { if (!gridRef.current?.api) { return false; } - if (!scrolledToTop.current) { - const createdAt = dataRef.current?.[0]?.node.createdAt; - if (createdAt) { - newRows.current += delta.filter( - (trade) => trade.createdAt > createdAt - ).length; + if (dataRef.current?.length) { + if (!scrolledToTop.current) { + const createdAt = dataRef.current?.[0]?.node.createdAt; + if (createdAt) { + newRows.current += delta.filter( + (trade) => trade.createdAt > createdAt + ).length; + } } + dataRef.current = data; + gridRef.current.api.refreshInfiniteCache(); + return true; } dataRef.current = data; - gridRef.current.api.refreshInfiniteCache(); - return true; + return false; }, [] ); @@ -115,7 +119,8 @@ export const TradesContainer = ({ marketId }: TradesContainerProps) => {