Conversation
WalkthroughThis PR refactors state management across 70+ components by replacing useState/useEffect-based derived state with memoized computations (useMemo, useCallback, useEffectEvent). Mounted guards and cancellation mechanisms are introduced for async operations. A new HTML workbench tool for lint file editing is added. Changes
Sequence Diagram(s)sequenceDiagram
participant Render as Component Render
participant Memo as useMemo/useCallback
participant Effect as useEffect
Note over Render,Effect: Before: State-Based Derivation
Render->>Render: Initial render
Render->>Effect: Mount effect
Effect->>Effect: Compute derived state
Effect->>Render: setState() → re-render
Render->>Render: Render with derived state
Note over Render,Memo: After: Memoized Derivation
Render->>Memo: Dependency check
Memo->>Memo: Compute/memoize value
Render->>Render: Render with memoized value
Note over Render,Memo: No state update cycle
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
components/groups/page/list/card/GroupCardActionWrapper.tsx (1)
32-40: Handle division by zero edge case.The function doesn't guard against
membersCountbeing0, which would result inInfinity%orNaN%as the CSS width value, breaking the progress bar rendering.Apply this diff to handle the edge case:
const getProgress = (): string => { if ( typeof membersCount !== "number" || - typeof doneMembersCount !== "number" + typeof doneMembersCount !== "number" || + membersCount === 0 ) { return "0%"; } return `${(doneMembersCount / membersCount) * 100}%`; };components/user/settings/UserSettingsUsername.tsx (1)
37-37: Inconsistent dependency arrays between sync effects.Line 71 now includes
setIsAvailablein the dependency array, but line 37's similar effect doesn't includesetIsLoading. For consistency and to satisfy React's exhaustive-deps lint rule, both should include their respective setter functions.Apply this diff to add
setIsLoadingto line 37's dependency array:- useEffect(() => setIsLoading(loading), [loading]); + useEffect(() => setIsLoading(loading), [loading, setIsLoading]);Optional: Consider eliminating sync effects entirely.
Given this PR's theme of replacing useState/useEffect patterns with derived state, consider refactoring to eliminate these "sync effects" that propagate derived values upward. Instead, the parent component could either:
- Compute
isAvailableandisLoadingdirectly from the state this component manages, or- Receive and use a callback that accepts the full state object
This would reduce unnecessary effect cycles and make the data flow more explicit.
Also applies to: 69-71
components/subscriptions-report/SubscriptionsReport.tsx (1)
99-125: Add cancellation mechanism to prevent state updates on unmount.The initial fetch effect lacks a cancellation guard. If the component unmounts during the fetch, state updates at lines 115-117 and 119-121 will be attempted on an unmounted component, potentially causing warnings.
Apply this pattern consistent with the second effect:
useEffect(() => { + let isCancelled = false; const fetchData = async () => { try { let remainingCountForSeason = getCardsRemainingUntilEndOf("szn"); const redeemed = await fetchRedeemedCounts(1); if (isMintingToday()) { const latestDrop = redeemed.data[0]; if (latestDrop?.mint_date) { const mintDate = Time.fromString(latestDrop.mint_date); if (mintDate.toIsoDateString() !== Time.now().toIsoDateString()) { remainingCountForSeason += 1; } } } const upcoming = await fetchUpcomingCounts(remainingCountForSeason); - setRedeemedCounts(redeemed.data); - setTotalRedeemed(redeemed.count); - setUpcomingCounts(upcoming); + if (!isCancelled) { + setRedeemedCounts(redeemed.data); + setTotalRedeemed(redeemed.count); + setUpcomingCounts(upcoming); + } } finally { - hasLoadedInitialRedeemedPage.current = true; - setRedeemedLoading(false); - setUpcomingLoading(false); + if (!isCancelled) { + hasLoadedInitialRedeemedPage.current = true; + setRedeemedLoading(false); + setUpcomingLoading(false); + } } }; fetchData(); + return () => { + isCancelled = true; + }; }, []);components/user/waves/UserPageWaves.tsx (1)
33-46: Incomplete refactoring: Apply the same pattern used forusePublicWaves.Lines 48-49 correctly refactor derived state to a direct constant, but this segment still uses the old
useState+useEffectpattern that the PR aims to eliminate. TheshowCreateNewWaveButtonvalue is fully derived fromconnectedProfile,activeProfileProxy, andprofile, so it should be computed directly likeusePublicWaves.Apply this diff to complete the refactoring:
- const getShowCreateNewWaveButton = () => { - return ( - !!connectedProfile?.handle && - !activeProfileProxy && - connectedProfile.handle === profile.handle - ); - }; - const [showCreateNewWaveButton, setShowCreateNewWaveButton] = useState( - getShowCreateNewWaveButton() - ); - - useEffect(() => { - setShowCreateNewWaveButton(getShowCreateNewWaveButton()); - }, [connectedProfile, profile, activeProfileProxy]); + const showCreateNewWaveButton = + !!connectedProfile?.handle && + !activeProfileProxy && + connectedProfile.handle === profile.handle;components/nextGen/admin/NextGenAdminSetPhases.tsx (3)
140-154: Critical: Missing dependencies in useEffect causes stale closure bug.The effect only depends on
[submitting], but usescollectionID,selectedAllowlist,publicStartTime,publicEndTime, andcontractWriteproperties. If any of these values change between whensubmittingis set to true and when the effect runs, stale values will be sent to the contract.Apply this diff to fix the missing dependencies:
useEffect(() => { if (submitting) { contractWrite.writeContract({ ...contractWrite.params, args: [ collectionID, selectedAllowlist?.start_time ?? publicStartTime, selectedAllowlist?.end_time ?? publicStartTime, publicStartTime, publicEndTime, selectedAllowlist?.merkle_root ?? NULL_MERKLE, ], }); } - }, [submitting]); + }, [ + submitting, + collectionID, + selectedAllowlist, + publicStartTime, + publicEndTime, + contractWrite, + ]);
156-161: Critical: Invalid dependency array with expression.Line 161 uses an expression
contractWrite.isSuccess || contractWrite.isErrorin the dependency array. React's dependency arrays must contain actual values, not expressions. This will cause the effect to check if the boolean result changes, not if the individual properties change.Apply this diff:
useEffect(() => { if (contractWrite.isSuccess || contractWrite.isError) { setLoading(false); setSubmitting(false); } - }, [contractWrite.isSuccess || contractWrite.isError]); + }, [contractWrite.isSuccess, contractWrite.isError]);
73-81: Add cleanup for async operation to prevent setState on unmounted component.The async fetch doesn't have a mounted guard or cleanup mechanism. If the component unmounts or
collectionIDchanges before the promise resolves, it will callsetAvailableAllowlistCollectionson an unmounted component, causing React warnings and potential memory leaks.Apply this diff to add cleanup:
useEffect(() => { + let mounted = true; if (collectionID) { commonApiFetch<NextgenAllowlistCollection[]>({ endpoint: `nextgen/allowlist_phases/${collectionID}`, }).then((collections) => { - setAvailableAllowlistCollections(collections); + if (mounted) { + setAvailableAllowlistCollections(collections); + } }); } + return () => { + mounted = false; + }; }, [collectionID]);components/mapping-tools/ConsolidationMappingTool.tsx (2)
101-145: Race condition: effect can trigger multiple times during processing.The dependency array
[file, processing]allows the effect to re-run iffilechanges whileprocessingis still true. This creates a race condition:
- User uploads file A → clicks submit →
processing=true→ effect runs- User uploads file B while processing →
filechanges → effect runs again- Two simultaneous operations: multiple API calls, FileReader instances, and
setCsvDatacallsThis results in wasted API calls and potentially inconsistent state, as the second effect (line 147) may run multiple times and trigger multiple downloads.
Consider one of these approaches:
Option 1: Prevent file changes during processing
onChange={(e: any) => { + if (processing) return; if (e.target.files) {Option 2: Add cleanup to cancel ongoing operations
useEffect(() => { if (!processing || !file) { return; } + let cancelled = false; async function fetchConsolidations(url: string, selectedFile: File) { fetchAllPages<Consolidation>(url).then((consolidations) => { + if (cancelled) return; setConsolidations(consolidations);reader.onload = async () => { + if (cancelled) return; const data = reader.result;const initialUrl = `${publicEnv.API_ENDPOINT}/api/consolidations`; fetchConsolidations(initialUrl, file); + return () => { cancelled = true; }; }, [file, processing]);Option 3: Remove
filefrom dependencies (if file should only be read once when processing starts)-}, [file, processing]); +}, [processing]);
106-141: Add error handling for API call and file reading operations.If
fetchAllPagesfails, the error is unhandled andprocessingremainstrueindefinitely, leaving the UI in an unusable state. Similarly, the FileReader has noonerrorhandler.Apply this diff to add error handling:
async function fetchConsolidations(url: string, selectedFile: File) { - fetchAllPages<Consolidation>(url).then((consolidations) => { + fetchAllPages<Consolidation>(url) + .then((consolidations) => { setConsolidations(consolidations); const reader = new FileReader(); reader.onload = async () => {reader.readAsText(selectedFile); - }); + }) + .catch((error) => { + console.error("Failed to fetch consolidations:", error); + setProcessing(false); + // TODO: Show error message to user + }); }Additionally, add a FileReader error handler:
reader.onload = async () => {(after line 137)
}; + reader.onerror = () => { + console.error("Failed to read file:", reader.error); + setProcessing(false); + // TODO: Show error message to user + }; + reader.readAsText(selectedFile);components/user/stats/activity/distributions/UserPageStatsActivityDistributions.tsx (1)
93-105: Add missing dependencies to the effect.The effect uses
pageFilter(line 101) andonPageFilter(lines 96, 102) but doesn't include them in the dependency array. This violates the exhaustive-deps rule and can lead to stale closure bugs.Apply this diff to fix the issue:
First, wrap
onPageFilterinuseCallback:- const onPageFilter = (page: number) => { + const onPageFilter = useCallback((page: number) => { router.replace( `${pathname}?${createQueryString([ { name: WALLET_DISTRIBUTION_PAGE_PARAM, value: `${page}` }, ])}`, { scroll: false } ); - }; + }, [pathname, router, searchParams]);Then add the missing dependencies to the effect:
useEffect(() => { if (isFetching) return; if (!data?.count) { onPageFilter(1); setTotalPages(1); return; } const totalPages = Math.ceil(data.count / PAGE_SIZE); if (totalPages < pageFilter) { onPageFilter(totalPages); } setTotalPages(totalPages); - }, [data?.count, data?.page, isFetching]); + }, [data?.count, data?.page, isFetching, pageFilter, onPageFilter]);components/user/identity/statements/consolidated-addresses/UserPageIdentityStatementsConsolidatedAddresses.tsx (1)
25-30: RefactorisMyProfilefor consistency with other derived state.The file refactors
canEdit,primaryAddress, andsortedByPrimaryfromuseState+useEffectto direct values oruseMemo, butisMyProfilestill uses the old pattern. For consistency, refactor to a direct derived value:Apply this diff:
- const [isMyProfile, setIsMyProfile] = useState<boolean>(true); - - useEffect( - () => setIsMyProfile(amIUser({ profile, address })), - [profile, address] - ); + const isMyProfile = amIUser({ profile, address });If
amIUseris computationally expensive, wrap it inuseMemo:- const [isMyProfile, setIsMyProfile] = useState<boolean>(true); - - useEffect( - () => setIsMyProfile(amIUser({ profile, address })), - [profile, address] - ); + const isMyProfile = useMemo( + () => amIUser({ profile, address }), + [profile, address] + );components/lfg-slideshow/LFGSlideshow.tsx (1)
111-149: GuardcurrentMediato avoid runtime error when opening before media loadsIn this effect,
const currentMedia = media[currentIndex];assumes thatmedia[currentIndex]exists wheneverisOpenistrue. If the user opens the slideshow beforecommonApiFetchresolves (somediais still[]),currentMediabecomesundefinedandcurrentMedia.animationwill throw, breaking the slideshow.You can avoid this by bailing out early when there is no current media:
- useEffect(() => { - if (!isOpen) return; + useEffect(() => { + if (!isOpen || media.length === 0 || !media[currentIndex]) return;This keeps the rest of the effect (preloading, video listeners, timeout handling) unchanged while preventing crashes when data hasn’t arrived yet or if
currentIndexis temporarily out of range.components/nextGen/collections/NextGenCollections.tsx (1)
32-45: ReplaceuseEffectEventwithuseCallbackor refactor to remove the wrapper.Since
useEffectEventdoesn't exist, consider these alternatives:Option 1: Use
useCallbackwith dependencies:- const fetchResults = useEffectEvent((mypage: number) => { + const fetchResults = useCallback((mypage: number) => { setCollectionsLoaded(false); let statusFilter = ""; if (selectedStatus !== StatusFilter.ALL) { statusFilter = `&status=${selectedStatus}`; } let url = `${publicEnv.API_ENDPOINT}/api/nextgen/collections?page_size=${PAGE_SIZE}&page=${mypage}${statusFilter}`; fetchUrl(url).then((response: DBResponse) => { setTotalResults(response.count); setCollections(response.data); setCollectionsLoaded(true); }); - }); + }, [selectedStatus]);Option 2: Define
fetchResultsinside each effect where it's used to avoid stale closures.
♻️ Duplicate comments (1)
components/nextGen/collections/NextGenCollections.tsx (1)
8-8: Critical:useEffectEventdoes not exist in React.Same issue as in other files -
useEffectEventis not a valid React export.
| const onViewModeChange = useCallback( | ||
| async (mode: GroupsViewMode): Promise<void> => { | ||
| if (mode === GroupsViewMode.CREATE) { | ||
| const { success } = await requestAuth(); | ||
| if (!success) return; | ||
| } else if (pathname) { | ||
| router.replace(pathname); | ||
| } | ||
|
|
||
| setViewMode(mode); | ||
| }, | ||
| [pathname, requestAuth, router], | ||
| ); |
There was a problem hiding this comment.
Add mounted guard to prevent state updates after unmount.
The PR summary indicates that "mounted guards and cancellation mechanisms are introduced for async operations," but this async callback lacks a mounted check before calling setViewMode(mode) on line 46. If the component unmounts while authentication is pending, this will trigger React warnings and potentially cause memory leaks.
Apply this diff to add a mounted guard:
+ const mountedRef = useRef(true);
+
+ useEffect(() => {
+ return () => {
+ mountedRef.current = false;
+ };
+ }, []);
+
const onViewModeChange = useCallback(
async (mode: GroupsViewMode): Promise<void> => {
if (mode === GroupsViewMode.CREATE) {
const { success } = await requestAuth();
if (!success) return;
} else if (pathname) {
router.replace(pathname);
}
+ if (!mountedRef.current) return;
setViewMode(mode);
},
[pathname, requestAuth, router],
);Don't forget to add useRef to the imports on line 3.
🤖 Prompt for AI Agents
In components/groups/page/Groups.tsx around lines 37 to 49, the async
onViewModeChange callback can call setViewMode after the component has
unmounted; add a mounted guard using useRef (add useRef to the imports on line
3), set mountedRef.current = true in a useEffect and flip to false on cleanup,
then check mountedRef.current before calling setViewMode(mode) (and also after
awaiting requestAuth) to avoid state updates after unmount; keep the rest of the
logic intact.
| const [sortDir, setSortDir] = useState<SortDirection>(() => { | ||
| const { initialSortDir } = getInitialRouterValues( | ||
| searchParams?.get("sortDir") ?? null, | ||
| searchParams?.get("sort") ?? null | ||
| ); | ||
| setSortDir(initialSortDir); | ||
| setSort(initialSort); | ||
| setVolumeType(VolumeType.HOURS_24); | ||
| }, []); | ||
| return initialSortDir; | ||
| }); | ||
| const [sort, setSort] = useState<MemeLabSort>(() => { | ||
| const { initialSort } = getInitialRouterValues( | ||
| searchParams?.get("sortDir") ?? null, | ||
| searchParams?.get("sort") ?? null | ||
| ); | ||
| return initialSort; | ||
| }); | ||
|
|
||
| const [volumeType, setVolumeType] = useState<VolumeType>(VolumeType.HOURS_24); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify URL change handling.
The lazy initializers correctly compute initial sort state from URL parameters. However, unlike MemeLab.tsx (line 457) which responds to searchParams changes via a useEffect dependency, this component only initializes once on mount. If the URL changes without remounting (e.g., browser back/forward), the sort state won't update.
Run the following script to check if this component is always remounted when URL parameters change, or if it should include a useEffect to respond to searchParams changes:
🏁 Script executed:
#!/bin/bash
# Check if MemeLabCollection is used in contexts where it needs to respond to URL changes
rg -nP -C5 'MemeLabCollection|LabCollection' --type=tsx --type=ts -g '!**/node_modules/**'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Find MemeLabCollection usage with corrected syntax
rg -nP -C5 'MemeLabCollection' -g '*.tsx' -g '*.ts' -g '!**/node_modules/**'Length of output: 8470
🏁 Script executed:
#!/bin/bash
# Also search for imports of MemeLabCollection to find all usages
rg -nP 'import.*MemeLabCollection|from.*MemeLabCollection' -g '*.tsx' -g '*.ts'Length of output: 472
🏁 Script executed:
#!/bin/bash
# Check the parent component structure around MemeLabCollection usage
rg -nP 'useRouter|useSearchParams|searchParams' components/memelab/MemeLabCollection.tsx -A2 -B2Length of output: 976
🏁 Script executed:
#!/bin/bash
# Check MemeLab.tsx around line 457 to see the useEffect pattern mentioned in review
rg -nP 'useEffect' components/memelab/MemeLab.tsx -B3 -A5 | head -100Length of output: 1545
🏁 Script executed:
#!/bin/bash
# Get the full context of MemeLab.tsx to understand routing/state management
cat components/memelab/MemeLab.tsx | head -500Length of output: 15361
Add useEffect to sync sort state when URL query parameters change.
MemeLabCollection initializes sort state from URL parameters only on mount via lazy initializers. Unlike MemeLab.tsx (which has useEffect(() => { ... }, [searchParams]) at line 450), this component won't update if the URL changes without remounting. When users navigate back/forward or manually edit query parameters, the sort state will remain stale. Add a useEffect with [searchParams] dependency to keep state synchronized with URL parameters.
🤖 Prompt for AI Agents
In components/memelab/MemeLabCollection.tsx around lines 52 to 67, the sort
state (sortDir and sort) is only initialized from URL query params via lazy
initializers and will not update if searchParams changes; add a useEffect that
runs when searchParams changes, calls getInitialRouterValues with the same
searchParams?.get(...) fallbacks, and updates setSortDir and setSort accordingly
so the component stays synchronized with URL query parameters (keep the same
null handling and types).
| const { | ||
| collection, | ||
| setAvailable: notifyAvailable, | ||
| shouldRefetchMintCounts = false, | ||
| setShouldRefetchMintCounts, | ||
| } = props; | ||
| const { id: collectionId, total_supply: totalSupply } = collection; | ||
| const [enableRefresh, setEnableRefresh] = useState<boolean>(true); | ||
| const [available, setAvailable] = useState<number>(0); | ||
|
|
||
| const collectionMintCount = useCollectionMintCount( | ||
| props.collection.id, | ||
| collectionId, | ||
| enableRefresh | ||
| ); | ||
| const { data: mintCountData, isFetching, refetch } = collectionMintCount; | ||
| const [mintCount, setMintCount] = useState<number>(0); |
There was a problem hiding this comment.
Guard against undefined mintCountData to avoid NaN UI state
NextGenMintCounts now parses mintCountData unconditionally:
const mintC = parseInt(mintCountData as any);
setMintCount(mintC);
const avail = totalSupply - mintC;On initial render mintCountData will typically be undefined, which leads to parseInt(undefined) → NaN, and propagates through to mintCount/available. That will briefly render “NaN / X minted” until the first contract read completes.
Recommend short‑circuiting when there’s no usable value yet:
useEffect(() => {
- const mintC = parseInt(mintCountData as any);
- setMintCount(mintC);
- const avail = totalSupply - mintC;
- setAvailable(avail);
- setEnableRefresh(avail > 0);
- if (notifyAvailable) {
- notifyAvailable(avail);
- }
- }, [mintCountData, totalSupply, notifyAvailable]);
+ if (mintCountData == null) {
+ return;
+ }
+ const mintC = parseInt(mintCountData as any);
+ if (Number.isNaN(mintC)) {
+ return;
+ }
+ setMintCount(mintC);
+ const avail = totalSupply - mintC;
+ setAvailable(avail);
+ setEnableRefresh(avail > 0);
+ if (notifyAvailable) {
+ notifyAvailable(avail);
+ }
+ }, [mintCountData, totalSupply, notifyAvailable]);This keeps the initial state at 0 / total_supply with the loader visible, and only flips to real values once the contract call has succeeded.
Also applies to: 351-375
🤖 Prompt for AI Agents
In components/nextGen/collections/collectionParts/NextGenCollectionHeader.tsx
around lines 332-347 (and similarly 351-375), guard against undefined or
non-numeric mintCountData before calling parseInt: only parse and set mintCount
when mintCountData is defined and yields a finite number (use a simple check
like typeof mintCountData !== 'undefined' &&
!Number.isNaN(Number(mintCountData))). If the check fails, leave mintCount at
the initial 0 (and available = totalSupply) and keep the loader/ isFetching
state visible; when valid, setMintCount(parsedValue) and set available =
totalSupply - parsedValue (and call notifyAvailable if required). Ensure no NaN
is written to state by validating before updating mintCount/available.
|
|
||
| import styles from "./Prenodes.module.scss"; | ||
| import { useEffect, useState } from "react"; | ||
| import { useEffect, useEffectEvent, useState } from "react"; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that useEffectEvent is available in your React version.
useEffectEvent is an experimental React API that may not be available in stable React releases. This could cause runtime errors if the API is not supported in your project's React version.
Please verify the availability by searching for the official React documentation:
🌐 Web query:
Is useEffectEvent available in stable React releases as of November 2025?
💡 Result:
Yes. React added useEffectEvent to the stable release in React 19.2 (released October 1, 2025). [1][2]
Sources:
- React 19.2 release notes (Oct 1, 2025). [1]
- useEffectEvent API reference (React v19.2). [2]
🏁 Script executed:
#!/bin/bash
# Check React version and examine PrenodesStatus.tsx
find . -name "package.json" -type f | head -5 | xargs grep -l '"react"' 2>/dev/null | head -1 | xargs cat | grep -A2 '"react"'Length of output: 223
🏁 Script executed:
#!/bin/bash
# Examine the PrenodesStatus.tsx file, focusing on lines 45-57
find . -name "PrenodesStatus.tsx" -type f | head -1 | xargs sed -n '40,65p'Length of output: 780
🏁 Script executed:
#!/bin/bash
# Quick check of commonly used build config for React version
echo "Checking for React version clues in standard locations..."
ls -la package.json package-lock.json yarn.lock 2>/dev/null || echo "Standard config files not found"Length of output: 222
🏁 Script executed:
#!/bin/bash
# Look for import statement and usage pattern
find . -name "PrenodesStatus.tsx" -type f | head -1 | xargs head -70Length of output: 1906
🏁 Script executed:
#!/bin/bash
# Check if error handling exists in fetchResults or useEffect
find . -name "PrenodesStatus.tsx" -type f | head -1 | xargs cat | grep -n "try\|catch\|error\|Error" | head -20Length of output: 326
🏁 Script executed:
#!/bin/bash
# Look for AbortController or race condition prevention
find . -name "PrenodesStatus.tsx" -type f | head -1 | xargs cat | grep -n "AbortController\|cancel\|Race\|race"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Count lines in file to understand context
find . -name "PrenodesStatus.tsx" -type f | head -1 | xargs wc -lLength of output: 44
Remove fetchResults from the useEffect dependency array; it should only include [page].
useEffectEvent-created functions like fetchResults should not be included in dependency arrays. Per React 19.2 documentation, Effect Events are designed to be stable references and must be omitted from dependencies. Update line 57 from [fetchResults, page] to [page].
Additionally, add error handling to the fetch call—the current implementation lacks a try-catch block, so network failures will crash the component. Consider implementing race condition prevention (e.g., AbortController) for rapid pagination changes.
🤖 Prompt for AI Agents
In components/prenodes/PrenodesStatus.tsx around line 57, update the useEffect
dependency array to only include [page] instead of [fetchResults, page]; remove
fetchResults from dependencies because it is an Effect Event and should not be
listed. Inside the effect, wrap the fetch logic in try-catch so network errors
are caught and logged/handled instead of crashing the component. Also implement
an AbortController (signal passed to fetch and aborted in the cleanup) or
similar cancellation to prevent race conditions when page changes rapidly, and
ensure you check response.ok before parsing.
| const fetchResults = useEffectEvent(async (pageToLoad: number) => { | ||
| const response = await fetch( | ||
| `https://api.6529.io/oracle/prenodes?page=${pageToLoad}&page_size=${PAGE_SIZE}` | ||
| ); | ||
| const { data, count }: { data: Prenode[]; count: number } = | ||
| await response.json(); | ||
| setPrenodes(data); | ||
| setTotalResults(count); | ||
| }); |
There was a problem hiding this comment.
Add error handling and race condition protection.
The async fetch operation is missing:
- Error handling: No try-catch block to handle network failures or invalid responses
- Race condition protection: Rapid page changes can cause stale data to be set if an earlier request completes after a later one
Consider this refactor with error handling and cancellation:
+const [isLoading, setIsLoading] = useState(false);
+const [error, setError] = useState<string | null>(null);
+
const fetchResults = useEffectEvent(async (pageToLoad: number) => {
+ setIsLoading(true);
+ setError(null);
+ try {
const response = await fetch(
`https://api.6529.io/oracle/prenodes?page=${pageToLoad}&page_size=${PAGE_SIZE}`
);
+ if (!response.ok) {
+ throw new Error(`Failed to fetch prenodes: ${response.status}`);
+ }
const { data, count }: { data: Prenode[]; count: number } =
await response.json();
setPrenodes(data);
setTotalResults(count);
+ } catch (err) {
+ setError(err instanceof Error ? err.message : 'Failed to fetch prenodes');
+ setPrenodes([]);
+ setTotalResults(0);
+ } finally {
+ setIsLoading(false);
+ }
});For race condition handling, consider using an AbortController in the useEffect:
useEffect(() => {
const controller = new AbortController();
fetchResults(page, controller.signal);
return () => controller.abort();
}, [page]);| useEffect(() => { | ||
| if (connectedWallets.length > 0 && nftId) { | ||
| commonApiFetch<ConsolidatedTDH>({ | ||
| endpoint: `tdh/consolidation/${connectedProfile?.consolidation_key}`, | ||
| endpoint: `tdh/consolidation/${consolidationKey}`, | ||
| }).then((response) => { | ||
| setMyOwner(response); | ||
| setMyTDH(response.memes.find((m) => m.id === parseInt(nftId))); | ||
| setMyRank(response.memes_ranks.find((m) => m.id === parseInt(nftId))); | ||
| }); | ||
| } | ||
| }, [nftId, connectedWallets]); | ||
| }, [nftId, connectedWallets, consolidationKey]); | ||
|
|
There was a problem hiding this comment.
Guard consolidation fetch on consolidationKey
The consolidation fetch now uses:
if (connectedWallets.length > 0 && nftId) {
commonApiFetch<ConsolidatedTDH>({
endpoint: `tdh/consolidation/${consolidationKey}`,
}).then(/* ... */);
}If a user has connected wallets but connectedProfile.consolidation_key is missing, this will call tdh/consolidation/undefined, which is likely invalid.
Consider guarding on consolidationKey and optionally resetting local TDH state when it’s absent:
useEffect(() => {
- if (connectedWallets.length > 0 && nftId) {
+ if (connectedWallets.length > 0 && nftId && consolidationKey) {
commonApiFetch<ConsolidatedTDH>({
endpoint: `tdh/consolidation/${consolidationKey}`,
}).then((response) => {
setMyOwner(response);
setMyTDH(response.memes.find((m) => m.id === parseInt(nftId)));
setMyRank(response.memes_ranks.find((m) => m.id === parseInt(nftId)));
});
+ } else {
+ setMyOwner(undefined);
+ setMyTDH(undefined);
+ setMyRank(undefined);
}
- }, [nftId, connectedWallets, consolidationKey]);
+ }, [nftId, connectedWallets, consolidationKey]);This avoids hitting an invalid endpoint and keeps local state consistent when consolidation isn’t configured.
🤖 Prompt for AI Agents
In components/the-memes/MemePage.tsx around lines 237 to 248, the effect calls
commonApiFetch with consolidationKey even when it may be undefined, causing a
request to tdh/consolidation/undefined; update the guard to require
consolidationKey (e.g., if (connectedWallets.length > 0 && nftId &&
consolidationKey)) before fetching, and when consolidationKey is absent reset
local TDH-related state (setMyOwner(null), setMyTDH(null), setMyRank(null) or
appropriate defaults) so no invalid endpoint is requested and local state stays
consistent.
| const isChangedAndValid = useMemo(() => { | ||
| if (profileProxyAction.end_time === endTime) { | ||
| return false; | ||
| } | ||
| if (endTime && endTime < Time.currentMillis()) { | ||
| return false; | ||
| } | ||
| return true; | ||
| }; | ||
|
|
||
| const [isChangedAndValid, setIsChangedAndValid] = useState( | ||
| getIsChangedAndValid() | ||
| ); | ||
| useEffect( | ||
| () => setIsChangedAndValid(getIsChangedAndValid()), | ||
| [endTime, profileProxyAction.end_time] | ||
| ); | ||
| }, [endTime, profileProxyAction.end_time]); |
There was a problem hiding this comment.
Time-based validation may become stale.
The useMemo correctly depends on endTime and profileProxyAction.end_time, but the validation at line 41 calls Time.currentMillis() without including it in the dependency array. If a user selects a valid future time and then waits on the page, the validation won't update if that time becomes past—the UI will still show the selection as valid until the user changes the input.
This is likely acceptable since server-side validation should catch invalid submissions, but it's worth noting for UX consistency.
🤖 Prompt for AI Agents
In components/user/proxy/proxy/action/utils/time/ProfileProxyEndTimeEdit.tsx
around lines 37-45, the useMemo uses Time.currentMillis() inside its logic but
doesn’t include a time reference in the dependency array, so the validation
becomes stale as the wall-clock advances; fix by introducing a stable "now"
value that updates (e.g., useState + useEffect with a short interval or timeout)
or by removing the memoization and computing validity each render, then include
that "now" value in the useMemo dependency array (or rely on no memo) and ensure
any interval is cleaned up in the effect.
| profile.wallets?.map((wallet) => wallet.wallet.toLowerCase()), | ||
| ].join(","), | ||
| [activeAddress, profile] | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Refine the dependency array to avoid unnecessary recomputations.
Depending on the entire profile object causes this memo to recompute whenever the profile reference changes, even if profile.wallets remains the same. Depend on profile.wallets instead.
Apply this diff:
- [activeAddress, profile]
+ [activeAddress, profile.wallets]Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
components/user/stats/activity/distributions/UserPageStatsActivityDistributions.tsx
around line 66, the memo hook depends on the entire `profile` object causing
unnecessary recomputations; change the dependency to `profile.wallets` (use
optional chaining if needed, e.g., `profile?.wallets`) so the memo only re-runs
when wallets change, and ensure imports/types still satisfy the new dependency
usage.
| useEffect(() => { | ||
| setHaveChanges(classification !== profile.classification); | ||
| }, [classification]); | ||
| const haveChanges = classification !== profile.classification; |
There was a problem hiding this comment.
Inconsistent default value handling causes incorrect change detection.
When profile.classification is null or undefined, the comparison yields true even though the user hasn't made any changes. This is because classification initializes to ApiProfileClassification.Pseudonym (line 31) while the comparison checks against the raw profile.classification value. As a result, the save button is incorrectly enabled on mount, allowing unintended profile updates.
Apply this diff to align the comparison with the initialization logic:
- const haveChanges = classification !== profile.classification;
+ const haveChanges = classification !== (profile.classification ?? ApiProfileClassification.Pseudonym);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const haveChanges = classification !== profile.classification; | |
| const haveChanges = classification !== (profile.classification ?? ApiProfileClassification.Pseudonym); |
🤖 Prompt for AI Agents
In
components/user/user-page-header/name/classification/UserPageHeaderEditClassification.tsx
around line 34, the change-detection compares the local classification state to
raw profile.classification which can be null/undefined while local state
defaults to ApiProfileClassification.Pseudonym; update the comparison to
normalize profile.classification to the same default (e.g. use
profile.classification ?? ApiProfileClassification.Pseudonym) so haveChanges
only becomes true when the normalized values actually differ.
| const [animateContentMarginLeft, setAnimateContentMarginLeft] = | ||
| useState(false); | ||
| const [init, setInit] = useState(false); | ||
|
|
||
| useEffect( | ||
| () => setAnimateContentMarginLeft(getAnimateContentMarginLeft()), | ||
| () => { | ||
| const shouldAnimate = !["XXL", "S"].includes(breakpoint) && open; | ||
| setAnimateContentMarginLeft(shouldAnimate); | ||
| }, | ||
| [breakpoint, open] | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Complete the refactor: Replace derived state with useMemo.
The current implementation still uses the useState + useEffect pattern for derived state, which the PR objectives aim to eliminate. Since animateContentMarginLeft is purely computed from breakpoint and open, it should be memoized rather than stored in state.
Apply this diff to complete the refactor:
- const [animateContentMarginLeft, setAnimateContentMarginLeft] =
- useState(false);
+ const animateContentMarginLeft = useMemo(
+ () => !["XXL", "S"].includes(breakpoint) && open,
+ [breakpoint, open]
+ );
const [init, setInit] = useState(false);
- useEffect(
- () => {
- const shouldAnimate = !["XXL", "S"].includes(breakpoint) && open;
- setAnimateContentMarginLeft(shouldAnimate);
- },
- [breakpoint, open]
- );
-
useEffect(() => {Note: You'll need to import useMemo from React at the top of the file.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/utils/sidebar/SidebarLayout.tsx around lines 43 to 53, the derived
boolean animateContentMarginLeft is implemented with useState + useEffect but
should be computed from props instead; replace the useState and effect with a
useMemo that returns !["XXL","S"].includes(breakpoint) && open, remove the
corresponding setAnimateContentMarginLeft usages, and import useMemo from React
at the top of the file so the value is memoized and not stored in component
state.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Refactor