-
Notifications
You must be signed in to change notification settings - Fork 4
Refactor to remove SimpleArweaveDataProvider #798
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
base: develop
Are you sure you want to change the base?
Refactor to remove SimpleArweaveDataProvider #798
Conversation
WalkthroughProvider-based Arweave access is removed across the app. New hooks (price, balance, block height, ArNS) and utility validators replace Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as ArPrice Component
participant Hook as useArPrice
participant G as GlobalState (gateway)
participant A as Arweave API
UI->>Hook: invoke useArPrice(dataSize)
Hook->>G: read gateway
Hook->>A: GET /price/{dataSize} (via SDK)
A-->>Hook: price (winston)
Hook-->>UI: data (AR) or error
note over UI: On error, emit 'error'
sequenceDiagram
autonumber
actor UI as BlockHeightCounter
participant Hook as useArweaveBlockHeight
participant G as GlobalState (gateway)
participant A as Arweave API
UI->>Hook: refetch()
Hook->>G: read gateway
Hook->>A: get current block height
A-->>Hook: height
Hook-->>UI: data
UI->>G: dispatch setBlockHeight(height)
sequenceDiagram
autonumber
actor UI as NameTokenSelector
participant QC as QueryClient
participant Util as buildArNSRecordsQuery
participant C as ArNS/ARIO
UI->>Util: build query (arioContract, processId)
Util-->>UI: query config
UI->>QC: fetchQuery(config)
QC->>C: fetch records
C-->>QC: records
QC-->>UI: records
note over UI: validateArweaveId via utils (async)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (6)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (4)
src/hooks/useArNSRecord.tsx (1)
6-18: Well-structured hook, but consider adjusting staleTime.The hook implementation is clean and follows React Query best practices. However,
staleTime: Infinitymight be too aggressive since ArNS records can change over time, potentially leading to stale data issues.Consider using a more reasonable staleTime:
- staleTime: Infinity, + staleTime: 1000 * 60 * 5, // 5 minutessrc/state/contexts/WalletState.tsx (1)
174-183: Correct balance fetching implementation with potential optimization opportunity.The direct Arweave client usage and balance conversion logic are correct. The
instanceofcheck properly handles different address types.Consider extracting the Arweave client creation to avoid recreating it on every balance update:
+ const arweave = useMemo(() => new Arweave({ host: gateway, protocol: 'https' }), [gateway]); async function updateBalances(address: AoAddress, arioTicker: string) { try { - const arweave = new Arweave({ host: gateway, protocol: 'https' });src/hooks/useArBalance.tsx (1)
6-20: Well-implemented hook following React Query best practices.The hook properly handles optional addresses, includes all dependencies in the query key, and has appropriate caching strategy. The error handling and conditional enabling are correctly implemented.
Consider using
address?.toString() ?? 'no-address'in the query key instead of relying ontoString()returning an empty string for null addresses, to make the cache key more explicit:- queryKey: ['ar-balance', address?.toString(), gateway], + queryKey: ['ar-balance', address?.toString() ?? 'no-address', gateway],src/utils/arweave.ts (1)
10-30: Good validation logic but consider preserving error context.The address validation logic correctly distinguishes between transaction IDs and wallet addresses. However, the generic error message in the catch block loses potentially useful debugging information.
Consider preserving the original error for debugging:
- } catch { - throw new Error('Unable to verify this is an arweave address.'); + } catch (error) { + throw new Error(`Unable to verify this is an arweave address: ${error instanceof Error ? error.message : 'Unknown error'}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
src/components/cards/NavMenuCard/NavMenuCard.tsx(0 hunks)src/components/devtools/ArNSRegistrySettings.tsx(5 hunks)src/components/forms/DomainSettings/DescriptionRow.tsx(2 hunks)src/components/forms/DomainSettings/LogoRow.tsx(2 hunks)src/components/forms/DomainSettings/TargetIDRow.tsx(2 hunks)src/components/inputs/text/NameTokenSelector/NameTokenSelector.tsx(4 hunks)src/components/layout/ArPrice/ArPrice.tsx(1 hunks)src/components/layout/BlockHeightCounter/BlockHeightCounter.tsx(2 hunks)src/components/modals/ant-management/EditUndernameModal/EditUndernameModal.tsx(3 hunks)src/components/pages/Register/Register.tsx(3 hunks)src/components/pages/Settings/ArNSSettings.tsx(2 hunks)src/hooks/index.ts(1 hunks)src/hooks/useArBalance.tsx(1 hunks)src/hooks/useArNSRecord.tsx(1 hunks)src/hooks/useArNSReserved.tsx(1 hunks)src/hooks/useArPrice.tsx(1 hunks)src/hooks/useArweaveBlockHeight.tsx(1 hunks)src/hooks/useRegistrationStatus/useRegistrationStatus.tsx(2 hunks)src/services/arweave/ArweaveCompositeDataProvider.ts(0 hunks)src/services/arweave/SimpleArweaveDataProvider.ts(0 hunks)src/state/actions/dispatchNewGateway.ts(1 hunks)src/state/contexts/GlobalState.tsx(1 hunks)src/state/contexts/WalletState.tsx(3 hunks)src/state/reducers/GlobalReducer.ts(0 hunks)src/types.ts(0 hunks)src/utils/arweave.ts(1 hunks)src/utils/index.ts(1 hunks)src/utils/network.ts(0 hunks)tests/common/mocks/ArweaveCompositeDataProviderMock.ts(0 hunks)
💤 Files with no reviewable changes (7)
- src/components/cards/NavMenuCard/NavMenuCard.tsx
- src/state/reducers/GlobalReducer.ts
- src/utils/network.ts
- src/types.ts
- tests/common/mocks/ArweaveCompositeDataProviderMock.ts
- src/services/arweave/SimpleArweaveDataProvider.ts
- src/services/arweave/ArweaveCompositeDataProvider.ts
🧰 Additional context used
🧬 Code Graph Analysis (16)
src/components/forms/DomainSettings/LogoRow.tsx (1)
src/utils/arweave.ts (1)
validateArweaveId(6-8)
src/components/modals/ant-management/EditUndernameModal/EditUndernameModal.tsx (2)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(66-67)src/utils/arweave.ts (1)
validateArweaveId(6-8)
src/components/pages/Register/Register.tsx (2)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(66-67)src/utils/arweave.ts (1)
validateArweaveId(6-8)
src/components/forms/DomainSettings/DescriptionRow.tsx (1)
src/utils/arweave.ts (1)
validateArweaveId(6-8)
src/hooks/useArNSReserved.tsx (1)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(66-67)
src/components/forms/DomainSettings/TargetIDRow.tsx (1)
src/utils/arweave.ts (1)
validateArweaveId(6-8)
src/hooks/useArweaveBlockHeight.tsx (1)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(66-67)
src/components/layout/BlockHeightCounter/BlockHeightCounter.tsx (2)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(66-67)src/hooks/useArweaveBlockHeight.tsx (1)
useArweaveBlockHeight(5-16)
src/components/layout/ArPrice/ArPrice.tsx (1)
src/hooks/useArPrice.tsx (1)
useArPrice(5-17)
src/hooks/useArNSRecord.tsx (2)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(66-67)src/utils/searchUtils/searchUtils.ts (1)
lowerCaseDomain(128-130)
src/hooks/useArPrice.tsx (1)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(66-67)
src/state/contexts/WalletState.tsx (1)
src/services/arweave/ArweaveTransactionID.ts (1)
ArweaveTransactionID(7-31)
src/hooks/useArBalance.tsx (2)
src/services/arweave/ArweaveTransactionID.ts (1)
ArweaveTransactionID(7-31)src/state/contexts/GlobalState.tsx (1)
useGlobalState(66-67)
src/components/devtools/ArNSRegistrySettings.tsx (1)
src/utils/arweave.ts (2)
validateArweaveId(6-8)validateArweaveAddress(10-30)
src/state/contexts/GlobalState.tsx (1)
src/state/reducers/GlobalReducer.ts (1)
reducer(46-106)
src/utils/arweave.ts (2)
src/services/arweave/ArweaveTransactionID.ts (1)
ArweaveTransactionID(7-31)src/utils/constants.ts (1)
ARWEAVE_HOST(42-42)
🪛 GitHub Check: lint_test_build
src/components/forms/DomainSettings/LogoRow.tsx
[failure] 88-88:
Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
src/components/modals/ant-management/EditUndernameModal/EditUndernameModal.tsx
[failure] 129-129:
Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
src/components/pages/Register/Register.tsx
[failure] 478-478:
Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
src/components/forms/DomainSettings/DescriptionRow.tsx
[failure] 78-78:
Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
src/components/forms/DomainSettings/TargetIDRow.tsx
[failure] 88-88:
Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
src/components/devtools/ArNSRegistrySettings.tsx
[failure] 165-165:
Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
🪛 GitHub Check: build_and_test / lint_test_build
src/components/forms/DomainSettings/LogoRow.tsx
[failure] 88-88:
Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
src/components/modals/ant-management/EditUndernameModal/EditUndernameModal.tsx
[failure] 129-129:
Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
src/components/pages/Register/Register.tsx
[failure] 478-478:
Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
src/components/forms/DomainSettings/DescriptionRow.tsx
[failure] 78-78:
Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
src/components/forms/DomainSettings/TargetIDRow.tsx
[failure] 88-88:
Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
src/components/devtools/ArNSRegistrySettings.tsx
[failure] 165-165:
Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
🪛 GitHub Actions: PR deploy
src/components/devtools/ArNSRegistrySettings.tsx
[error] 165-165: TypeScript error TS2322: Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
🪛 GitHub Actions: Build and Test
src/components/devtools/ArNSRegistrySettings.tsx
[error] 165-165: TypeScript error TS2322: Type '(id: string) => ArweaveTransactionID' is not assignable to type '(value: any) => Promise'.
🔇 Additional comments (23)
src/utils/index.ts (1)
4-4: LGTM - Clean module organizationThe addition of the arweave utilities export aligns well with the refactor to remove provider abstractions. This centralizes access to validation functions like
validateArweaveIdandvalidateArweaveAddress.src/components/pages/Register/Register.tsx (2)
32-32: LGTM - Import alignment with refactorThe import of
validateArweaveIdutility function aligns with the broader refactor to remove provider abstractions.
49-49: LGTM - Global state cleanupRemoving
arweaveDataProviderfrom the global state destructuring is consistent with the refactor to eliminate the provider pattern.src/components/modals/ant-management/EditUndernameModal/EditUndernameModal.tsx (2)
2-2: LGTM - Consistent import patternAdding the
validateArweaveIdimport follows the established pattern for removing provider dependencies.
36-36: LGTM - Global state simplificationRemoving
arweaveDataProviderfrom the global state destructuring continues the cleanup of provider abstractions.src/components/forms/DomainSettings/LogoRow.tsx (2)
12-12: LGTM - Consistent refactor patternThe import change aligns with the systematic removal of provider dependencies across components.
88-88: To ensure we catch all instances, let’s retry the search without relying on Ripgrep’s built-in type definitions:#!/bin/bash # Search across .ts and .tsx files for any usage of validateArweaveId rg -C 2 "validateArweaveId" -g '*.ts' -g '*.tsx'src/hooks/index.ts (1)
5-9: LGTM - Excellent architectural improvementAdding these new hooks (
useArPrice,useArBalance,useArweaveBlockHeight,useArNSRecord,useArNSReserved) represents a solid architectural shift from the provider pattern to a more modular hook-based approach. This will provide better:
- Modularity: Each hook has a single responsibility
- Testability: Hooks can be tested in isolation
- Reusability: Components can selectively use only the data they need
- Performance: Better caching and state management with React Query
src/state/actions/dispatchNewGateway.ts (1)
19-19: Clean refactor to direct Arweave SDK usage.The replacement of provider-based block height fetching with direct
arweave.blocks.getCurrent().heightis correct and aligns well with the overall architecture simplification.src/hooks/useArNSReserved.tsx (1)
1-16: Well-structured React Query hook with appropriate caching strategy.The hook follows React Query best practices with proper query keys, conditional enabling, and clean async handling. The
staleTime: Infinityis appropriate for reserved names data which rarely changes, ensuring good performance.src/hooks/useArweaveBlockHeight.tsx (1)
1-16: Solid implementation with appropriate refresh strategy.The hook is well-structured with a reasonable 1-minute
staleTimefor frequently changing block height data. The query key correctly includes the gateway to handle cache invalidation when the gateway changes.src/components/pages/Settings/ArNSSettings.tsx (1)
37-68: LGTM! Clean refactor from provider to direct Arweave client.The conversion to async and direct Arweave client instantiation properly implements the removal of the data provider abstraction. The block height fetching logic is correct and error handling is maintained.
src/components/layout/BlockHeightCounter/BlockHeightCounter.tsx (2)
1-1: Good hook integration.The import and usage of
useArweaveBlockHeightproperly replaces the provider-based approach.
22-27: Correct refetch pattern implementation.The refetch usage maintains the existing error handling while properly utilizing the new hook pattern.
src/state/contexts/WalletState.tsx (1)
1-1: Appropriate imports for the refactor.The new imports support the transition from provider-based to direct Arweave client usage.
src/components/layout/ArPrice/ArPrice.tsx (1)
1-15: Excellent refactor to use the new useArPrice hook!The component is now much cleaner and follows the separation of concerns principle. The hook handles data fetching and caching while the component focuses on presentation and error handling.
src/components/devtools/ArNSRegistrySettings.tsx (1)
40-40: Good refactor to remove provider dependency.The changes to make
confirmSettingasync and use direct Arweave client calls are well-implemented. The block height fetching is now more straightforward and the dispatch payload simplification aligns with the provider removal.Also applies to: 64-69
src/utils/arweave.ts (1)
6-8: Simple and effective validation wrapper.The
validateArweaveIdfunction provides a clean interface for creating validated transaction IDs.src/state/contexts/GlobalState.tsx (1)
79-79: Clean simplification removing provider dependency.The removal of
arweaveDataProviderfrom the global state and the simplified initialization aligns well with the architectural shift toward hook-based data fetching. The code is now more straightforward without conditional provider merging.src/hooks/useRegistrationStatus/useRegistrationStatus.tsx (2)
14-34: Good refactoring to React Query hooks!The migration from manual async calls to React Query hooks (
useArNSRecordanduseArNSReserved) is a solid improvement. It provides better caching, error handling, and loading state management out of the box.
43-43: Verify ifarioProcessIddependency is necessary.The
arioProcessIdis included in the dependency array but not used within the effect body. This causes the effect to re-run wheneverarioProcessIdchanges.Is this intentional? If
useArNSRecordanduseArNSReservedinternally depend onarioProcessIdand handle their own re-fetching, you might not need it in this dependency array.Run the following script to check if the hooks use
arioProcessId:#!/bin/bash # Description: Check if useArNSRecord and useArNSReserved hooks use arioProcessId # Search for useArNSRecord implementation echo "=== useArNSRecord implementation ===" fd -e tsx -e ts "useArNSRecord" --exec rg -A 20 "export.*useArNSRecord" # Search for useArNSReserved implementation echo "=== useArNSReserved implementation ===" fd -e tsx -e ts "useArNSReserved" --exec rg -A 20 "export.*useArNSReserved"src/components/inputs/text/NameTokenSelector/NameTokenSelector.tsx (2)
381-381: Good use of standalone utility function.The migration to use the imported
validateArweaveIdutility function is cleaner than accessing it through the data provider.
150-155: ```shell
#!/bin/bashShow the full implementation of buildArNSRecordsQuery
echo "=== buildArNSRecordsQuery implementation (lines 99-200) ==="
sed -n '99,200p' src/utils/network.tsHighlight how the meta parameter is used
echo "=== Usage of meta within buildArNSRecordsQuery ==="
rg -n "meta" -A3 -B3 src/utils/network.ts | sed -n '1,20p'</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
src/components/modals/ant-management/EditUndernameModal/EditUndernameModal.tsx
Outdated
Show resolved
Hide resolved
| export function useArPrice(dataSize: number) { | ||
| const [{ gateway }] = useGlobalState(); | ||
|
|
||
| return useQuery({ | ||
| queryKey: ['ar-price', dataSize, gateway], | ||
| queryFn: async () => { | ||
| const arweave = new Arweave({ host: gateway, protocol: 'https' }); | ||
| const { data } = await arweave.api.get(`/price/${dataSize}`); | ||
| return +arweave.ar.winstonToAr(data, { formatted: true }); | ||
| }, | ||
| staleTime: Infinity, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Solid implementation, but staleTime needs adjustment for price data.
The hook correctly implements direct Arweave client usage and proper price conversion. However, staleTime: Infinity is inappropriate for price data which can fluctuate frequently.
Price data should have a reasonable staleTime to ensure users see current prices:
- staleTime: Infinity,
+ staleTime: 1000 * 60, // 1 minute📝 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.
| export function useArPrice(dataSize: number) { | |
| const [{ gateway }] = useGlobalState(); | |
| return useQuery({ | |
| queryKey: ['ar-price', dataSize, gateway], | |
| queryFn: async () => { | |
| const arweave = new Arweave({ host: gateway, protocol: 'https' }); | |
| const { data } = await arweave.api.get(`/price/${dataSize}`); | |
| return +arweave.ar.winstonToAr(data, { formatted: true }); | |
| }, | |
| staleTime: Infinity, | |
| }); | |
| } | |
| export function useArPrice(dataSize: number) { | |
| const [{ gateway }] = useGlobalState(); | |
| return useQuery({ | |
| queryKey: ['ar-price', dataSize, gateway], | |
| queryFn: async () => { | |
| const arweave = new Arweave({ host: gateway, protocol: 'https' }); | |
| const { data } = await arweave.api.get(`/price/${dataSize}`); | |
| return +arweave.ar.winstonToAr(data, { formatted: true }); | |
| }, | |
| - staleTime: Infinity, | |
| + staleTime: 1000 * 60, // 1 minute | |
| }); | |
| } |
🤖 Prompt for AI Agents
In src/hooks/useArPrice.tsx around lines 5 to 17, the staleTime is set to
Infinity, which is unsuitable for frequently changing price data. Adjust the
staleTime to a reasonable finite value, such as a few minutes (e.g., 5 minutes),
to ensure the price data is refreshed regularly and users see up-to-date prices.
| function reset() { | ||
| setIsAvailable(false); | ||
| setIsReserved(defaultReserved); | ||
| setValidated(false); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Move reset function outside the component or memoize it.
The reset function is redefined on every render. Since it doesn't depend on any props or state, it should be defined outside the component or memoized with useCallback.
Move the function outside the component:
+function reset(
+ setIsAvailable: React.Dispatch<React.SetStateAction<boolean>>,
+ setIsReserved: React.Dispatch<React.SetStateAction<{isReserved: boolean; reservedFor?: ArweaveTransactionID}>>,
+ setValidated: React.Dispatch<React.SetStateAction<boolean>>
+) {
+ setIsAvailable(false);
+ setIsReserved(defaultReserved);
+ setValidated(false);
+}
export function useRegistrationStatus(domain: string) {
// ... existing code ...
useEffect(() => {
if (!domain.length) {
- reset();
+ reset(setIsAvailable, setIsReserved, setValidated);
return;
}
// ... rest of effect
}, [domain, record, reserved, recordLoading, reservedLoading, arioProcessId]);
- function reset() {
- setIsAvailable(false);
- setIsReserved(defaultReserved);
- setValidated(false);
- }
return {Or use useCallback:
- function reset() {
- setIsAvailable(false);
- setIsReserved(defaultReserved);
- setValidated(false);
- }
+ const reset = useCallback(() => {
+ setIsAvailable(false);
+ setIsReserved(defaultReserved);
+ setValidated(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.
| function reset() { | |
| setIsAvailable(false); | |
| setIsReserved(defaultReserved); | |
| setValidated(false); | |
| } | |
| function reset( | |
| setIsAvailable: React.Dispatch<React.SetStateAction<boolean>>, | |
| setIsReserved: React.Dispatch<React.SetStateAction<{ isReserved: boolean; reservedFor?: ArweaveTransactionID }>>, | |
| setValidated: React.Dispatch<React.SetStateAction<boolean>> | |
| ) { | |
| setIsAvailable(false); | |
| setIsReserved(defaultReserved); | |
| setValidated(false); | |
| } | |
| export function useRegistrationStatus(domain: string) { | |
| // ... existing imports, state hooks, and queries ... | |
| useEffect(() => { | |
| if (!domain.length) { | |
| reset(setIsAvailable, setIsReserved, setValidated); | |
| return; | |
| } | |
| // ... rest of effect logic ... | |
| }, [domain, record, reserved, recordLoading, reservedLoading, arioProcessId]); | |
| return { | |
| isAvailable, | |
| reserved, | |
| validated, | |
| isLoading: recordLoading || reservedLoading | |
| }; | |
| } |
🤖 Prompt for AI Agents
In src/hooks/useRegistrationStatus/useRegistrationStatus.tsx around lines 45 to
49, the reset function is redefined on every render even though it does not
depend on any props or state. To fix this, either move the reset function
definition outside the component so it is created once, or wrap the reset
function with React's useCallback hook inside the component to memoize it and
prevent unnecessary redefinitions on each render.
| arweave: arweaveDataProvider, | ||
| }); | ||
|
|
||
| const blockHeight = (await arweave.blocks.getCurrent()).height; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use useArweaveBlockHeight
| const associatedRecords = await queryClient.fetchQuery( | ||
| buildArNSRecordsQuery({ | ||
| arioContract, | ||
| meta: [arioProcessId.toString()], | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: revert this change back
| arweave: arweaveDataProvider, | ||
| }); | ||
|
|
||
| const blockHeight = (await arweave.blocks.getCurrent()).height; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use useArweaveBlockHeight hook
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thinking further, why do we need block height here?
| const [arioBalance, arBalance] = await Promise.all([ | ||
| arweaveDataProvider.getTokenBalance(address), | ||
| arioContract | ||
| .getBalance({ address: address.toString() }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, use hook
| .then((b) => new mARIOToken(b).toARIO().valueOf()), | ||
| address instanceof ArweaveTransactionID | ||
| ? arweaveDataProvider.getArBalance(address) | ||
| ? arweave.wallets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ni: use hook
| contract: contract, | ||
| }); | ||
| const blockHeight = await provider.getCurrentBlockHeight(); | ||
| const blockHeight = (await arweave.blocks.getCurrent()).height; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we have to set block height here?
| const [{ gateway }] = useGlobalState(); | ||
|
|
||
| return useQuery({ | ||
| queryKey: ['block-height', gateway], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure we need gateway here
| const [{ gateway }] = useGlobalState(); | ||
|
|
||
| return useQuery({ | ||
| queryKey: ['ar-price', dataSize, gateway], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure we need gateway here
dtfiedler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some cleanup to do
…avecompositedataprovider-with-hooks
b431cb0 to
c71ee54
Compare
| const arweave = new Arweave({ host: gateway, protocol: 'https' }); | ||
| try { | ||
| const status = await arweave.api | ||
| .get(`/tx/${address}/status`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit - this could be a HEAD check
|
Visit the preview URL for this PR (updated for commit 53e278c): https://arns-portal--pr798-codex-replace-arweav-ktv69mef.web.app (expires Thu, 04 Sep 2025 13:12:31 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 1f59769442cb89cf2a92042a342e51fe94047b94 |
Summary
SimpleArweaveDataProviderimplementation and switch to direct Arweave callsvalidateArweaveIdTesting
yarn test(fails: The 'import.meta' meta-property is only allowed when the '--module' option is 'es2020', 'es2022', 'esnext', 'system', 'node16', or 'nodenext')https://chatgpt.com/codex/tasks/task_b_685a9fce81c08328bfedb93b8bd5f8e9
Summary by CodeRabbit