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.

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

re-enable react hooks lint #1510

Merged
merged 9 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const DestinationAddr = () => {
// Set initial account to trigger address loading
useEffect(() => {
setAccount(0);
}, []);
}, [setAccount]);

return (
<div className='mb-2 flex w-full flex-col gap-1 text-stone-700'>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,10 @@ 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 });
return () => ac.abort();
}, [inputRef]);
turbocrime marked this conversation as resolved.
Show resolved Hide resolved

return inputRef;
};
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const BalanceItem = ({ asset, value, onSelect }: BalanceItemProps) => {
return metadataFromValue?.equals(metadataFromAsset);
}
return false;
}, [asset, value]);
}, [asset, metadataFromAsset, metadataFromValue, value]);

return (
<DialogClose asChild onClick={() => onSelect(asset)}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export const useStakingTokensAndFilter = (
(prev, curr) => toAccountSwitcherFilter(prev, curr, stakingTokenMetadata),
[],
);
}, [balancesByAccount]);
}, [balancesByAccount, stakingTokenMetadata]);

const stakingTokens = stakingTokensByAccount.get(account);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export const TokenSwapInput = () => {
} else {
return priceHistory.load();
}
}, [assetIn, assetOut]);
}, [assetIn, assetOut, priceHistory]);
turbocrime marked this conversation as resolved.
Show resolved Hide resolved

useEffect(() => {
if (!priceHistory.candles.length) {
Expand Down
6 changes: 2 additions & 4 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',

Expand Down
27 changes: 4 additions & 23 deletions packages/react/src/components/penumbra-context-provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -107,14 +107,7 @@ export const PenumbraContextProvider = ({
break;
}
}
}, [
failure,
makeApprovalRequest,
penumbra?.request,
providerManifest,
providerState,
setFailure,
]);
}, [failure, makeApprovalRequest, penumbra, providerManifest, providerState, setFailure]);

// connect effect
useEffect(() => {
Expand All @@ -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(
() => ({
Expand Down Expand Up @@ -181,12 +167,7 @@ export const PenumbraContextProvider = ({
}),
[
failure,
penumbra?.addEventListener,
penumbra?.connect,
penumbra?.disconnect,
penumbra?.manifest,
penumbra?.removeEventListener,
penumbra?.request,
penumbra,
providerConnected,
providerManifest,
providerPort,
Expand Down
55 changes: 27 additions & 28 deletions packages/ui/components/ui/candlestick-plot/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,31 @@ export const CandlestickPlot = withTooltip<CandlestickPlotProps, CandlestickData
}: CandlestickPlotProps & WithTooltipProvidedProps<CandlestickData>) => {
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<number>({
range: [50, w - 5],
domain: [startBlock, latestKnownBlockHeight ?? endBlock],
});

const priceScale = scaleLinear<number>({
range: [h, 0],
domain: [minPrice - maxSpread / 2, maxPrice + maxSpread / 2],
});

return { priceScale, blockScale };
}, [candles, latestKnownBlockHeight, w, h]);

const useTooltip = useCallback(
(d: CandlestickData) => ({
Expand All @@ -105,32 +118,18 @@ export const CandlestickPlot = withTooltip<CandlestickPlotProps, CandlestickData
hideTooltip();
},
}),
[],
[blockScale, hideTooltip, priceScale, showTooltip],
);

if (!candles.length) {
return null;
}

// assertions here okay because we've just checked length
const startBlock = blockHeight(candles[0]!);
const endBlock = blockHeight(candles[candles.length - 1]!);

// candle width as fraction of graph width. likely too thin to really
// matter. if there's lots of records this will overlap, and we'll need to
// implement some kind of binning.
const blockWidth = Math.min(w / candles.length, 6);

const blockScale = scaleLinear<number>({
range: [50, w - 5],
domain: [startBlock, latestKnownBlockHeight ?? endBlock],
});

const priceScale = scaleLinear<number>({
range: [h, 0],
domain: [minPrice - maxSpread / 2, maxPrice + maxSpread / 2],
});

return (
<>
<div className={className ?? 'size-full'} ref={parentRef}>
Expand Down Expand Up @@ -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]!;
Expand Down
2 changes: 1 addition & 1 deletion packages/ui/components/ui/tx/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
};
Expand Down
33 changes: 21 additions & 12 deletions packages/zquery/src/get-use-hook.ts
turbocrime marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useRef } from 'react';
import { useEffect, useRef, useState } from 'react';
import {
AbridgedZQueryState,
CreateZQueryStreamingProps,
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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<ZQueryState<DataType | ProcessedDataType, FetchArgs> | undefined>();
const prevSelectedState = useRef<
Expand Down
Loading