diff --git a/.changeset/angry-guests-pay.md b/.changeset/angry-guests-pay.md new file mode 100644 index 0000000000..d47ed880f7 --- /dev/null +++ b/.changeset/angry-guests-pay.md @@ -0,0 +1,5 @@ +--- +'@penumbra-zone/zquery': patch +--- + +correct state dependency for fetch diff --git a/apps/minifront/src/components/ibc/ibc-in/destination-addr.tsx b/apps/minifront/src/components/ibc/ibc-in/destination-addr.tsx index 435ad877a5..7ae54f024e 100644 --- a/apps/minifront/src/components/ibc/ibc-in/destination-addr.tsx +++ b/apps/minifront/src/components/ibc/ibc-in/destination-addr.tsx @@ -16,7 +16,7 @@ export const DestinationAddr = () => { // Set initial account to trigger address loading useEffect(() => { setAccount(0); - }, []); + }, [setAccount]); return (
diff --git a/apps/minifront/src/components/shared/number-input/use-wheel-prevent.ts b/apps/minifront/src/components/shared/number-input/use-wheel-prevent.ts index 4a0d3975a3..7b80f9f3cc 100644 --- a/apps/minifront/src/components/shared/number-input/use-wheel-prevent.ts +++ b/apps/minifront/src/components/shared/number-input/use-wheel-prevent.ts @@ -15,11 +15,9 @@ export const useWheelPrevent = () => { }; useEffect(() => { - inputRef.current?.addEventListener('wheel', onWheel); - - return () => { - inputRef.current?.removeEventListener('wheel', onWheel); - }; + const ac = new AbortController(); + inputRef.current?.addEventListener('wheel', onWheel, { signal: ac.signal, passive: false }); + return () => ac.abort(); }, []); return inputRef; diff --git a/apps/minifront/src/components/shared/selectors/balance-item.tsx b/apps/minifront/src/components/shared/selectors/balance-item.tsx index 53f637cb08..87b9c75caf 100644 --- a/apps/minifront/src/components/shared/selectors/balance-item.tsx +++ b/apps/minifront/src/components/shared/selectors/balance-item.tsx @@ -36,7 +36,7 @@ export const BalanceItem = ({ asset, value, onSelect }: BalanceItemProps) => { return metadataFromValue?.equals(metadataFromAsset); } return false; - }, [asset, value]); + }, [asset, metadataFromAsset, metadataFromValue, value]); return ( onSelect(asset)}> diff --git a/apps/minifront/src/components/staking/account/use-staking-tokens-and-filter.ts b/apps/minifront/src/components/staking/account/use-staking-tokens-and-filter.ts index ae677f6407..fb70736ca8 100644 --- a/apps/minifront/src/components/staking/account/use-staking-tokens-and-filter.ts +++ b/apps/minifront/src/components/staking/account/use-staking-tokens-and-filter.ts @@ -87,7 +87,7 @@ export const useStakingTokensAndFilter = ( (prev, curr) => toAccountSwitcherFilter(prev, curr, stakingTokenMetadata), [], ); - }, [balancesByAccount]); + }, [balancesByAccount, stakingTokenMetadata]); const stakingTokens = stakingTokensByAccount.get(account); diff --git a/apps/minifront/src/components/swap/swap-form/token-swap-input.tsx b/apps/minifront/src/components/swap/swap-form/token-swap-input.tsx index 7bf250a3c3..2706b1a871 100644 --- a/apps/minifront/src/components/swap/swap-form/token-swap-input.tsx +++ b/apps/minifront/src/components/swap/swap-form/token-swap-input.tsx @@ -10,7 +10,7 @@ import { getMetadataFromBalancesResponseOptional, } from '@penumbra-zone/getters/balances-response'; import { ArrowRight } from 'lucide-react'; -import { useEffect, useMemo } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import { getBlockDate } from '../../../fetchers/block-date'; import { AllSlices } from '../../../state'; import { useStoreShallow } from '../../../utils/use-store-shallow'; @@ -79,23 +79,45 @@ export const TokenSwapInput = () => { return getDisplayDenomExponent.optional()(getMetadataFromBalancesResponseOptional(assetIn)); }, [assetIn]); + /** + * @todo this memo, state, and effects below are awkward. but this prevents + * excessive state churn. this is a temporary solution and will be corrected + * after implementing use of zquery for this data. + * + * @issue https://github.com/penumbra-zone/web/issues/1530 + */ + const loadPriceHistoryMemo = useMemo( + () => (assetIn && assetOut ? priceHistory.load : undefined), + [assetIn, assetOut, priceHistory.load], + ); + + const [updatedHeight, setUpdatedHeight] = useState(); + + // initial price history useEffect(() => { - if (!assetIn || !assetOut) { + if (updatedHeight ?? !loadPriceHistoryMemo) { return; - } else { - return priceHistory.load(); } - }, [assetIn, assetOut]); + setUpdatedHeight(latestKnownBlockHeight); + return loadPriceHistoryMemo(); + }, [assetIn, assetOut, latestKnownBlockHeight, loadPriceHistoryMemo, updatedHeight]); + // update price history useEffect(() => { - if (!priceHistory.candles.length) { + // don't attempt to update an absent price history + if (!updatedHeight || !loadPriceHistoryMemo) { return; - } else if (latestKnownBlockHeight % 10n) { + } + + // rate limit pricing updates + const distanceFromLatest = latestKnownBlockHeight - updatedHeight; + if (!distanceFromLatest || distanceFromLatest < 10n) { return; - } else { - return priceHistory.load(); } - }, [priceHistory, latestKnownBlockHeight]); + + setUpdatedHeight(latestKnownBlockHeight); + return loadPriceHistoryMemo(); + }, [priceHistory.candles, latestKnownBlockHeight, loadPriceHistoryMemo, updatedHeight]); const maxAmount = getAmount.optional()(assetIn); const maxAmountAsString = maxAmount ? joinLoHiAmount(maxAmount).toString() : undefined; diff --git a/apps/minifront/src/state/swap/helpers.ts b/apps/minifront/src/state/swap/helpers.ts index f273a76f90..af1c5d022f 100644 --- a/apps/minifront/src/state/swap/helpers.ts +++ b/apps/minifront/src/state/swap/helpers.ts @@ -140,7 +140,7 @@ export const sendCandlestickDataRequests = async ( }, ); - return combinedCandles; + return combinedCandles.sort((a, b) => Number(a.height - b.height)); }; const byBalanceDescending = (a: BalancesResponse, b: BalancesResponse) => { diff --git a/eslint.config.js b/eslint.config.js index 2d462681d0..ec8eac7270 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -74,15 +74,13 @@ export default tseslint.config( rules: { ...react.configs.recommended.rules, ...react_hooks.configs.recommended.rules, + 'react-hooks/exhaustive-deps': 'warn', + 'react-hooks/rules-of-hooks': 'error', }, }, { name: 'custom:react-wishlist-improvements', rules: { - // these were from a broken plugin. should be enabled and fixed. - 'react-hooks/exhaustive-deps': 'off', - 'react-hooks/rules-of-hooks': 'off', - // this plugin was formerly included, but was never actually applied. 'react-refresh/only-export-components': 'off', @@ -156,7 +154,6 @@ export default tseslint.config( 'error', { requireDefaultForNonUnion: true }, ], - curly: ['error', 'all'], eqeqeq: ['error', 'smart'], }, }, @@ -258,4 +255,9 @@ export default tseslint.config( // disable rules covered by prettier prettier, + + { + name: 'custom:prettier-would-disable', + rules: { curly: ['error', 'all'] }, + }, ); diff --git a/packages/react/src/components/penumbra-context-provider.tsx b/packages/react/src/components/penumbra-context-provider.tsx index 5e2268b42b..71091701c3 100644 --- a/packages/react/src/components/penumbra-context-provider.tsx +++ b/packages/react/src/components/penumbra-context-provider.tsx @@ -91,7 +91,7 @@ export const PenumbraContextProvider = ({ { signal: ac.signal }, ); return () => ac.abort(); - }, [failure, penumbra?.addEventListener, providerManifest, penumbra?.manifest, setFailure]); + }, [failure, providerManifest, setFailure, penumbra]); // request effect useEffect(() => { @@ -107,14 +107,7 @@ export const PenumbraContextProvider = ({ break; } } - }, [ - failure, - makeApprovalRequest, - penumbra?.request, - providerManifest, - providerState, - setFailure, - ]); + }, [failure, makeApprovalRequest, penumbra, providerManifest, providerState, setFailure]); // connect effect useEffect(() => { @@ -139,14 +132,7 @@ export const PenumbraContextProvider = ({ break; } } - }, [ - failure, - makeApprovalRequest, - penumbra?.connect, - providerManifest, - providerState, - setFailure, - ]); + }, [failure, makeApprovalRequest, penumbra, providerManifest, providerState, setFailure]); const createdContext: PenumbraContext = useMemo( () => ({ @@ -181,12 +167,7 @@ export const PenumbraContextProvider = ({ }), [ failure, - penumbra?.addEventListener, - penumbra?.connect, - penumbra?.disconnect, - penumbra?.manifest, - penumbra?.removeEventListener, - penumbra?.request, + penumbra, providerConnected, providerManifest, providerPort, diff --git a/packages/ui/components/ui/candlestick-plot/index.tsx b/packages/ui/components/ui/candlestick-plot/index.tsx index a25500393e..73da1da586 100644 --- a/packages/ui/components/ui/candlestick-plot/index.tsx +++ b/packages/ui/components/ui/candlestick-plot/index.tsx @@ -79,18 +79,31 @@ export const CandlestickPlot = withTooltip) => { const { parentRef, width: w, height: h } = useParentSize({ debounceTime: 150 }); - const { maxPrice, minPrice } = useMemo( - () => - candles.reduce( - (acc, d) => ({ - minPrice: Math.min(acc.minPrice, lowPrice(d)), - maxPrice: Math.max(acc.maxPrice, highPrice(d)), - }), - { minPrice: Infinity, maxPrice: -Infinity }, - ), - [candles], - ); - const maxSpread = maxPrice - minPrice; + const { blockScale, priceScale } = useMemo(() => { + const { maxPrice, minPrice } = candles.reduce( + (acc, d) => ({ + minPrice: Math.min(acc.minPrice, lowPrice(d)), + maxPrice: Math.max(acc.maxPrice, highPrice(d)), + }), + { minPrice: Infinity, maxPrice: -Infinity }, + ); + const maxSpread = maxPrice - minPrice; + + const startBlock = candles.length && blockHeight(candles[0]!); + const endBlock = candles.length && blockHeight(candles[candles.length - 1]!); + + const blockScale = scaleLinear({ + range: [50, w - 5], + domain: [startBlock, latestKnownBlockHeight ?? endBlock], + }); + + const priceScale = scaleLinear({ + range: [h, 0], + domain: [minPrice - maxSpread / 2, maxPrice + maxSpread / 2], + }); + + return { priceScale, blockScale }; + }, [candles, latestKnownBlockHeight, w, h]); const useTooltip = useCallback( (d: CandlestickData) => ({ @@ -105,32 +118,18 @@ export const CandlestickPlot = withTooltip({ - range: [50, w - 5], - domain: [startBlock, latestKnownBlockHeight ?? endBlock], - }); - - const priceScale = scaleLinear({ - range: [h, 0], - domain: [minPrice - maxSpread / 2, maxPrice + maxSpread / 2], - }); - return ( <>
@@ -266,7 +265,7 @@ export const CandlesticksTooltip = ({ const ac = new AbortController(); void getBlockDate(data.height, ac.signal).then(setBlockDate); return () => ac.abort('Abort tooltip date query'); - }, [data]); + }, [data, getBlockDate]); const endBase = endMetadata.denomUnits.filter(d => !d.exponent)[0]!; const startBase = startMetadata.denomUnits.filter(d => !d.exponent)[0]!; diff --git a/packages/ui/components/ui/tx/index.tsx b/packages/ui/components/ui/tx/index.tsx index ab7bc2ee85..0bcb2d93b5 100644 --- a/packages/ui/components/ui/tx/index.tsx +++ b/packages/ui/components/ui/tx/index.tsx @@ -49,7 +49,7 @@ const useFeeMetadata = (txv: TransactionView, getMetadata: MetadataFetchFn) => { setIsLoading(false); }) .catch((e: unknown) => setError(e)); - }, [txv, getMetadata, setFeeValueView]); + }, [txv, getMetadata, setFeeValueView, amount]); return { feeValueView, isLoading, error }; }; diff --git a/packages/zquery/src/get-use-hook.ts b/packages/zquery/src/get-use-hook.ts index be0f9c0e19..5b0a0f488b 100644 --- a/packages/zquery/src/get-use-hook.ts +++ b/packages/zquery/src/get-use-hook.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { AbridgedZQueryState, CreateZQueryStreamingProps, @@ -7,6 +7,9 @@ import { ZQueryState, } from './types.js'; +const shallowCompareArrays = (a: unknown[], b: unknown[]): boolean => + a.every((item, index) => item === b[index]) && a.length === b.length; + /** * Returns a hook that can be used via `use[Name]()` to access the ZQuery state. */ @@ -72,29 +75,35 @@ export const getUseHook = < ) => { const useStore = props.getUseStore(); + // We want to use a custom comparator to see if `fetchArgs` changed. + // `useMemo()` does not support custom comparators, so we'll roll it ourself + // using a combination of `useState` and `useEffect`. + const [fetchArgsMemo, setFetchArgsMemo] = useState(fetchArgs); useEffect(() => { - const fetch = props.get(useStore.getState())._zQueryInternal.fetch; + if (!shallowCompareArrays(fetchArgs, fetchArgsMemo)) { + setFetchArgsMemo(fetchArgs); + } + }, [fetchArgs, fetchArgsMemo]); - { - const newReferenceCount = incrementReferenceCounter(); + useEffect(() => { + const fetch = props.get(useStore.getState())._zQueryInternal.fetch; - if (newReferenceCount === 1) { - setAbortController(new AbortController()); - void fetch(...fetchArgs); - } + const incrementedReferenceCount = incrementReferenceCounter(); + if (incrementedReferenceCount === 1) { + setAbortController(new AbortController()); + void fetch(...fetchArgsMemo); } const onUnmount = () => { - const newReferenceCount = decrementReferenceCounter(); - - if (newReferenceCount === 0) { + const decrementedReferenceCount = decrementReferenceCounter(); + if (decrementedReferenceCount === 0) { props.get(useStore.getState())._zQueryInternal.abortController?.abort(); setAbortController(undefined); } }; return onUnmount; - }, [fetch]); + }, [fetchArgsMemo, useStore]); const prevState = useRef | undefined>(); const prevSelectedState = useRef<