Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: SendProvider #2078

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

feat: SendProvider #2078

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b0f51bc
SendProvider, components use context
brendan-defi Mar 4, 2025
c766c53
fix type
brendan-defi Mar 4, 2025
bf36e0d
remove constants
brendan-defi Mar 4, 2025
0cc4028
derive initialized state from eth balance
brendan-defi Mar 4, 2025
1b97092
fix useExchangeRate hook
brendan-defi Mar 4, 2025
59ff71d
reset values if no token
brendan-defi Mar 5, 2025
27a8976
set error in catch
brendan-defi Mar 5, 2025
40855cf
update tests
brendan-defi Mar 5, 2025
b0a0c74
refactor provider
brendan-defi Mar 5, 2025
66e1998
fix test name
brendan-defi Mar 5, 2025
27739d3
default exchange rate to 0, not undefined
brendan-defi Mar 5, 2025
ee481d0
fix tests
brendan-defi Mar 5, 2025
67bad8c
update types
brendan-defi Mar 5, 2025
2fc35a4
better error handling
brendan-defi Mar 5, 2025
4a91dc4
Merge branch 'main' into bf/send-prereqs
brendan-defi Mar 5, 2025
b49a389
remove memo
brendan-defi Mar 5, 2025
059d701
fix type narrowing
brendan-defi Mar 5, 2025
cfb66ef
Merge branch 'main' into bf/send-prereqs
brendan-defi Mar 6, 2025
6532df7
add usePriceQuote
brendan-defi Mar 6, 2025
7e5be49
refactor useExchangeRate with usePriceQuote
brendan-defi Mar 6, 2025
da4eef7
incorporate useExchageRate updates
brendan-defi Mar 6, 2025
b58a0bf
readability refactor
brendan-defi Mar 6, 2025
36e4557
fix lints
brendan-defi Mar 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 20 additions & 44 deletions src/internal/hooks/useExchangeRate.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,25 @@ describe('useExchangeRate', () => {
vi.resetAllMocks();
});

