fix: remove useDataProviderHook updateOnInit (#2041)

* fix: fix useDataProviderHook updateOnInit

* chore: remove updateOnInit - always execute initial update in useDataProviderHook

* chore: remove useless variables param in useDataProvider update and insert callbacks

* chore: remove console.log
This commit is contained in:
Bartłomiej Głownia 2022-11-14 16:08:12 +01:00 committed by GitHub
parent 94c93ce790
commit 0c93023998
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
28 changed files with 235 additions and 81 deletions

View File

@ -32,7 +32,6 @@ const AccountsManager = () => {
dataProvider: aggregatedAccountsDataProvider, dataProvider: aggregatedAccountsDataProvider,
update, update,
variables, variables,
updateOnInit: true,
}); });
const getRows = async ({ const getRows = async ({
successCallback, successCallback,

View File

@ -44,7 +44,7 @@ const SimpleMarketList = () => {
const { data, error, loading } = useDataProvider({ const { data, error, loading } = useDataProvider({
dataProvider: marketsWithCandlesProvider, dataProvider: marketsWithCandlesProvider,
variables, variables,
noUpdate: true, skipUpdates: true,
}); });
const localData = useMarketsFilterData(data, params); const localData = useMarketsFilterData(data, params);

View File

@ -44,7 +44,7 @@ export const lpDataProvider = makeDerivedDataProvider(
const useMarketDetails = (marketId: string | undefined) => { const useMarketDetails = (marketId: string | undefined) => {
const { data, loading, error } = useDataProvider({ const { data, loading, error } = useDataProvider({
dataProvider: lpDataProvider, dataProvider: lpDataProvider,
noUpdate: true, skipUpdates: true,
variables: useMemo(() => ({ marketId }), [marketId]), variables: useMemo(() => ({ marketId }), [marketId]),
}); });

View File

@ -96,7 +96,6 @@ export const Last24hVolume = ({
update: updateCandle24hAgo, update: updateCandle24hAgo,
variables: variables24hAgo, variables: variables24hAgo,
skip: !marketId || !data, skip: !marketId || !data,
updateOnInit: true,
}); });
return ( return (

View File

@ -90,6 +90,18 @@ describe('Market trading page', () => {
}); });
}); });
}); });
it('must see market mode', () => {
cy.getByTestId(marketSummaryBlock).within(() => {
cy.getByTestId(marketMode).within(() => {
cy.getByTestId(itemHeader).should('have.text', 'Trading mode');
cy.getByTestId(itemValue).should(
'have.text',
'Monitoring auction - liquidity'
);
});
});
});
}); });
describe('Market tooltips', { tags: '@regression' }, () => { describe('Market tooltips', { tags: '@regression' }, () => {

View File

@ -37,7 +37,7 @@ export const Liquidity = () => {
const { data: marketProvision } = useDataProvider({ const { data: marketProvision } = useDataProvider({
dataProvider: marketLiquidityDataProvider, dataProvider: marketLiquidityDataProvider,
noUpdate: true, skipUpdates: true,
variables: useMemo(() => ({ marketId }), [marketId]), variables: useMemo(() => ({ marketId }), [marketId]),
}); });
const dataRef = useRef<LiquidityProvisionData[] | null>(null); const dataRef = useRef<LiquidityProvisionData[] | null>(null);

View File

@ -106,7 +106,6 @@ export const Market = ({
update: updateProvider, update: updateProvider,
variables, variables,
skip: !marketId || !data, skip: !marketId || !data,
updateOnInit: true,
}); });
const tradeView = useMemo(() => { const tradeView = useMemo(() => {

View File

@ -69,7 +69,6 @@ export const Last24hPriceChange = ({ marketId }: { marketId: string }) => {
update, update,
variables, variables,
skip: !marketId || !data, skip: !marketId || !data,
updateOnInit: true,
}); });
return ( return (

View File

@ -71,7 +71,6 @@ export const Last24hVolume = ({ marketId }: { marketId: string }) => {
update, update,
variables, variables,
skip: !marketId || !data, skip: !marketId || !data,
updateOnInit: true,
}); });
return ( return (

View File

@ -49,7 +49,6 @@ export const MarketMarkPrice = ({ marketId }: { marketId: string }) => {
update, update,
variables, variables,
skip: !marketId || !data, skip: !marketId || !data,
updateOnInit: true,
}); });
return ( return (

View File

@ -62,7 +62,6 @@ export const MarketTradingModeComponent = ({ marketId, onSelect }: Props) => {
update, update,
variables, variables,
skip: !marketId || !data, skip: !marketId || !data,
updateOnInit: true,
}); });
return ( return (
@ -83,8 +82,7 @@ export const MarketTradingModeComponent = ({ marketId, onSelect }: Props) => {
{tradingMode === MarketTradingMode.TRADING_MODE_MONITORING_AUCTION && {tradingMode === MarketTradingMode.TRADING_MODE_MONITORING_AUCTION &&
trigger && trigger &&
trigger !== AuctionTrigger.AUCTION_TRIGGER_UNSPECIFIED trigger !== AuctionTrigger.AUCTION_TRIGGER_UNSPECIFIED
? `${MarketTradingModeMapping[tradingMode]} ? `${MarketTradingModeMapping[tradingMode]} - ${AuctionTriggerMapping[trigger]}`
- ${AuctionTriggerMapping[trigger]}`
: MarketTradingModeMapping[tradingMode as Types.MarketTradingMode]} : MarketTradingModeMapping[tradingMode as Types.MarketTradingMode]}
</div> </div>
</HeaderStat> </HeaderStat>

View File

@ -54,7 +54,6 @@ export const MarketVolume = ({ marketId }: { marketId: string }) => {
update, update,
variables, variables,
skip: !marketId || !data, skip: !marketId || !data,
updateOnInit: true,
}); });
return ( return (

View File

@ -133,7 +133,7 @@ export const SelectMarketPopover = ({
const variables = useMemo(() => ({ partyId: pubKey }), [pubKey]); const variables = useMemo(() => ({ partyId: pubKey }), [pubKey]);
const { data: party, loading: positionsLoading } = useDataProvider({ const { data: party, loading: positionsLoading } = useDataProvider({
dataProvider: positionsDataProvider, dataProvider: positionsDataProvider,
noUpdate: true, skipUpdates: true,
variables, variables,
skip: !pubKey, skip: !pubKey,
}); });

View File

@ -38,7 +38,6 @@ export const AccountManager = memo(
dataProvider: aggregatedAccountsDataProvider, dataProvider: aggregatedAccountsDataProvider,
update, update,
variables, variables,
updateOnInit: true,
}); });
const getRows = async ({ const getRows = async ({
successCallback, successCallback,

View File

@ -56,7 +56,7 @@ export const MarketSelector = ({ market, setMarket, ItemRenderer }: Props) => {
const { data, loading, error } = useDataProvider({ const { data, loading, error } = useDataProvider({
dataProvider: marketsProvider, dataProvider: marketsProvider,
noUpdate: true, skipUpdates: true,
}); });
const outsideClickCb = useCallback(() => { const outsideClickCb = useCallback(() => {

View File

@ -28,7 +28,7 @@ export const useCalculateSlippage = ({ marketId, order }: Props) => {
); );
const { data: market } = useDataProvider<SingleMarketFieldsFragment, never>({ const { data: market } = useDataProvider<SingleMarketFieldsFragment, never>({
dataProvider: marketProvider, dataProvider: marketProvider,
noUpdate: true, skipUpdates: true,
variables, variables,
}); });
const volPriceArr = const volPriceArr =

View File

@ -82,7 +82,6 @@ export const useFillsList = ({ partyId, gridRef, scrolledToTop }: Props) => {
update, update,
insert, insert,
variables, variables,
updateOnInit: true,
}); });
totalCountRef.current = totalCount; totalCountRef.current = totalCount;

View File

@ -123,7 +123,7 @@ export const useMarketsLiquidity = () => {
const { data, loading, error } = useDataProvider({ const { data, loading, error } = useDataProvider({
dataProvider: liquidityProvisionProvider, dataProvider: liquidityProvisionProvider,
variables, variables,
noUpdate: true, skipUpdates: true,
}); });
return { return {

View File

@ -126,7 +126,7 @@ export const DepthChartContainer = ({ marketId }: DepthChartManagerProps) => {
loading: marketLoading, loading: marketLoading,
} = useDataProvider({ } = useDataProvider({
dataProvider: marketProvider, dataProvider: marketProvider,
noUpdate: true, skipUpdates: true,
variables, variables,
}); });

View File

@ -96,7 +96,7 @@ export const OrderbookManager = ({ marketId }: OrderbookManagerProps) => {
loading: marketLoading, loading: marketLoading,
} = useDataProvider({ } = useDataProvider({
dataProvider: marketProvider, dataProvider: marketProvider,
noUpdate: true, skipUpdates: true,
variables, variables,
}); });

View File

@ -72,7 +72,7 @@ export const MarketInfoContainer = ({
const { data, loading, error } = useDataProvider({ const { data, loading, error } = useDataProvider({
dataProvider: marketInfoDataProvider, dataProvider: marketInfoDataProvider,
noUpdate: true, skipUpdates: true,
variables, variables,
}); });

View File

@ -11,7 +11,7 @@ interface MarketsContainerProps {
export const MarketsContainer = ({ onSelect }: MarketsContainerProps) => { export const MarketsContainer = ({ onSelect }: MarketsContainerProps) => {
const { data, error, loading } = useDataProvider<MarketWithData[], never>({ const { data, error, loading } = useDataProvider<MarketWithData[], never>({
dataProvider, dataProvider,
noUpdate: true, skipUpdates: true,
}); });
return ( return (

View File

@ -101,7 +101,7 @@ export const useMarketList = () => {
const { data, loading, error } = useDataProvider({ const { data, loading, error } = useDataProvider({
dataProvider: marketListProvider, dataProvider: marketListProvider,
variables, variables,
noUpdate: true, skipUpdates: true,
}); });
return { return {

View File

@ -112,7 +112,6 @@ export const useOrderListData = ({
update, update,
insert, insert,
variables, variables,
updateOnInit: true,
}); });
totalCountRef.current = totalCount; totalCountRef.current = totalCount;

View File

@ -1,5 +1,6 @@
import { renderHook, act } from '@testing-library/react'; import { renderHook, act } from '@testing-library/react';
import { useThrottledDataProvider } from './use-data-provider'; import { useDataProvider, useThrottledDataProvider } from './use-data-provider';
import type { useDataProviderParams } from './use-data-provider';
import type { Subscribe, UpdateCallback } from '../lib/generic-data-provider'; import type { Subscribe, UpdateCallback } from '../lib/generic-data-provider';
import { MockedProvider } from '@apollo/client/testing'; import { MockedProvider } from '@apollo/client/testing';
@ -10,11 +11,14 @@ const unsubscribe = jest.fn();
const reload = jest.fn(); const reload = jest.fn();
const flush = jest.fn(); const flush = jest.fn();
const load = jest.fn(); const load = jest.fn();
const update = jest.fn();
const insert = jest.fn();
const variables = { partyId: '0x123' };
const updateCallbackPayload: Parameters<UpdateCallback<Data, Delta>>['0'] = { const updateCallbackPayload: Parameters<UpdateCallback<Data, Delta>>['0'] = {
data: 0, data: 0,
loading: false, loading: false,
loaded: false, loaded: true,
pageInfo: null, pageInfo: null,
}; };
@ -32,7 +36,179 @@ dataProvider.mockReturnValue({
jest.useFakeTimers(); jest.useFakeTimers();
describe('useDataProvider hook', () => {
const render = (initialProps?: useDataProviderParams<Data, Delta>) =>
renderHook<
ReturnType<typeof useDataProvider>,
useDataProviderParams<Data, Delta>
>((props) => useDataProvider(props), {
wrapper: MockedProvider,
initialProps,
});
beforeEach(() => {
update.mockClear();
insert.mockClear();
dataProvider.mockClear();
unsubscribe.mockClear();
});
it('calls update on load', async () => {
const { result } = render({ dataProvider, update, variables });
expect(result.current.data).toEqual(null);
expect(result.current.loading).toEqual(true);
expect(result.current.error).toEqual(undefined);
const callback = dataProvider.mock.calls[0][0];
await act(async () => {
callback(updateCallbackPayload);
});
expect(result.current.data).toEqual(updateCallbackPayload.data);
expect(result.current.loading).toEqual(false);
expect(update).toBeCalledTimes(1);
expect(update.mock.calls[0][0].data).toEqual(updateCallbackPayload.data);
});
it('calls update on error', async () => {
const { result } = render({ dataProvider, update, variables });
expect(result.current.data).toEqual(null);
expect(result.current.loading).toEqual(true);
expect(result.current.error).toEqual(undefined);
const callback = dataProvider.mock.calls[0][0];
await act(async () => {
callback(updateCallbackPayload);
});
expect(result.current.data).toEqual(updateCallbackPayload.data);
expect(result.current.loading).toEqual(false);
expect(update).toBeCalledTimes(1);
expect(update.mock.calls[0][0].data).toEqual(updateCallbackPayload.data);
});
it('calls update if isUpdate and skip setting state if update returns true', async () => {
const { result } = render({ dataProvider, update, variables });
let data = 0;
const delta = 0;
const callback = dataProvider.mock.calls[0][0];
await act(async () => {
callback({ ...updateCallbackPayload, data: ++data });
});
expect(update).toBeCalledTimes(1);
await act(async () => {
callback({
...updateCallbackPayload,
data: ++data,
delta,
isUpdate: true,
});
});
expect(result.current.data).toEqual(data);
expect(update).toBeCalledTimes(2);
expect(update.mock.calls[1][0].data).toEqual(data);
expect(update.mock.calls[1][0].delta).toEqual(delta);
update.mockReturnValueOnce(true);
await act(async () => {
callback({
...updateCallbackPayload,
data: ++data,
delta,
isUpdate: true,
});
});
expect(result.current.data).toEqual(update.mock.calls[1][0].data);
expect(update).toBeCalledTimes(3);
expect(update.mock.calls[2][0].data).toEqual(data);
expect(update.mock.calls[2][0].delta).toEqual(delta);
});
it('calls insert if isInsert and skip setting state if update returns true', async () => {
const { result } = render({ dataProvider, insert, variables });
let data = 0;
const insertionData = 0;
const callback = dataProvider.mock.calls[0][0];
await act(async () => {
callback({ ...updateCallbackPayload, data: ++data });
});
await act(async () => {
callback({
...updateCallbackPayload,
data: ++data,
insertionData,
isInsert: true,
});
});
expect(result.current.data).toEqual(data);
expect(insert).toBeCalledTimes(1);
expect(insert.mock.calls[0][0].data).toEqual(data);
expect(insert.mock.calls[0][0].insertionData).toEqual(insertionData);
insert.mockReturnValueOnce(true);
await act(async () => {
callback({
...updateCallbackPayload,
data: ++data,
insertionData,
isInsert: true,
});
});
expect(result.current.data).toEqual(insert.mock.calls[0][0].data);
expect(insert).toBeCalledTimes(2);
expect(insert.mock.calls[1][0].data).toEqual(data);
expect(insert.mock.calls[1][0].insertionData).toEqual(insertionData);
});
it('change data provider instance on variables change', async () => {
const { result, rerender } = render({ dataProvider, update, variables });
const callback = dataProvider.mock.calls[0][0];
expect(dataProvider).toBeCalledTimes(1);
await act(async () => {
callback({ ...updateCallbackPayload });
});
expect(update).toBeCalledTimes(1);
// changing variables after date was loaded
await act(async () => {
rerender({ dataProvider, update, variables: { partyId: '0x321' } });
});
expect(unsubscribe).toBeCalledTimes(1);
expect(dataProvider).toBeCalledTimes(2);
expect(result.current.data).toEqual(null);
expect(result.current.loading).toEqual(true);
expect(result.current.error).toEqual(undefined);
await act(async () => {
callback({ ...updateCallbackPayload });
});
expect(update).toBeCalledTimes(2);
// changing variables, apollo query will return error
await act(async () => {
rerender({ dataProvider, update, variables });
});
expect(unsubscribe).toBeCalledTimes(2);
expect(dataProvider).toBeCalledTimes(3);
expect(result.current.data).toEqual(null);
expect(result.current.loading).toEqual(true);
expect(result.current.error).toEqual(undefined);
await act(async () => {
callback({
data: null,
loading: false,
loaded: false,
error: new Error(),
pageInfo: null,
});
});
expect(update).toBeCalledTimes(3);
});
it('do not create data provider instance when skip is true', async () => {
const { result } = render({ dataProvider, update, variables, skip: true });
expect(dataProvider).toBeCalledTimes(0);
expect(result.current.data).toEqual(null);
expect(result.current.loading).toEqual(false);
expect(result.current.error).toEqual(undefined);
});
});
describe('useThrottledDataProvider hook', () => { describe('useThrottledDataProvider hook', () => {
beforeEach(() => {
dataProvider.mockClear();
});
it('throttling should delay update', async () => { it('throttling should delay update', async () => {
const wait = 100; const wait = 100;
const { result } = renderHook( const { result } = renderHook(
@ -48,18 +224,17 @@ describe('useThrottledDataProvider hook', () => {
expect(result.current.data).toEqual(null); expect(result.current.data).toEqual(null);
expect(result.current.loading).toEqual(true); expect(result.current.loading).toEqual(true);
expect(result.current.error).toEqual(undefined); expect(result.current.error).toEqual(undefined);
const callback = const callback = dataProvider.mock.calls[0][0];
dataProvider.mock.calls[dataProvider.mock.calls.length - 1][0];
await act(async () => { await act(async () => {
callback({ ...updateCallbackPayload, data: 1 }); // initial value callback({ ...updateCallbackPayload, data: 1 }); // initial value
}); });
await act(async () => { await act(async () => {
callback({ ...updateCallbackPayload, data: 2, isUpdate: true, delta: 1 }); // first update, executed immediately callback({ ...updateCallbackPayload, data: 2, isUpdate: true, delta: 1 }); // first update, should be excluded
callback({ ...updateCallbackPayload, data: 3, isUpdate: true, delta: 1 }); // next update, should be excluded callback({ ...updateCallbackPayload, data: 3, isUpdate: true, delta: 1 }); // next update, should be excluded
callback({ ...updateCallbackPayload, data: 4, isUpdate: true, delta: 1 }); // next update, should be excluded callback({ ...updateCallbackPayload, data: 4, isUpdate: true, delta: 1 }); // next update, should be excluded
callback({ ...updateCallbackPayload, data: 5, isUpdate: true, delta: 1 }); // last update, should be executed after timeout callback({ ...updateCallbackPayload, data: 5, isUpdate: true, delta: 1 }); // last update, should be executed after timeout
}); });
expect(result.current.data).toEqual(2); expect(result.current.data).toEqual(1);
await act(async () => { await act(async () => {
jest.runAllTimers(); jest.runAllTimers();
}); });

View File

@ -6,42 +6,26 @@ import type {
Subscribe, Subscribe,
Load, Load,
UpdateCallback, UpdateCallback,
UpdateDelta,
} from '../lib/generic-data-provider'; } from '../lib/generic-data-provider';
function hasDelta<T>( export interface useDataProviderParams<
updateData: UpdateDelta<T>
): updateData is Required<UpdateDelta<T>> {
return !!updateData.isUpdate;
}
interface useDataProviderParams<
Data, Data,
Delta, Delta,
Variables extends OperationVariables = OperationVariables Variables extends OperationVariables = OperationVariables
> { > {
dataProvider: Subscribe<Data, Delta, Variables>; dataProvider: Subscribe<Data, Delta, Variables>;
update?: ({ update?: ({ delta, data }: { delta?: Delta; data: Data | null }) => boolean;
delta,
data,
variables,
}: {
delta?: Delta;
data: Data | null;
variables?: Variables;
}) => boolean;
insert?: ({ insert?: ({
insertionData, insertionData,
data, data,
totalCount, totalCount,
}: { }: {
insertionData: Data; insertionData?: Data | null;
data: Data | null; data: Data | null;
totalCount?: number; totalCount?: number;
}) => boolean; }) => boolean;
variables?: Variables; variables?: Variables;
updateOnInit?: boolean; skipUpdates?: boolean;
noUpdate?: boolean;
skip?: boolean; skip?: boolean;
} }
@ -61,8 +45,7 @@ export const useDataProvider = <
update, update,
insert, insert,
variables, variables,
updateOnInit, skipUpdates,
noUpdate,
skip, skip,
}: useDataProviderParams<Data, Delta, Variables>) => { }: useDataProviderParams<Data, Delta, Variables>) => {
const client = useApolloClient(); const client = useApolloClient();
@ -91,57 +74,53 @@ export const useDataProvider = <
return Promise.reject(); return Promise.reject();
}, []); }, []);
const callback = useCallback<UpdateCallback<Data, Delta>>( const callback = useCallback<UpdateCallback<Data, Delta>>(
(arg) => { (args) => {
const { const {
data, data,
delta,
error, error,
loading, loading,
insertionData, insertionData,
totalCount, totalCount,
isInsert, isInsert,
isUpdate, isUpdate,
} = arg; } = args;
setError(error); setError(error);
setLoading(loading); setLoading(loading);
// if update or insert function returns true it means that component handles updates // if update or insert function returns true it means that component handles updates
// component can use flush() which will call callback without delta and cause data state update // component can use flush() which will call callback without delta and cause data state update
if (initialized.current) { if (!loading) {
if ( if (isUpdate && !skipUpdates && update && update({ delta, data })) {
isUpdate &&
!noUpdate &&
update &&
hasDelta<Delta>(arg) &&
update({ delta: arg.delta, data, variables })
) {
return; return;
} }
if ( if (isInsert && insert && insert({ insertionData, data, totalCount })) {
isInsert &&
insert &&
(!insertionData || insert({ insertionData, data, totalCount }))
) {
return; return;
} }
} }
setTotalCount(totalCount); setTotalCount(totalCount);
setData(data); setData(data);
if (updateOnInit && !initialized.current && update) { if (!loading && !initialized.current) {
initialized.current = true;
if (update) {
update({ data }); update({ data });
} }
initialized.current = true; }
}, },
[update, insert, noUpdate, updateOnInit, variables] [update, insert, skipUpdates]
); );
useEffect(() => { useEffect(() => {
setData(null); setData(null);
setError(undefined); setError(undefined);
setTotalCount(undefined); setTotalCount(undefined);
initialized.current = false;
if (skip) { if (skip) {
setLoading(false); setLoading(false);
if (update) {
update({ data: null });
}
return; return;
} }
setLoading(true); setLoading(true);
initialized.current = false;
const { unsubscribe, flush, reload, load } = dataProvider( const { unsubscribe, flush, reload, load } = dataProvider(
callback, callback,
client, client,
@ -150,8 +129,13 @@ export const useDataProvider = <
flushRef.current = flush; flushRef.current = flush;
reloadRef.current = reload; reloadRef.current = reload;
loadRef.current = load; loadRef.current = load;
return unsubscribe; return () => {
}, [client, initialized, dataProvider, callback, variables, skip]); flushRef.current = undefined;
reloadRef.current = undefined;
loadRef.current = undefined;
return unsubscribe();
};
}, [client, initialized, dataProvider, callback, variables, skip, update]);
return { data, loading, error, flush, reload, load, totalCount }; return { data, loading, error, flush, reload, load, totalCount };
}; };

View File

@ -8,13 +8,9 @@ import type {
import type { Subscription } from 'zen-observable-ts'; import type { Subscription } from 'zen-observable-ts';
import isEqual from 'lodash/isEqual'; import isEqual from 'lodash/isEqual';
import type { Schema } from '@vegaprotocol/types'; import type { Schema } from '@vegaprotocol/types';
interface UpdateData<Data, Delta> {
export interface UpdateDelta<Delta> {
delta?: Delta; delta?: Delta;
isUpdate?: boolean; isUpdate?: boolean;
}
interface UpdateData<Data, Delta> extends UpdateDelta<Delta> {
insertionData?: Data | null; insertionData?: Data | null;
isInsert?: boolean; isInsert?: boolean;
} }

View File

@ -85,7 +85,6 @@ export const TradesContainer = ({ marketId }: TradesContainerProps) => {
update, update,
insert, insert,
variables, variables,
updateOnInit: true,
}); });
totalCountRef.current = totalCount; totalCountRef.current = totalCount;
const getRows = makeInfiniteScrollGetRows<TradeEdge>( const getRows = makeInfiniteScrollGetRows<TradeEdge>(