chore: fix update results from dataProvider when variable change (#1962)

* chore: fix race conditions in useDataProvider hook

* chore: fix race conditions in useDataProvider hook - populate fix to other modules

* chore: fix race conditions in useDataProvider hook - populate fix to other modules

* chore: fix race conditions in useDataProvider hook - populate fix to other modules

* chore: fix race conditions in useDataProvider hook - populate fix to other modules

Co-authored-by: maciek <maciek@vegaprotocol.io>

apps/console-lite/src/app/components/portfolio/accounts/accounts.tsx

@@ -32,10 +32,8 @@ const AccountsManager = () => {
     dataProvider: aggregatedAccountsDataProvider,
     update,
     variables,
+    updateOnInit: true,
   });
-  if (!dataRef.current && data) {
-    dataRef.current = data;
-  }
   const getRows = async ({
     successCallback,
     startRow,

apps/trading/pages/portfolio/accounts-container.tsx

@@ -1,8 +1,9 @@
-import { useState } from 'react';
+import { useCallback, useState } from 'react';
 import { Button } from '@vegaprotocol/ui-toolkit';
 import { t } from '@vegaprotocol/react-helpers';
 import { WithdrawalDialogs } from '@vegaprotocol/withdraws';
 import { Web3Container } from '@vegaprotocol/web3';
+import type { AssetFieldsFragment } from '@vegaprotocol/assets';
 import { useAssetDetailsDialogStore } from '@vegaprotocol/assets';
 import { Splash } from '@vegaprotocol/ui-toolkit';
 import { useVegaWallet } from '@vegaprotocol/wallet';
@@ -16,6 +17,23 @@ export const AccountsContainer = () => {
   const { open: openAssetDetailsDialog } = useAssetDetailsDialogStore();
   const [assetId, setAssetId] = useState<string>();
 
+  const onClickAsset = useCallback(
+    (value?: string | AssetFieldsFragment) => {
+      value && openAssetDetailsDialog(value);
+    },
+    [openAssetDetailsDialog]
+  );
+
+  const onClickWithdraw = useCallback((assetId?: string) => {
+    setWithdrawDialog(true);
+    setAssetId(assetId);
+  }, []);
+
+  const onClickDeposit = useCallback((assetId?: string) => {
+    setDepositDialog(true);
+    setAssetId(assetId);
+  }, []);
+
   if (!pubKey) {
     return (
       <Splash>
@@ -30,17 +48,9 @@ export const AccountsContainer = () => {
         <div>
           <AccountManager
             partyId={pubKey}
-            onClickAsset={(value) => {
+            onClickAsset={onClickAsset}
-              value && openAssetDetailsDialog(value);
+            onClickWithdraw={onClickWithdraw}
-            }}
+            onClickDeposit={onClickDeposit}
-            onClickWithdraw={(assetId) => {
-              setWithdrawDialog(true);
-              setAssetId(assetId);
-            }}
-            onClickDeposit={(assetId) => {
-              setDepositDialog(true);
-              setAssetId(assetId);
-            }}
           />
         </div>
         <div className="flex justify-end p-2 px-[11px]">

apps/trading/pages/portfolio/index.page.tsx

@@ -24,7 +24,7 @@ const Portfolio = () => {
   const wrapperClasses = 'h-full max-h-full flex flex-col';
   return (
     <div className={wrapperClasses}>
-      <ResizableGrid vertical={true}>
+      <ResizableGrid vertical>
         <ResizableGridPanel minSize={75}>
           <PortfolioGridChild>
             <Tabs>

libs/accounts/src/lib/accounts-manager.spec.tsx

@@ -1,12 +1,11 @@
 import { act, render } from '@testing-library/react';
 import { AccountManager } from './accounts-manager';
+import * as helpers from '@vegaprotocol/react-helpers';
 
-const mockReload = jest.fn();
 jest.mock('@vegaprotocol/react-helpers', () => ({
   ...jest.requireActual('@vegaprotocol/react-helpers'),
   useDataProvider: jest.fn(() => ({
     data: [],
-    reload: mockReload,
   })),
 }));
 
@@ -15,10 +14,14 @@ describe('AccountManager', () => {
     const { rerender } = render(
       <AccountManager partyId="partyOne" onClickAsset={jest.fn} />
     );
-    expect(mockReload).not.toHaveBeenCalled();
+    expect(
+      (helpers.useDataProvider as jest.Mock).mock.calls[0][0].variables.partyId
+    ).toEqual('partyOne');
     await act(() => {
       rerender(<AccountManager partyId="partyTwo" onClickAsset={jest.fn} />);
     });
-    expect(mockReload).toHaveBeenCalledWith(true);
+    expect(
+      (helpers.useDataProvider as jest.Mock).mock.calls[1][0].variables.partyId
+    ).toEqual('partyTwo');
   });
 });

libs/accounts/src/lib/accounts-manager.tsx

@@ -2,7 +2,7 @@ import type { Asset } from '@vegaprotocol/assets';
 import { useDataProvider } from '@vegaprotocol/react-helpers';
 import { AsyncRenderer } from '@vegaprotocol/ui-toolkit';
 import type { AgGridReact } from 'ag-grid-react';
-import { useRef, useMemo, useCallback } from 'react';
+import { useRef, useMemo, useCallback, memo } from 'react';
 import type { AccountFields } from './accounts-data-provider';
 import { aggregatedAccountsDataProvider } from './accounts-data-provider';
 import type { GetRowsParams } from './accounts-table';
@@ -15,64 +15,56 @@ interface AccountManagerProps {
   onClickDeposit?: (assetId?: string) => void;
 }
 
-export const AccountManager = ({
+export const AccountManager = memo(
-  onClickAsset,
+  ({
-  onClickWithdraw,
+    onClickAsset,
-  onClickDeposit,
+    onClickWithdraw,
-  partyId,
+    onClickDeposit,
-}: AccountManagerProps) => {
+    partyId,
-  const partyIdRef = useRef<string>(partyId);
+  }: AccountManagerProps) => {
-  const gridRef = useRef<AgGridReact | null>(null);
+    const gridRef = useRef<AgGridReact | null>(null);
-  const dataRef = useRef<AccountFields[] | null>(null);
+    const dataRef = useRef<AccountFields[] | null>(null);
-  const variables = useMemo(() => ({ partyId }), [partyId]);
+    const variables = useMemo(() => ({ partyId }), [partyId]);
-  const update = useCallback(
+    const update = useCallback(
-    ({ data }: { data: AccountFields[] | null }) => {
+      ({ data }: { data: AccountFields[] | null }) => {
-      dataRef.current = data;
+        dataRef.current = data;
-      gridRef.current?.api?.refreshInfiniteCache();
+        gridRef.current?.api?.refreshInfiniteCache();
-      return true;
+        return true;
-    },
+      },
-    [gridRef]
+      [gridRef]
-  );
+    );
 
-  const { data, loading, error, reload } = useDataProvider<
+    const { data, loading, error } = useDataProvider<AccountFields[], never>({
-    AccountFields[],
+      dataProvider: aggregatedAccountsDataProvider,
-    never
+      update,
-  >({
+      variables,
-    dataProvider: aggregatedAccountsDataProvider,
+      updateOnInit: true,
-    update,
+    });
-    variables,
+    const getRows = async ({
-  });
+      successCallback,
-  if (partyId !== partyIdRef.current) {
+      startRow,
-    reload(true);
+      endRow,
-    partyIdRef.current = partyId;
+    }: GetRowsParams) => {
+      const rowsThisBlock = dataRef.current
+        ? dataRef.current.slice(startRow, endRow)
+        : [];
+      const lastRow = dataRef.current?.length ?? -1;
+      successCallback(rowsThisBlock, lastRow);
+    };
+    return (
+      <AsyncRenderer data={data || []} error={error} loading={loading}>
+        <AccountTable
+          rowModelType={data?.length ? 'infinite' : 'clientSide'}
+          rowData={data?.length ? undefined : []}
+          ref={gridRef}
+          datasource={{ getRows }}
+          onClickAsset={onClickAsset}
+          onClickDeposit={onClickDeposit}
+          onClickWithdraw={onClickWithdraw}
+        />
+      </AsyncRenderer>
+    );
   }
-  if (!dataRef.current && data) {
+);
-    dataRef.current = data;
-  }
-  const getRows = async ({
-    successCallback,
-    startRow,
-    endRow,
-  }: GetRowsParams) => {
-    const rowsThisBlock = dataRef.current
-      ? dataRef.current.slice(startRow, endRow)
-      : [];
-    const lastRow = dataRef.current?.length ?? -1;
-    successCallback(rowsThisBlock, lastRow);
-  };
-  return (
-    <AsyncRenderer data={data || []} error={error} loading={loading}>
-      <AccountTable
-        rowModelType={data?.length ? 'infinite' : 'clientSide'}
-        rowData={data?.length ? undefined : []}
-        ref={gridRef}
-        datasource={{ getRows }}
-        onClickAsset={onClickAsset}
-        onClickDeposit={onClickDeposit}
-        onClickWithdraw={onClickWithdraw}
-      />
-    </AsyncRenderer>
-  );
-};
 
-export default AccountManager;
+export default memo(AccountManager);

libs/fills/src/lib/use-fills-list.spec.ts

@@ -86,8 +86,9 @@ describe('useFillsList Hook', () => {
       addNewRows: expect.any(Function),
       getRows: expect.any(Function),
     });
+    updateMock({ data: mockData });
     expect(mockRefreshAgGridApi).not.toHaveBeenCalled();
-    updateMock({ data: {} });
+    updateMock({ data: mockData });
     expect(mockRefreshAgGridApi).toHaveBeenCalled();
   });
 });

libs/fills/src/lib/use-fills-list.ts

@@ -77,11 +77,14 @@ export const useFillsList = ({ partyId, gridRef, scrolledToTop }: Props) => {
   const { data, error, loading, load, totalCount } = useDataProvider<
     (TradeEdge | null)[],
     Trade[]
-  >({ dataProvider: fillsWithMarketProvider, update, insert, variables });
+  >({
-  if (!dataRef.current && data) {
+    dataProvider: fillsWithMarketProvider,
-    totalCountRef.current = totalCount;
+    update,
-    dataRef.current = data;
+    insert,
-  }
+    variables,
+    updateOnInit: true,
+  });
+  totalCountRef.current = totalCount;
 
   const getRows = makeInfiniteScrollGetRows<TradeEdge>(
     newRows,

libs/orders/src/lib/components/order-list-manager/use-order-list-data.spec.ts

@@ -8,7 +8,7 @@ import type { IGetRowsParams } from 'ag-grid-community';
 
 const loadMock = jest.fn();
 
-let mockData = null;
+let mockData: Edge<OrderFieldsFragment>[] | null = null;
 let mockDataProviderData = {
   data: mockData as (Edge<OrderFieldsFragment> | null)[] | null,
   error: undefined,
@@ -94,8 +94,9 @@ describe('useOrderListData Hook', () => {
       addNewRows: expect.any(Function),
       getRows: expect.any(Function),
     });
+    updateMock({ data: mockData, delta: [] });
     expect(mockRefreshAgGridApi).not.toHaveBeenCalled();
-    updateMock({ data: [], delta: [] });
+    updateMock({ data: mockData, delta: [] });
     expect(mockRefreshAgGridApi).toHaveBeenCalled();
   });
 
@@ -148,6 +149,10 @@ describe('useOrderListData Hook', () => {
       endRow: 4,
     } as unknown as IGetRowsParams;
 
+    await waitFor(async () => {
+      updateMock({ data: mockData });
+    });
+
     await waitFor(async () => {
       const promise = result.current.getRows(getRowsParams);
       updateMock({ data: mockNextData, delta: mockDelta });

libs/orders/src/lib/components/order-list-manager/use-order-list-data.ts

@@ -77,11 +77,9 @@ export const useOrderListData = ({
     update,
     insert,
     variables,
+    updateOnInit: true,
   });
-  if (!dataRef.current && data) {
+  totalCountRef.current = totalCount;
-    totalCountRef.current = totalCount;
-    dataRef.current = data;
-  }
 
   const getRows = makeInfiniteScrollGetRows<OrderEdge>(
     newRows,

libs/trades/src/lib/trades-container.tsx

@@ -85,12 +85,9 @@ export const TradesContainer = ({ marketId }: TradesContainerProps) => {
     update,
     insert,
     variables,
+    updateOnInit: true,
   });
   totalCountRef.current = totalCount;
-  if (!dataRef.current && data) {
-    dataRef.current = data;
-  }
-
   const getRows = makeInfiniteScrollGetRows<TradeEdge>(
     newRows,
     dataRef,