it('should return undefined without calling setExchangeRate if a token is not provided', async () => {
it('should return nullish values without calling setExchangeRate if a token is not provided', async () => {
const mockSetExchangeRate = vi.fn();
const { result } = renderHook(() =>
useExchangeRate({
token: undefined as unknown as PriceQuoteToken,
selectedInputType: 'crypto',
setExchangeRate: mockSetExchangeRate,
setExchangeRateLoading: vi.fn(),
}),
);

const resolvedValue = await result.current;
expect(resolvedValue).toBeUndefined();
expect(resolvedValue).toEqual({
isLoading: false,
exchangeRate: 0,
error: null,
});
expect(mockSetExchangeRate).not.toHaveBeenCalled();
});

it('should set the correct exchange rate when the selected input type is crypto', async () => {
const mockSetExchangeRate = vi.fn();
const mockSetExchangeRateLoading = vi.fn();

(getPriceQuote as Mock).mockResolvedValue({
priceQuote: [
{
Expand All @@ -46,24 +45,19 @@ describe('useExchangeRate', () => {
],
});

renderHook(() =>
const { result } = renderHook(() =>
useExchangeRate({
token: ethToken.symbol as PriceQuoteToken,
selectedInputType: 'crypto',
setExchangeRate: mockSetExchangeRate,
setExchangeRateLoading: mockSetExchangeRateLoading,
}),
);

await waitFor(() => {
expect(mockSetExchangeRate).toHaveBeenCalledWith(1 / 2400);
expect(result.current.exchangeRate).toEqual(1 / 2400);
});
});

it('should set the correct the exchange rate when the selected input type is fiat', async () => {
const mockSetExchangeRate = vi.fn();
const mockSetExchangeRateLoading = vi.fn();

(getPriceQuote as Mock).mockResolvedValue({
priceQuote: [
{
Expand All @@ -76,60 +70,44 @@ describe('useExchangeRate', () => {
],
});

renderHook(() =>
const { result } = renderHook(() =>
useExchangeRate({
token: ethToken.symbol as PriceQuoteToken,
selectedInputType: 'fiat',
setExchangeRate: mockSetExchangeRate,
setExchangeRateLoading: mockSetExchangeRateLoading,
}),
);

await waitFor(() => {
expect(mockSetExchangeRate).toHaveBeenCalledWith(2400);
expect(result.current.exchangeRate).toEqual(2400);
});
});

it('should log an error and not set the exchange rate when the api call returns an error', async () => {
const mockSetExchangeRate = vi.fn();
const mockSetExchangeRateLoading = vi.fn();
const consoleSpy = vi.spyOn(console, 'error');

(getPriceQuote as Mock).mockResolvedValue({
error: 'test error',
});

renderHook(() =>
const { result } = renderHook(() =>
useExchangeRate({
token: ethToken.symbol as PriceQuoteToken,
selectedInputType: 'fiat',
setExchangeRate: mockSetExchangeRate,
setExchangeRateLoading: mockSetExchangeRateLoading,
}),
);

await waitFor(() => {
expect(consoleSpy).toHaveBeenCalledWith(
'Error fetching price quote:',
'test error',
);
expect(mockSetExchangeRate).not.toHaveBeenCalled();
expect(result.current.error).toEqual('test error');
});
});

it('should log an error and not set the exchange rate when the api fails', async () => {
const mockSetExchangeRate = vi.fn();
const mockSetExchangeRateLoading = vi.fn();
const consoleSpy = vi.spyOn(console, 'error');

(getPriceQuote as Mock).mockRejectedValue(new Error('test error'));

renderHook(() =>
const { result } = renderHook(() =>
useExchangeRate({
token: ethToken.symbol as PriceQuoteToken,
selectedInputType: 'fiat',
setExchangeRate: mockSetExchangeRate,
setExchangeRateLoading: mockSetExchangeRateLoading,
}),
);

Expand All @@ -138,14 +116,11 @@ describe('useExchangeRate', () => {
'Uncaught error fetching price quote:',
expect.any(Error),
);
expect(mockSetExchangeRate).not.toHaveBeenCalled();
expect(result.current.error).toEqual('Error: test error');
});
});

it('should set and unset loading state', async () => {
const mockSetExchangeRate = vi.fn();
const mockSetExchangeRateLoading = vi.fn();

(getPriceQuote as Mock).mockResolvedValue({
priceQuote: [
{
Expand All @@ -158,18 +133,19 @@ describe('useExchangeRate', () => {
],
});

renderHook(() =>
const { result } = renderHook(() =>
useExchangeRate({
token: ethToken.symbol as PriceQuoteToken,
selectedInputType: 'crypto',
setExchangeRate: mockSetExchangeRate,
setExchangeRateLoading: mockSetExchangeRateLoading,
}),
);

await waitFor(() => {
expect(mockSetExchangeRateLoading).toHaveBeenCalledWith(true);
expect(mockSetExchangeRateLoading).toHaveBeenLastCalledWith(false);
expect(result.current.isLoading).toEqual(true);
});

await waitFor(() => {
expect(result.current.isLoading).toEqual(false);
});
});
});
81 changes: 48 additions & 33 deletions src/internal/hooks/useExchangeRate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,47 +2,62 @@ import { getPriceQuote } from '@/api';
import type { PriceQuoteToken } from '@/api/types';
import { RequestContext } from '@/core/network/constants';
import { isApiError } from '@/internal/utils/isApiResponseError';
import type { Dispatch, SetStateAction } from 'react';
import { useEffect, useState } from 'react';

type UseExchangeRateParams = {
token: PriceQuoteToken;
token: PriceQuoteToken | undefined | '';
Copy link
Contributor

@alessey alessey Mar 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

token?: PriceQuoteToken | ''? why allow ''?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I use useExchangeRate in SendProvider, I'm passing in a PortfolioTokenWithFiatValue.address:
image

PortfolioTokenWithFiatValue inherits from Token type.
image

And Token allows empty string for address:
image

i suppose I could add a type assertion in SendProvider if that's preferred:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could update the type to reflect that address: '' is only valid for ETH, but in general, I try not to pass through implementation details from other components.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think i have a better solve. in the provider, i'll test against address, not symbol. this will resolve the type narrowing issue:

token: selectedToken?.address === '' ? 'ETH' : selectedToken?.address,

selectedInputType: 'crypto' | 'fiat';
setExchangeRate: Dispatch<SetStateAction<number>>;
setExchangeRateLoading?: Dispatch<SetStateAction<boolean>>;
};

export async function useExchangeRate({
export function useExchangeRate({
token,
selectedInputType,
setExchangeRate,
setExchangeRateLoading,
}: UseExchangeRateParams) {
if (!token) {
return;
}

setExchangeRateLoading?.(true);

try {
const response = await getPriceQuote(
{ tokens: [token] },
RequestContext.Wallet,
);
if (isApiError(response)) {
console.error('Error fetching price quote:', response.error);
const [isLoading, setIsLoading] = useState<boolean>(false);
const [exchangeRate, setExchangeRate] = useState<number>(0);
const [error, setError] = useState<string | null>(null);

useEffect(() => {
if (!token) {
setIsLoading(false);
setExchangeRate(0);
setError(null);
return;
}
const priceQuote = response.priceQuote[0];

const rate =
selectedInputType === 'crypto'
? 1 / Number(priceQuote.price)
: Number(priceQuote.price);

setExchangeRate(rate);
} catch (error) {
console.error('Uncaught error fetching price quote:', error);
} finally {
setExchangeRateLoading?.(false);
}

setIsLoading(true);
setError(null);

getPriceQuote({ tokens: [token] }, RequestContext.Wallet)
Copy link
Contributor

@alessey alessey Mar 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could wrap this in a react-query hook, i.e. https://github.com/coinbase/onchainkit/blob/main/src/nft/hooks/useMintDetails.ts and export that along with the API and then just use that hook to calculate exchange rate here or in the consumer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alessey, based on this feedback I created usePriceQuote hook and integrated into useExchangeRate. please let me know if that's what you were suggesting

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this call always be in the Wallet context? If so should this hook be moved into src/internal/wallet/hooks?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right now useExchangeRate is only used in Wallet, but I could imagine this being useful in many other contexts. Happy to move if you prefer, but I think it's very likely we'll end up moving to internal later

.then((response) => {
if (isApiError(response)) {
setError(response.error);
setExchangeRate(0);
console.error('Error fetching price quote:', response.error);
return;
}

const priceQuote = response.priceQuote[0];
const rate =
selectedInputType === 'crypto'
? 1 / Number(priceQuote.price)
: Number(priceQuote.price);

setExchangeRate(rate);
setError(null);
})
.catch((error) => {
setError(String(error));
console.error('Uncaught error fetching price quote:', error);
})
.finally(() => {
setIsLoading(false);
});
}, [token, selectedInputType]);

return {
isLoading,
exchangeRate,
error,
};
}
Loading