Skip to content

Refactor supporters fetching#7

Open
sktbrd wants to merge 7 commits intomainfrom
077zb4-codex/study-supporters-component-and-api-usage
Open

Refactor supporters fetching#7
sktbrd wants to merge 7 commits intomainfrom
077zb4-codex/study-supporters-component-and-api-usage

Conversation

@sktbrd
Copy link
Owner

@sktbrd sktbrd commented Jul 31, 2025

Summary

  • improve /api/supporters with viem multicall and caching
  • simplify supporters hook
  • simplify supporters section display

Testing

  • pnpm lint (fails: How would you like to configure ESLint?)

https://chatgpt.com/codex/tasks/task_e_688b71acff14832cadaa53599a240a0e

Summary by CodeRabbit

  • Refactor
    • Simplified the supporters API endpoint for improved performance and efficiency.
    • Streamlined the SupportersSection component with a cleaner UI and reduced complexity by removing error and cached state displays.
    • Overhauled the useSupporters hook to remove error handling, caching, and complex pagination, resulting in a more straightforward data-fetching experience.
    • Ensured a default application URL is set when environment variable is missing.

@vercel
Copy link

vercel bot commented Jul 31, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
termignar ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2025 7:53pm

@coderabbitai
Copy link

coderabbitai bot commented Jul 31, 2025

Walkthrough

This update refactors the supporters feature across the API route, React component, and custom hook. The API endpoint is streamlined for efficiency, using multicall for batch owner queries and simplified caching. The React component and hook are both simplified, removing complex error handling and pagination, and introducing incremental loading triggered explicitly on mount or user action.

Changes

Cohort / File(s) Change Summary
API Refactor
src/app/api/supporters/route.ts
Refactored the supporters API endpoint for more concise logic: replaced per-entry TTL cache with a simpler timestamp check, switched to multicall for batch owner queries, and simplified aggregation. Removed redundant error handling and formatting. Response no longer includes a cached flag. Exported and internal functions updated, added, or removed accordingly.
Component Simplification
src/components/droposal/SupportersSection.tsx
Simplified the SupportersSection UI and logic: removed the SupporterAvatar wrapper, error display, and cached label. Changed to inline React.FC prop typing. Removed autoLoad from hook usage and instead exposes loadMore without explicit call in this snippet. Supporters grid rendering simplified with raw address keys and consistent styling.
Hook Simplification
src/hooks/useSupporters.ts
Major simplification of the useSupporters hook: removed error, caching, visibility, and pagination logic, eliminated complex state management, and now only tracks loading, hasMore, and supporters. The hook appends fetched supporters and exposes a minimal API. Removed related interfaces and helpers, renamed and simplified API response types. Initial fetch triggered automatically on dependency changes.
Environment Variable Default
src/app/opengraph-image.tsx
Added a fallback default URL for appUrl when NEXT_PUBLIC_URL environment variable is undefined or empty, ensuring a valid URL string is always available.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant SupportersSection (React)
    participant useSupporters (Hook)
    participant API /supporters
    participant NFT Contract

    User->>SupportersSection (React): Loads page
    SupportersSection->>useSupporters: useSupporters({contractAddress, totalSupply})
    SupportersSection->>useSupporters: useEffect calls loadMore()
    useSupporters->>API /supporters: GET /api/supporters?contract=...&start=...&end=...
    API /supporters->>NFT Contract: multicall ownerOf(tokenId) for range
    NFT Contract-->>API /supporters: owners for token IDs
    API /supporters-->>useSupporters: Aggregated supporters data
    useSupporters-->>SupportersSection: { visibleSupporters, loadMore, loading, hasMore }
    User->>SupportersSection: Clicks "Load More"
    SupportersSection->>useSupporters: loadMore()
    useSupporters->>API /supporters: GET next page
    API /supporters->>NFT Contract: multicall for next range
    NFT Contract-->>API /supporters: owners for next token IDs
    API /supporters-->>useSupporters: More supporters data
    useSupporters-->>SupportersSection: Updated supporters
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~18 minutes

Poem

🐇✨
Refactored code, a lighter load,
Supporters now in simpler mode.
Multicall hops, the cache is neat,
UI and hook both trim and sweet.
With "Load More" joy, the list extends—
A rabbit’s work that never ends!
🥕

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 077zb4-codex/study-supporters-component-and-api-usage

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Nitpick comments (3)
src/components/droposal/SupportersSection.tsx (2)

28-28: Consider truncating addresses for better readability.

Displaying full Ethereum addresses can hurt readability on smaller screens. Consider using a truncated format (e.g., 0x1234...5678).

-              <Text>{holder.address}</Text>
+              <Text>{`${holder.address.slice(0, 6)}...${holder.address.slice(-4)}`}</Text>

6-15: The totalSupply prop is not being used within the component.

The totalSupply is passed to the hook but not used for any display or logic in the component itself. If it's only needed by the hook, consider removing it from the component's props interface.

src/app/api/supporters/route.ts (1)

46-49: Optimize multicall batch construction.

The current loop creates a new array on each iteration. Consider using Array.from for better performance.

-  const contracts = [] as { address: Address; abi: any; functionName: string; args: [bigint] }[];
-  for (let id = startId; id <= endId; id++) {
-    contracts.push({ address, abi: zoraMintAbi, functionName: 'ownerOf', args: [id] });
-  }
+  const count = Number(endId - startId + 1n);
+  const contracts = Array.from({ length: count }, (_, i) => ({
+    address,
+    abi: zoraMintAbi,
+    functionName: 'ownerOf',
+    args: [startId + BigInt(i)]
+  }));
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 03a58f8 and 9363519.

📒 Files selected for processing (3)
  • src/app/api/supporters/route.ts (1 hunks)
  • src/components/droposal/SupportersSection.tsx (1 hunks)
  • src/hooks/useSupporters.ts (2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/app/api/supporters/route.ts (3)
src/utils/client.ts (1)
  • publicClient (4-7)
src/hooks/useSupporters.ts (1)
  • AggregatedHolder (4-8)
src/utils/abis/zoraNftAbi.ts (1)
  • zoraMintAbi (378-710)

Comment on lines 9 to 12
const publicClient = createPublicClient({
chain: base,
transport: http(),
});
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use the existing publicClient from utils.

You're redefining publicClient here when it's already available in src/utils/client.ts.

-const publicClient = createPublicClient({
-  chain: base,
-  transport: http(),
-});
+import { publicClient } from '@/utils/client';
🤖 Prompt for AI Agents
In src/app/api/supporters/route.ts around lines 9 to 12, you are redefining the
publicClient instead of using the existing one from src/utils/client.ts. Remove
the local declaration and import the publicClient from src/utils/client.ts to
reuse the existing instance.

Comment on lines +33 to +35
// simple in-memory cache
const cache = new Map<string, { data: SupportersResponse; time: number }>();
const TTL = 5 * 60 * 1000; // 5 minutes
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add cache size limit to prevent memory leaks.

The cache can grow indefinitely, potentially causing memory issues.

 // simple in-memory cache
 const cache = new Map<string, { data: SupportersResponse; time: number }>();
 const TTL = 5 * 60 * 1000; // 5 minutes
+const MAX_CACHE_SIZE = 100;
+
+function addToCache(key: string, data: SupportersResponse) {
+  // Remove oldest entries if cache is full
+  if (cache.size >= MAX_CACHE_SIZE) {
+    const oldestKey = Array.from(cache.entries())
+      .sort(([, a], [, b]) => a.time - b.time)[0][0];
+    cache.delete(oldestKey);
+  }
+  cache.set(key, { data, time: Date.now() });
+}

Then update line 107:

-  cache.set(key, { data: response, time: Date.now() });
+  addToCache(key, response);
🤖 Prompt for AI Agents
In src/app/api/supporters/route.ts around lines 33 to 35, the in-memory cache
currently has no size limit, which can lead to unbounded memory growth.
Implement a maximum size for the cache Map and add logic to evict the oldest or
least recently used entries when the limit is exceeded. Then update the cache
usage at line 107 to respect this size limit and eviction policy.

Comment on lines +76 to +78
const start = BigInt(searchParams.get('startTokenId') ?? '1');
const end = BigInt(searchParams.get('endTokenId') ?? (start + 19n));
const limit = parseInt(searchParams.get('limit') ?? '0');
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add validation for token ID range to prevent DoS attacks.

Large token ranges could cause excessive memory usage and slow response times.

  const start = BigInt(searchParams.get('startTokenId') ?? '1');
  const end = BigInt(searchParams.get('endTokenId') ?? (start + 19n));
+  
+  // Limit range to prevent DoS
+  const MAX_RANGE = 1000n;
+  if (end - start + 1n > MAX_RANGE) {
+    return NextResponse.json({ error: 'Range too large, maximum 1000 tokens' }, { status: 400 });
+  }
+  
  const limit = parseInt(searchParams.get('limit') ?? '0');
🤖 Prompt for AI Agents
In src/app/api/supporters/route.ts around lines 76 to 78, add validation to
ensure the token ID range between start and end is within a reasonable limit to
prevent excessive memory usage and slow responses. Implement checks to reject
requests where the range (end - start) exceeds a predefined maximum threshold,
returning an error or limiting the range accordingly to mitigate potential DoS
attacks.

Comment on lines +88 to +92
const totalSupply: bigint = await publicClient.readContract({
address: contract,
abi: zoraMintAbi,
functionName: 'totalSupply',
});
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for contract calls.

Contract calls can fail for various reasons (invalid contract, network issues, etc.).

-  const totalSupply: bigint = await publicClient.readContract({
-    address: contract,
-    abi: zoraMintAbi,
-    functionName: 'totalSupply',
-  });
+  let totalSupply: bigint;
+  try {
+    totalSupply = await publicClient.readContract({
+      address: contract,
+      abi: zoraMintAbi,
+      functionName: 'totalSupply',
+    });
+  } catch (error) {
+    return NextResponse.json({ error: 'Failed to fetch total supply' }, { status: 500 });
+  }
📝 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.

Suggested change
const totalSupply: bigint = await publicClient.readContract({
address: contract,
abi: zoraMintAbi,
functionName: 'totalSupply',
});
// Fetch total supply, with error handling
let totalSupply: bigint;
try {
totalSupply = await publicClient.readContract({
address: contract,
abi: zoraMintAbi,
functionName: 'totalSupply',
});
} catch (error) {
return NextResponse.json(
{ error: 'Failed to fetch total supply' },
{ status: 500 }
);
}
🤖 Prompt for AI Agents
In src/app/api/supporters/route.ts around lines 88 to 92, the contract call to
read totalSupply lacks error handling. Wrap the await publicClient.readContract
call in a try-catch block to catch any errors from invalid contracts or network
issues, and handle or log the error appropriately to prevent unhandled promise
rejections.

Comment on lines 10 to 41
const { visibleSupporters, loadMore, loading, hasMore } = useSupporters({
contractAddress,
totalSupply,
batchSize: fetchSize,
itemsPerPage: fetchSize,
autoLoad: true,
batchSize: 40,
itemsPerPage: 8,
});

return (
<Box
borderWidth={1}
display='flex'
flexDir='column'
alignItems='stretch'
gap={3}
rounded='lg'
p={6}
_dark={{ borderColor: 'yellow' }}
>
<HStack gap={2}>
<FaUsers size={24} color='#FFD700' />
<Heading size='xl'>
Community Supporters
{cached && (
<Text as='span' fontSize='sm' color='gray.500' ml={2}>
(cached)
</Text>
)}
{totalSupply && totalSupply > maxFetchSize && (
<Text as='span' fontSize='sm' color='blue.500' ml={2}>
(showing first {maxFetchSize})
</Text>
)}
</Heading>
</HStack>
<Box borderWidth={1} rounded="lg" p={6}>
<Heading size="lg" mb={4}>Community Supporters</Heading>

{loading && supporters.length === 0 ? (
<Flex justify='center' my={4}>
<Spinner size='sm' mr={2} />
<Text>Loading supporters...</Text>
</Flex>
) : supporters.length > 0 ? (
<>
{/* Grid of supporter cards */}
<Box pb={2}>
<SimpleGrid columns={{ base: 2, sm: 3, md: 4 }} gap={3} w='100%'>
{supporters.map((holder, index) => (
<Box
key={`${holder.address}`}
bg='gray.50'
p={3}
borderRadius='lg'
textAlign='center'
border='1px solid'
borderColor='gray.200'
transition='all 0.2s'
_hover={{
transform: 'translateY(-2px)',
boxShadow: 'sm',
}}
_dark={{
bg: 'gray.800',
borderColor: 'yellow.600',
}}
>
<VStack gap={2}>
<Box position='relative'>
<SupporterAvatar address={holder.address as Address} />
{holder.tokenCount > 1 && (
<Flex
position='absolute'
bottom='-2px'
right='-2px'
bg='blue.500'
color='white'
borderRadius='full'
w='20px'
h='20px'
justifyContent='center'
alignItems='center'
fontSize='xs'
fontWeight='bold'
>
{holder.tokenCount}
</Flex>
)}
</Box>
<FormattedAddress address={holder.address} />
<Text fontSize='xs' color='gray.500'>
#{holder.tokenIds[0].toString()}
</Text>
</VStack>
</Box>
))}
</SimpleGrid>
</Box>
</>
{visibleSupporters.length === 0 && loading ? (
<Spinner />
) : (
<Text>No supporters found</Text>
<SimpleGrid columns={{ base: 2, md: 4 }} gap={4}>
{visibleSupporters.map(holder => (
<VStack key={holder.address}>
<OptimizedAvatar address={holder.address as Address} size="md" />
<Text>{holder.address}</Text>
{holder.tokenCount > 1 && (
<Badge colorScheme="blue">{holder.tokenCount}</Badge>
)}
</VStack>
))}
</SimpleGrid>
)}

{error && (
<Text color='red.500' mt={2} fontSize='sm'>
{error}
</Text>
{hasMore && (
<Button onClick={loadMore} mt={4} isLoading={loading}>
Load More
</Button>
)}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding error handling for failed API calls.

The current implementation doesn't handle errors that might occur during data fetching. Users won't know if the loading failed.

Would you like me to generate an implementation that includes proper error handling and user feedback?

🤖 Prompt for AI Agents
In src/components/droposal/SupportersSection.tsx around lines 10 to 41, the
current code lacks error handling for the API calls made by useSupporters, so
users won't be informed if data fetching fails. Modify the useSupporters hook or
component state to track errors, then display an error message or alert in the
UI when an error occurs, ensuring users receive clear feedback about loading
failures.

Comment on lines 40 to +44
useEffect(() => {
if (!autoLoad) {
return;
}

if (!contractAddress) {
return;
}

if (totalSupply === null || totalSupply === undefined) {
return;
}

if (loadedContracts.current.has(contractAddress)) {
return;
}

loadedContracts.current.add(contractAddress);

const fetchInitialSupporters = async () => {
try {
setLoading(true);
setError(null);

const initialEndTokenId = totalSupply > 0n
? totalSupply < BigInt(batchSize) ? totalSupply : BigInt(batchSize)
: BigInt(batchSize);

const params = new URLSearchParams({
contractAddress,
startTokenId: '1',
endTokenId: initialEndTokenId.toString()
});

const response = await fetch(`/api/supporters?${params}`);

if (!response.ok) {
throw new Error(`Failed to fetch supporters: ${response.statusText}`);
}

const apiData: SupportersApiResponse = await response.json();

// Convert string tokenIds back to bigint
const supportersData = apiData.supporters.map(supporter => ({
...supporter,
tokenIds: supporter.tokenIds.map(id => BigInt(id))
}));

setSupporters(supportersData);
setVisibleSupporters(supportersData.slice(0, itemsPerPage));
setVisibleCount(itemsPerPage);
setHasMore(apiData.hasMore);
setNextTokenId(BigInt(apiData.nextTokenId));
setActualTotalSupply(BigInt(apiData.totalSupply));
setCached(apiData.cached);

} catch (err) {
const errorMessage = err instanceof Error ? err.message : 'Failed to fetch supporters';
setError(errorMessage);
console.error('Error fetching supporters:', err);
// Remove from loaded contracts so it can be retried
loadedContracts.current.delete(contractAddress);
} finally {
setLoading(false);
}
};

fetchInitialSupporters();
}, [contractAddress, totalSupply, autoLoad, batchSize, itemsPerPage]); // Add back necessary dependencies

// Load more supporters (fetch next batch from API)
const loadMore = useCallback(async () => {
if (!hasMore || loadingMore) return;

try {
setLoadingMore(true);

const endTokenId = nextTokenId + BigInt(batchSize) - 1n;
const params = new URLSearchParams({
contractAddress,
startTokenId: nextTokenId.toString(),
endTokenId: endTokenId.toString()
});

const response = await fetch(`/api/supporters?${params}`);

if (!response.ok) {
throw new Error(`Failed to fetch supporters: ${response.statusText}`);
}

const apiData: SupportersApiResponse = await response.json();

// Convert string tokenIds back to bigint
const newSupportersData = apiData.supporters.map(supporter => ({
...supporter,
tokenIds: supporter.tokenIds.map(id => BigInt(id))
}));

// Merge with existing supporters and re-aggregate
setSupporters(currentSupporters => {
const allTokenData = [
...currentSupporters.flatMap(s => s.tokenIds.map(tokenId => ({
address: s.address,
tokenId
}))),
...newSupportersData.flatMap(s => s.tokenIds.map(tokenId => ({
address: s.address,
tokenId
})))
];

// Re-aggregate
const holdersMap = new Map<string, { count: number; tokens: bigint[] }>();

allTokenData.forEach(({ address, tokenId }) => {
if (holdersMap.has(address)) {
const holder = holdersMap.get(address)!;
holder.count += 1;
holder.tokens.push(tokenId);
} else {
holdersMap.set(address, { count: 1, tokens: [tokenId] });
}
});

const aggregated = Array.from(holdersMap.entries())
.map(([address, data]) => ({
address,
tokenCount: data.count,
tokenIds: data.tokens,
}))
.sort((a, b) => b.tokenCount - a.tokenCount);

// Update visible supporters with current visible count
setVisibleSupporters(aggregated.slice(0, visibleCount));

return aggregated;
});

setHasMore(apiData.hasMore);
setNextTokenId(BigInt(apiData.nextTokenId));
setActualTotalSupply(BigInt(apiData.totalSupply));
setCached(apiData.cached);

} catch (err) {
const errorMessage = err instanceof Error ? err.message : 'Failed to load more supporters';
setError(errorMessage);
console.error('Error loading more supporters:', err);
} finally {
setLoadingMore(false);
}
}, [hasMore, loadingMore, nextTokenId, batchSize, contractAddress, visibleCount]);

// Show more supporters (from already fetched data)
const showMore = useCallback(async () => {
const newVisibleCount = visibleCount + itemsPerPage;
setVisibleCount(newVisibleCount);
setVisibleSupporters(supporters.slice(0, newVisibleCount));
}, [supporters, visibleCount, itemsPerPage]);

// Combined load more function that handles both cases
const handleLoadMore = useCallback(async () => {
const canShowMoreFromExisting = supporters.length > visibleCount;
if (canShowMoreFromExisting) {
await showMore();
} else {
await loadMore();
}
}, [supporters.length, visibleCount, showMore, loadMore]);

// Determine if we can show more from existing data or need to load more
const canShowMoreFromExisting = supporters.length > visibleCount;
const finalHasMore = canShowMoreFromExisting || hasMore;

return {
supporters,
visibleSupporters,
loading,
loadingMore,
error,
hasMore: finalHasMore,
loadMore: handleLoadMore,
showMore,
totalSupply: actualTotalSupply,
cached
};
if (!contractAddress || totalSupply === null) return;
loadMore();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [contractAddress, totalSupply]);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential issue with automatic loading in useEffect.

The effect calls loadMore() which depends on hasMore and loading states, but these aren't in the dependency array. This could lead to stale closures.

Consider either:

  1. Adding all dependencies to the array
  2. Moving the initial load logic to a separate function that doesn't depend on state
  3. Using a ref to track if initial load has occurred
🤖 Prompt for AI Agents
In src/hooks/useSupporters.ts around lines 40 to 44, the useEffect calls
loadMore() but does not include hasMore and loading states in its dependency
array, risking stale closures. To fix this, either add hasMore and loading to
the dependency array, or refactor the initial load logic into a separate
function that does not rely on these states, or use a ref to track if the
initial load has already occurred to prevent repeated calls.

Comment on lines +46 to +69
async function loadMore() {
if (!hasMore || loading) return;
setLoading(true);

const end = nextToken + BigInt(batchSize) - 1n;
const params = new URLSearchParams({
contractAddress,
startTokenId: nextToken.toString(),
endTokenId: end.toString(),
});

const res = await fetch(`/api/supporters?${params}`);
const data: SupportersResponse = await res.json();
const deserialized = data.supporters.map(h => ({
...h,
tokenIds: h.tokenIds.map(id => BigInt(id)),
}));

setSupporters(prev => [...prev, ...deserialized]);
setVisibleSupporters(prev => [...prev, ...deserialized].slice(0, itemsPerPage));
setHasMore(data.hasMore);
setNextToken(BigInt(data.nextTokenId));
setLoading(false);
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling and request cleanup.

The function lacks error handling for failed API calls and doesn't clean up pending requests when the component unmounts.

  async function loadMore() {
    if (!hasMore || loading) return;
    setLoading(true);

    const end = nextToken + BigInt(batchSize) - 1n;
    const params = new URLSearchParams({
      contractAddress,
      startTokenId: nextToken.toString(),
      endTokenId: end.toString(),
    });

-    const res = await fetch(`/api/supporters?${params}`);
-    const data: SupportersResponse = await res.json();
+    try {
+      const res = await fetch(`/api/supporters?${params}`);
+      if (!res.ok) throw new Error('Failed to fetch supporters');
+      const data: SupportersResponse = await res.json();
      const deserialized = data.supporters.map(h => ({
        ...h,
        tokenIds: h.tokenIds.map(id => BigInt(id)),
      }));

      setSupporters(prev => [...prev, ...deserialized]);
      setVisibleSupporters(prev => [...prev, ...deserialized].slice(0, itemsPerPage));
      setHasMore(data.hasMore);
      setNextToken(BigInt(data.nextTokenId));
+    } catch (error) {
+      console.error('Error loading supporters:', error);
+      setHasMore(false);
+    } finally {
      setLoading(false);
+    }
  }
📝 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.

Suggested change
async function loadMore() {
if (!hasMore || loading) return;
setLoading(true);
const end = nextToken + BigInt(batchSize) - 1n;
const params = new URLSearchParams({
contractAddress,
startTokenId: nextToken.toString(),
endTokenId: end.toString(),
});
const res = await fetch(`/api/supporters?${params}`);
const data: SupportersResponse = await res.json();
const deserialized = data.supporters.map(h => ({
...h,
tokenIds: h.tokenIds.map(id => BigInt(id)),
}));
setSupporters(prev => [...prev, ...deserialized]);
setVisibleSupporters(prev => [...prev, ...deserialized].slice(0, itemsPerPage));
setHasMore(data.hasMore);
setNextToken(BigInt(data.nextTokenId));
setLoading(false);
}
async function loadMore() {
if (!hasMore || loading) return;
setLoading(true);
const end = nextToken + BigInt(batchSize) - 1n;
const params = new URLSearchParams({
contractAddress,
startTokenId: nextToken.toString(),
endTokenId: end.toString(),
});
try {
const res = await fetch(`/api/supporters?${params}`);
if (!res.ok) throw new Error('Failed to fetch supporters');
const data: SupportersResponse = await res.json();
const deserialized = data.supporters.map(h => ({
...h,
tokenIds: h.tokenIds.map(id => BigInt(id)),
}));
setSupporters(prev => [...prev, ...deserialized]);
setVisibleSupporters(prev => [...prev, ...deserialized].slice(0, itemsPerPage));
setHasMore(data.hasMore);
setNextToken(BigInt(data.nextTokenId));
} catch (error) {
console.error('Error loading supporters:', error);
setHasMore(false);
} finally {
setLoading(false);
}
}
🤖 Prompt for AI Agents
In src/hooks/useSupporters.ts between lines 46 and 69, the loadMore function
lacks error handling for the fetch call and does not cancel or clean up pending
requests if the component unmounts. To fix this, wrap the fetch and subsequent
processing in a try-catch block to handle errors gracefully, setting loading to
false in finally. Also, use an AbortController to cancel the fetch request if
the component unmounts, ensuring no state updates occur after unmount.

}));

setSupporters(prev => [...prev, ...deserialized]);
setVisibleSupporters(prev => [...prev, ...deserialized].slice(0, itemsPerPage));
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: visibleSupporters incorrectly limits displayed items.

The current logic always slices to itemsPerPage, which means users will never see more than 8 items even after clicking "Load More".

-    setVisibleSupporters(prev => [...prev, ...deserialized].slice(0, itemsPerPage));
+    setVisibleSupporters(prev => {
+      const newVisible = [...prev, ...deserialized];
+      return newVisible.slice(0, prev.length + itemsPerPage);
+    });
📝 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.

Suggested change
setVisibleSupporters(prev => [...prev, ...deserialized].slice(0, itemsPerPage));
setVisibleSupporters(prev => {
const newVisible = [...prev, ...deserialized];
return newVisible.slice(0, prev.length + itemsPerPage);
});
🤖 Prompt for AI Agents
In src/hooks/useSupporters.ts at line 65, the code incorrectly limits the number
of visible supporters to itemsPerPage on every update, preventing the list from
growing beyond 8 items after "Load More" is clicked. To fix this, remove the
slice operation so that new supporters are appended without truncating the
array, allowing the visibleSupporters array to grow as more items are loaded.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (7)
src/hooks/useSupporters.ts (3)

40-44: Potential stale closure issue with useEffect dependencies.

The loadMore() function depends on hasMore and loading states, but these aren't included in the dependency array. This could lead to stale closures where the function captures outdated state values.


46-69: Missing error handling and request cleanup.

The function lacks error handling for failed API calls and doesn't clean up pending requests when the component unmounts. This could lead to unhandled errors and potential memory leaks.


65-65: Incorrect visible supporters logic limits display.

The current implementation always slices to itemsPerPage, which means users will never see more than 8 items even after clicking "Load More". The visible list should grow incrementally.

src/app/api/supporters/route.ts (4)

9-12: Reuse existing publicClient from utils.

You're creating a new publicClient instance when one already exists in src/utils/client.ts. This creates unnecessary instances and connections.


37-38: Add cache size limit to prevent memory issues.

The cache Map has no size limit and can grow indefinitely, potentially causing memory exhaustion in production environments.


79-81: Add validation for token ID range to prevent DoS.

Large token ranges could cause excessive memory usage and slow response times. Add a maximum range limit to prevent potential DoS attacks.


91-95: Add error handling for contract calls.

The contract call to read totalSupply lacks error handling. Contract calls can fail due to invalid addresses, network issues, or other reasons.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d6d9d54 and f1ac994.

📒 Files selected for processing (3)
  • src/app/api/supporters/route.ts (1 hunks)
  • src/components/droposal/SupportersSection.tsx (1 hunks)
  • src/hooks/useSupporters.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/droposal/SupportersSection.tsx

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (7)
src/app/api/supporters/route.ts (4)

9-12: Use the existing publicClient from utils.

You're redefining publicClient here when it's already available in src/utils/client.ts.

-const publicClient = createPublicClient({
-  chain: base,
-  transport: http(),
-});
+import { publicClient } from '@/utils/client';

37-44: Add cache size limit to prevent memory leaks.

The cache can grow indefinitely, potentially causing memory issues.

 const cache = new Map<string, { data: SupportersResponse; time: number }>();
 const TTL = 5 * 60 * 1000; // 5 minutes
 // Prevent overloading multicall by limiting the number of tokenIds fetched at once
 const MAX_BATCH_SIZE = 100;
+const MAX_CACHE_SIZE = 100;
+
+function addToCache(key: string, data: SupportersResponse) {
+  // Remove oldest entries if cache is full
+  if (cache.size >= MAX_CACHE_SIZE) {
+    const oldestKey = Array.from(cache.entries())
+      .sort(([, a], [, b]) => a.time - b.time)[0][0];
+    cache.delete(oldestKey);
+  }
+  cache.set(key, { data, time: Date.now() });
+}

Then update line 116:

-  cache.set(key, { data: response, time: Date.now() });
+  addToCache(key, response);

85-87: Add validation for token ID range to prevent DoS attacks.

Large token ranges could cause excessive memory usage and slow response times. While you validate batch size in fetchOwnersBatch, it's better to validate earlier in the request handler.

 const start = BigInt(searchParams.get('startTokenId') ?? '0');
 const end = BigInt(searchParams.get('endTokenId') ?? (start + 19n));
+
+// Validate range doesn't exceed batch size
+if (end - start + 1n > BigInt(MAX_BATCH_SIZE)) {
+  return NextResponse.json({ error: `Range too large, maximum ${MAX_BATCH_SIZE} tokens` }, { status: 400 });
+}
+
 const limit = parseInt(searchParams.get('limit') ?? '0');

97-101: Add error handling for contract calls.

Contract calls can fail for various reasons (invalid contract, network issues, etc.).

-  const totalSupply: bigint = await publicClient.readContract({
-    address: contract,
-    abi: zoraMintAbi,
-    functionName: 'totalSupply',
-  });
+  let totalSupply: bigint;
+  try {
+    totalSupply = await publicClient.readContract({
+      address: contract,
+      abi: zoraMintAbi,
+      functionName: 'totalSupply',
+    });
+  } catch (error) {
+    return NextResponse.json({ error: 'Failed to fetch total supply' }, { status: 500 });
+  }
src/hooks/useSupporters.ts (3)

41-45: Fix useEffect dependencies to prevent stale closures.

The effect calls loadMore() which depends on hasMore and loading states, but these aren't in the dependency array. The eslint-disable comment confirms this issue.

Consider using a callback ref pattern to avoid the dependency issue:

+  const loadMoreRef = useRef<() => Promise<void>>();
+  loadMoreRef.current = loadMore;
+
   useEffect(() => {
     if (!contractAddress) return;
-    loadMore();
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [contractAddress, totalSupply]);
+    loadMoreRef.current?.();
+  }, [contractAddress, totalSupply]);

47-70: Add error handling and request cleanup.

The function lacks error handling for failed API calls and doesn't clean up pending requests when the component unmounts.

  async function loadMore() {
    if (!hasMore || loading) return;
    setLoading(true);

    const end = nextToken + BigInt(batchSize) - 1n;
    const params = new URLSearchParams({
      contractAddress,
      startTokenId: nextToken.toString(),
      endTokenId: end.toString(),
    });

-    const res = await fetch(`/api/supporters?${params}`);
-    const data: SupportersResponse = await res.json();
+    try {
+      const res = await fetch(`/api/supporters?${params}`);
+      if (!res.ok) throw new Error('Failed to fetch supporters');
+      const data: SupportersResponse = await res.json();
      const deserialized = data.supporters.map(h => ({
        ...h,
        tokenIds: h.tokenIds.map(id => BigInt(id)),
      }));

      setSupporters(prev => [...prev, ...deserialized]);
      setVisibleSupporters(prev => [...prev, ...deserialized].slice(0, itemsPerPage));
      setHasMore(data.hasMore);
      setNextToken(BigInt(data.nextTokenId));
+    } catch (error) {
+      console.error('Error loading supporters:', error);
+      setHasMore(false);
+    } finally {
      setLoading(false);
+    }
  }

66-66: Bug: visibleSupporters incorrectly limits displayed items.

The current logic always slices to itemsPerPage, which means users will never see more than the initial page size even after clicking "Load More".

-    setVisibleSupporters(prev => [...prev, ...deserialized].slice(0, itemsPerPage));
+    setVisibleSupporters(prev => [...prev, ...deserialized]);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1ac994 and 43e0779.

📒 Files selected for processing (2)
  • src/app/api/supporters/route.ts (1 hunks)
  • src/hooks/useSupporters.ts (2 hunks)
🔇 Additional comments (2)
src/app/api/supporters/route.ts (1)

51-54: Good addition of batch size validation!

The batch size check properly prevents excessive multicall sizes that could cause memory issues or exceed RPC limits.

src/hooks/useSupporters.ts (1)

36-37: Good fix for the loading state initialization!

The loading state is now correctly initialized to false, allowing the initial loadMore call to proceed.

nextTokenId: nextTokenId.toString(),
cached: false
};
const owners = await fetchOwnersBatch(contract, start, end);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for fetchOwnersBatch call.

The function call could throw an error if the batch size exceeds the limit or if multicall fails.

-  const owners = await fetchOwnersBatch(contract, start, end);
+  let owners: { address: string; tokenId: bigint }[];
+  try {
+    owners = await fetchOwnersBatch(contract, start, end);
+  } catch (error) {
+    return NextResponse.json({ error: 'Failed to fetch token owners' }, { status: 500 });
+  }
📝 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.

Suggested change
const owners = await fetchOwnersBatch(contract, start, end);
- const owners = await fetchOwnersBatch(contract, start, end);
+ let owners: { address: string; tokenId: bigint }[];
+ try {
+ owners = await fetchOwnersBatch(contract, start, end);
+ } catch (error) {
+ return NextResponse.json({ error: 'Failed to fetch token owners' }, { status: 500 });
+ }
const holders = aggregateHolders(owners);
🤖 Prompt for AI Agents
In src/app/api/supporters/route.ts at line 103, the call to fetchOwnersBatch may
throw errors due to batch size limits or multicall failures. Wrap this call in a
try-catch block to handle potential exceptions gracefully. In the catch block,
log or handle the error appropriately and ensure the function can continue or
return a meaningful response instead of crashing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant