feat: enable raw data for all account headers#884
feat: enable raw data for all account headers#884askov wants to merge 12 commits intosolana-foundation:masterfrom
Conversation
|
@askov is attempting to deploy a commit to the Solana Foundation Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR extracts a reusable Key changes:
One P1 concern: Confidence Score: 4/5Safe to merge once the AccountDownloadDropdown removal is confirmed intentional or reinstated. One P1 finding: the download-to-disk feature previously available on UnknownAccountCard and FungibleTokenMintAccountCard has been silently dropped. All other changes are clean refactors with good test coverage; the prior loading-vs-not-found concern is now correctly resolved. app/components/account/UnknownAccountCard.tsx and app/components/account/TokenAccountSection.tsx — both lost AccountDownloadDropdown without replacement. Important Files Changed
Reviews (2): Last reviewed commit: "fix: memoize conn to not recreate it mul..." | Re-trigger Greptile |
| async function fetchRawAccountData([, url, address]: [string, string, string]): Promise<Uint8Array | null> { | ||
| const connection = new Connection(url, 'confirmed'); | ||
| const info = await connection.getAccountInfo(new PublicKey(address)); | ||
| return info ? Uint8Array.from(info.data) : null; | ||
| } |
There was a problem hiding this comment.
New
Connection instantiated on every fetch
A brand-new Connection object is created inside the SWR fetcher on every invocation. While useSWRImmutable prevents redundant calls for the same cache key, this still creates an extra connection object every time the cache key changes (different cluster or account). The existing cluster providers already manage a shared Connection instance.
Consider consuming the Connection from the cluster context, or at minimum memoizing it at module level keyed by URL, rather than constructing one inside the fetcher.
There was a problem hiding this comment.
Fixed. Allowed to reuse the connection and not to recreate it
| )} | ||
| </div> | ||
|
|
||
| <TableCardBody {...tableProps}>{showRaw ? rawContent : children}</TableCardBody> |
There was a problem hiding this comment.
rawContent is always mounted despite showRaw being false at start
AccountCardBase passes rawContent as a prop to AccountCard which immediately evaluates the JSX expression <RawAccountRows account={account} /> and creates the React element. However, since rawContent is only rendered in the tree when showRaw is true (via the ternary), RawAccountRows itself isn't mounted until the button is clicked — so the hook fires correctly on demand.
This is fine as-is, but it's worth noting that passing rawContent as a render prop () => <RawAccountRows /> instead of a pre-evaluated element would make the lazy-mounting intent more explicit and guard against future accidental eager rendering.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| export function useRawAccountDataOnMount(pubkey: PublicKey): Uint8Array | null { | ||
| const { url } = useCluster(); | ||
|
|
||
| const { data } = useSWRImmutable(getRawAccountDataKey(url, pubkey), fetchRawAccountData); | ||
|
|
||
| return data ?? null; | ||
| } |
There was a problem hiding this comment.
Indistinguishable loading vs. not-found state
useRawAccountDataOnMount returns null for both the initial loading state (SWR data is undefined, and undefined ?? null evaluates to null) and the case where fetchRawAccountData itself returns null (i.e., getAccountInfo found no account on chain).
BaseRawAccountRows treats any falsy rawData as "still loading" and shows a spinning indicator indefinitely. If a user visits a closed or non-existent account and clicks "Raw", the spinner will never resolve — there is no terminal state for "account not found".
The fix is to remove the ?? null coercion from the return value so that SWR's initial undefined (in-flight) remains distinct from the null that signals "account not found". The BaseRawAccountRowsProps type already allows rawData to be undefined, so the rendering logic just needs a third branch:
undefined→ loading spinnernull→ "Account data unavailable" messageUint8Array→ renderHexData
b41693c to
6d15487
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@greptile-apps check again |
Description
AccountCardcomponent (@features/account) that encapsulates the card shell (header, Raw/Refresh buttons, raw account data view), eliminating duplicated card markup across 13 account section filesType of change
Screenshots
Testing
Related Issues
Checklist
build:infoscript to update build information