Skip to content
This repository has been archived by the owner on Jun 28, 2021. It is now read-only.

Refactor "My Trades" and "All Trades" to use react-query instead of redux #513

Open
evgenyboxer opened this issue Aug 25, 2020 · 1 comment

Comments

@evgenyboxer
Copy link
Contributor

evgenyboxer commented Aug 25, 2020

We are in the process of moving data fetching to use react-query instead of handling it in redux (via sagas).
Please checkout the docs for it - https://github.com/tannerlinsley/react-query
If you are new to sagas, its basically a way to handle side effects in redux, similarly to what redux-thunk gives us (but way more robust). Our use of sagas is fairly simple, so migrating it to react-query is trivial.

Currently, both "My Trades" and "All Trades" use sagas whereas the recently added "Options Trade" view is using react-query. Its a great reference on how to use it in our codebase.

The trade view located here - https://synthetix.exchange/#/trade/sBTC-sUSD (under the chart/order card).
You need to be logged in to view "My Trades".

Options trade view - https://synthetix.exchange/#/options/0xa59a361e670ca290b5821bbc7b83fe0ed14791d2 (under chart/order card). In the "Recent/Your" activity tabs.

The PR should be on the dev branch.

Let us know if you got any questions!

@evgenyboxer evgenyboxer changed the title Refactor "My Trades" and "All Trades" to use react-query instead of sagas. Refactor "My Trades" and "All Trades" to use react-query instead of redux. Aug 25, 2020
@evgenyboxer evgenyboxer changed the title Refactor "My Trades" and "All Trades" to use react-query instead of redux. Refactor "My Trades" and "All Trades" to use react-query instead of redux Aug 25, 2020
@jwineman
Copy link
Contributor

jwineman commented Aug 26, 2020

I'm planning on PRing <MyTrades/> tomorrow with the following react-query logic:

  1. useQuery for snxData.exchanger.exchangeEntriesSettled with refetchInterval: REFRESH_INTERVAL
  2. useQuery for snxData.exchanges.since with enabled set to the data of step 1.

I assume this logic should move into the second hook/component now right based off the data from the preceeding two useQuerys?

			normalizedTrades.forEach((trade, idx) => {
				// if the input size gets large, a binary search could be used.
				const settledTrade = settledTrades.find(
					(settledTrade) =>
						trade.timestamp === settledTrade.exchangeTimestamp &&
						settledTrade.dest === trade.toCurrencyKey &&
						settledTrade.src === trade.fromCurrencyKey &&
						trade.fromAmount === settledTrade.amount
				);

				normalizedTrades[idx].isSettled = false;

				if (settledTrade) {
					normalizedTrades[idx].rebate = settledTrade.rebate;
					normalizedTrades[idx].reclaim = settledTrade.reclaim;

					// special case for when the currency is priced in sUSD
					const feeReclaimRebateAmount =
						trade.toCurrencyKey === SYNTHS_MAP.sUSD
							? settledTrade.rebate - settledTrade.reclaim
							: settledTrade.reclaim - settledTrade.rebate;

					// ( shiftAmount / amount ) * price -> gets us the price shift
					// to get the new price, we just add the price shift (which might be a negative or positive number)
					normalizedTrades[idx].settledPrice =
						(feeReclaimRebateAmount / trade.toAmount) * trade.price + trade.price;
					normalizedTrades[idx].isSettled = true;
					normalizedTrades[idx].amount = Math.abs(feeReclaimRebateAmount);
				}

I'm not sure how to type settledTrades yet and I'm having trouble getting react-querys enabled to work correctly right now.

jwineman pushed a commit that referenced this issue Sep 10, 2020
refactor: #513 migrate <AllTrades/> to react-query.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants