Skip to content

Feature implementation from commits 2cc3802..c365984 #3

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

Open
wants to merge 45 commits into
base: feature-base-branch-1
Choose a base branch
from

Conversation

codeOwlAI
Copy link
Owner

@codeOwlAI codeOwlAI commented Jun 29, 2025

PR Summary

Refactor Data Fetching with React Query and Add Server-Side Support

Overview

This PR refactors the data fetching approach across the application, moving from direct contract calls to React Query patterns. It adds server-side prefetching capabilities, implements new token-related hooks, and improves code organization while maintaining backward compatibility.

Change Types

Type Description
Refactor Convert direct contract calls to React Query pattern
Feature Add server-side prefetching for invoice details
Enhancement Implement new token data hooks (useTokenBalance, useTokenData)
Optimization Improve code organization and remove unused code

Affected Modules

Module / File Change Description
hooks/src/*.ts Refactor data fetching to React Query pattern
hooks/src/useTokenBalance.ts New hook for querying ERC20 token balances
hooks/src/useTokenData.ts New hook combining token metadata and balance
hooks/src/useInstantDetails.ts Replace direct contract reads with React Query
hooks/src/useInvoiceDetails.ts Add server-side prefetching capabilities
utils/src/invoice.ts Rename and optimize invoice details preparation function
utils/src/web3.ts Add server configuration for wagmi
dapp/next.config.js Update URL determination logic for different branches
graphql/src/invoice.ts Remove totalSupply field from TokenMetadata type

Notes for Reviewers

  • The refactoring maintains the same interfaces while changing the implementation details
  • Server-side prefetching includes BigInt serialization for proper data transfer
  • Several query invalidation mechanisms were removed as part of the refactoring

scottrepreneur and others added 30 commits December 19, 2023 16:10
],
});

return results.map(({ result }) => result) as unknown as [
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Unsafe Type Casting Without Validation.

Results from blockchain contract calls are forcefully cast to specific types without validation, which will cause runtime exceptions if the contract returns unexpected data.

Current Code (Diff):

-   return results.map(({ result }) => result) as unknown as [
-     number,
-     string,
-     string,
-   ];
+   try {
+     const [decimals, name, symbol] = results.map(({ result }) => result);
+     return [Number(decimals), String(name), String(symbol)] as [number, string, string];
+   } catch (error) {
+     console.error('Error processing token metadata:', error);
+     return undefined;
+   }
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
return results.map(({ result }) => result) as unknown as [
try {
const [decimals, name, symbol] = results.map(({ result }) => result);
return [Number(decimals), String(name), String(symbol)] as [number, string, string];
} catch (error) {
console.error('Error processing token metadata:', error);
return undefined;
}

],
});

return results.map(
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Unsafe Type Casting Without Validation.

Results from blockchain contract calls are forcefully cast to specific types without validation, which will cause runtime exceptions if the contract returns unexpected data.

Current Code (Diff):

-   return results.map(
-     ({ result }) => result,
-   ) as unknown as InstantInvoiceContractData;
+   try {
+     const mappedResults = results.map(({ result }) => result);
+     return [
+       mappedResults[0] as bigint,
+       mappedResults[1] as bigint,
+       Boolean(mappedResults[2]),
+       mappedResults[3] as bigint,
+       Number(mappedResults[4]),
+       Number(mappedResults[5])
+     ] as InstantInvoiceContractData;
+   } catch (error) {
+     console.error('Error processing invoice data:', error);
+     return undefined;
+   }
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
return results.map(
try {
const mappedResults = results.map(({ result }) => result);
return [
mappedResults[0] as bigint,
mappedResults[1] as bigint,
Boolean(mappedResults[2]),
mappedResults[3] as bigint,
Number(mappedResults[4]),
Number(mappedResults[5])
] as InstantInvoiceContractData;
} catch (error) {
console.error('Error processing invoice data:', error);
return undefined;
}

}: {
address: Hex | undefined;
chainId: number | undefined;
}) => ['tokenBalance', { address, chainId }] as QueryKey;
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Cache key collision: incorrect query key identifier.

The query key uses 'tokenBalance' but this hook is for instant invoice details, causing cache collisions that will break data fetching.

Current Code (Diff):

- }) => ['tokenBalance', { address, chainId }] as QueryKey;
+ }) => ['instantDetails', { address, chainId }] as QueryKey;
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
}) => ['tokenBalance', { address, chainId }] as QueryKey;
}) => ['instantDetails', { address, chainId }] as QueryKey;

);

return {
data: _.isNil(data) ? undefined : BigInt(data as string),
Copy link

Choose a reason for hiding this comment

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

🐛 Correctness Issue

Unsafe BigInt conversion may cause runtime exceptions.

The code attempts to convert data to BigInt without proper type validation, which could throw runtime exceptions if data is not a valid string representation of a number.

Current Code (Diff):

-     data: _.isNil(data) ? undefined : BigInt(data as string),
+     data: _.isNil(data) ? undefined : (typeof data === 'string' || typeof data === 'number' || typeof data === 'bigint') ? BigInt(data) : undefined,
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
data: _.isNil(data) ? undefined : BigInt(data as string),
data: _.isNil(data) ? undefined : (typeof data === 'string' || typeof data === 'number' || typeof data === 'bigint') ? BigInt(data) : undefined,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants