Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis pull request performs a major architectural refactoring by upgrading Near Wallet Selector packages to v10.1.4, replacing the naxios-based contract API pattern with a wallet-selector-driven approach in the core NEAR client, substituting Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/middleware.ts (1)
30-37: Remove the unreachableelse ifbranch (lines 30–37).The conditions checking for
.tg/,.near/, and.testnet/with trailing slashes cannot execute.lastPathnameSegmentis computed aspathname.split("/").at(-1), which returns the last path segment after splitting by/. By definition, this segment cannot end with/since/is the delimiter used to split. The firstifbranch (lines 21–29) already handles the correct cases (without trailing slashes), making thiselse ifdead code.src/entities/list/hooks/useListForm.ts (1)
77-107: Potential runtime issue: Transaction object shape mismatch.The
buildTransactionfrom naxios creates transactions with a structure (receiverId,args,deposit,gasat top level), but the newcontractApi.callMultiple(fromnear-protocol/client.ts) expects aTransaction<A>type withmethod,receiverId, and action-building props that get processed throughbuildAction.Looking at the relevant code snippet,
callMultipleinternally calls:actions: [buildAction(transaction.method, transaction as CallProps<object>)]This expects each transaction to have a
methodproperty, butbuildTransactionputs the method name as the first argument and returns an object without amethodfield.Proposed fix: Use the Transaction structure expected by the new API
-import { buildTransaction } from "@wpdas/naxios"; ... const handleUnRegisterAccount = (registrants: AccountGroupItem[]) => { if (!id) return; - const allTransactions: any = []; + const allTransactions = registrants.map((registrant: AccountGroupItem) => ({ + method: "unregister", + receiverId: LISTS_CONTRACT_ACCOUNT_ID, + args: { + list_id: Number(id), + registration_id: Number(registrant.registrationId), + }, + deposit: floatToYoctoNear(0.015), + gas: "300000000000000", + })); - registrants.map((registrant: AccountGroupItem) => { - allTransactions.push( - buildTransaction("unregister", { - receiverId: LISTS_CONTRACT_ACCOUNT_ID, - args: { - list_id: Number(id), - registration_id: Number(registrant.registrationId), - }, - deposit: floatToYoctoNear(0.015), - gas: "300000000000000", - }), - ); - }); contractApi({ contractId: LISTS_CONTRACT_ACCOUNT_ID, }) .callMultiple(allTransactions)Also, if
buildTransactionis no longer used elsewhere in this file after the fix, remove the unused import on line 3.src/common/contracts/core/voting/client.ts (1)
268-274: Stale JSDoc parameter.The
@param networkdocumentation references a parameter that no longer exists in the function signature.📝 Suggested fix
/** * Creates a new VotingClient instance * @param contractId The NEAR account ID of the deployed voting contract - * @param network The NEAR network to connect to */ export const createVotingClient = (contractId: string): VotingClient => new VotingClient(contractId);
🤖 Fix all issues with AI agents
In @src/common/blockchains/near-protocol/client.ts:
- Around line 154-168: The ensureWallet function may call
state.walletSelector.wallet() and then assert state.wallet! which is unsafe if
the call returns a falsy value; update ensureWallet to capture the result of
await state.walletSelector.wallet(), verify it is non-null/non-undefined before
assigning to state.wallet, and throw a clear error (or handle appropriately) if
the returned wallet is falsy; reference the ensureWallet function,
state.walletSelector.wallet(), and state.wallet so reviewers can locate and fix
the check and assignment.
- Around line 55-63: The metadata.url in the setupUnityWallet call is copied
from the example and should point to the Potlock app instead; update the
metadata.url property in the setupUnityWallet({...}) invocation (the metadata
object) to the correct Potlock application URL or repository URL (e.g., your
live app URL or Potlock repo) so the wallet metadata accurately represents the
app.
In @src/common/contracts/core/lists/interfaces.ts:
- Around line 42-52: The registrations[].status field in the ApplyToList type is
currently typed as string — change it to use the existing RegistrationStatus
enum for consistency and type safety; update the ApplyToList definition
(specifically the registrations property within ApplyToList) to reference
RegistrationStatus instead of string so it matches UpdateRegistration.status and
Registration.status across the contracts.
In @src/common/contracts/tokens/non-fungible/client.ts:
- Around line 17-21: The client currently swallows errors by appending .catch(()
=> undefined) to contract calls; remove the .catch(...) from the nft_token
function (and the other similar call in this file) so that
nearProtocolClient.contractApi(...).view<NftTokenArgs,
NonFungibleToken>("nft_token", { args: { token_id: tokenId } }) can throw to
callers; leave the call signatures and return types unchanged and rely on
consuming code (e.g., hooks) to handle errors.
In @src/common/ui/layout/components/LazyImage.tsx:
- Around line 1-8: The current LazyImage wrapper around LazyLoadImage (symbols:
LazyImage, LazyLoadImage, LazyLoadImageProps) relies on an unmaintained library
and uses an unsafe double-cast; replace this by removing the wrapper and
migrating consumers to a maintained approach: either use native browser
lazy-loading by returning/using a plain img with loading="lazy" and preserving
LazyLoadImageProps mappings, or switch to an actively maintained package (e.g.,
react-lazyload) and adapt consumers to that API (wrap images with the new
provider/component and adjust prop names). Update exports so LazyImage and
LazyLoadImageProps no longer reference the old package, and modify any
components importing LazyImage to use the chosen replacement API.
🧹 Nitpick comments (12)
src/middleware.ts (1)
19-19: Consider using the existingisTelegramAccountIdutility.There's already an
isTelegramAccountIdfunction insrc/common/lib/id.tsthat performs the same check. Reusing it would avoid duplication and ensure consistency if the definition changes.♻️ Suggested refactor
+import { isTelegramAccountId } from "./common/lib/id"; + // ... inside the middleware function: - const isTelegramAccountId = lastPathnameSegment.endsWith(".tg"); + const isTelegramAccount = isTelegramAccountId(lastPathnameSegment);Then update line 23 to use
isTelegramAccountinstead.src/common/ui/form/hooks/enhanced.ts (1)
111-118: Consider guarding against asyncdefaultValuesfor type safety.The
defaultValuesparameter accepts async functions perUseFormProps, but lines 115 and 118 assume it's a synchronous object. If a function is passed,keys(defaultValues)returns function properties andisDeepEqualcompares a function to an object, breaking the comparison.Currently, all usages in this codebase pass synchronous objects, so this is not a present issue. However, since the type contract allows async values, consider adding a type check for defensive robustness if the API is meant to support them.
src/entities/list/models/index.ts (1)
23-36: Consider simplifying the redundant spread pattern.The current implementation spreads
stateUpdateand then explicitly overrides the same fields. This works correctly but is redundant. The explicit field assignments alone would suffice:mergeAll([ state, { ...stateUpdate, // This spreads type, finalOutcome, etc. type: stateUpdate?.type ?? state.type, // Then overrides type again // ... }, ])If the intent is defensive preservation of required fields, the explicit overrides are enough without the spread.
♻️ Simplified approach
const handleList = ( state: ListEditorState, stateUpdate?: Partial<ListEditorState>, ): ListEditorState => - mergeAll([ - state, - { - ...stateUpdate, - type: stateUpdate?.type ?? state.type, - finalOutcome: stateUpdate?.finalOutcome ?? state.finalOutcome, - modalTextState: stateUpdate?.modalTextState ?? state.modalTextState, - donation: stateUpdate?.donation ?? state.donation, - }, - ]) as ListEditorState; + ({ + ...state, + ...stateUpdate, + type: stateUpdate?.type ?? state.type, + finalOutcome: stateUpdate?.finalOutcome ?? state.finalOutcome, + modalTextState: stateUpdate?.modalTextState ?? state.modalTextState, + donation: stateUpdate?.donation ?? state.donation, + });src/common/ui/layout/components/molecules/clipboard-copy-button.tsx (1)
7-8: Consider using a typed wrapper for consistency.The
as anycast loses type safety. For consistency with theLazyImagewrapper pattern in this PR, consider using a properly typed cast:♻️ Typed wrapper approach
+import { type ComponentType } from "react"; + import { CopyToClipboard } from "react-copy-to-clipboard"; +import type { Props as CopyToClipboardProps } from "react-copy-to-clipboard"; -// Type assertion to fix React 18 compatibility issue -const CopyToClipboardComponent = CopyToClipboard as any; +// Type assertion to fix React 18 compatibility issue +const CopyToClipboardComponent = CopyToClipboard as unknown as ComponentType<CopyToClipboardProps>;src/common/ui/layout/components/molecules/social-share.tsx (1)
90-95: Consider simplifying prop passing.The spread syntax
{...{ text: share, onCopy }}works but is unconventional. Direct props would be cleaner.♻️ Suggested simplification
- <CopyToClipboardComponent {...{ text: share, onCopy }}> + <CopyToClipboardComponent text={share} onCopy={onCopy}> <Button className="w-full hover:cursor-pointer" variant="standard-outline"> {copied ? <Check color="#A6A6A6" size={20} className="" /> : <CopyPasteIcon />} Copy Link </Button> </CopyToClipboardComponent>src/entities/campaign/models/index.ts (1)
19-31: Redundant merge logic with explicit fallbacks.The
...stateUpdatespread combined withmergeAll([state, ...])already preservesstatevalues whenstateUpdatedoesn't provide them. The explicit fallbacks fortype,finalOutcome, andmodalTextStateduplicate this behavior.♻️ Simplified approach
const handleCampaign = ( state: CampaignEditorState, stateUpdate?: Partial<CampaignEditorState>, ): CampaignEditorState => - mergeAll([ - state, - { - ...stateUpdate, - type: stateUpdate?.type ?? state.type, - finalOutcome: stateUpdate?.finalOutcome ?? state.finalOutcome, - modalTextState: stateUpdate?.modalTextState ?? state.modalTextState, - }, - ]) as CampaignEditorState; + mergeAll([state, stateUpdate ?? {}]) as CampaignEditorState;If there's a specific reason these three fields need special handling (e.g., preventing
undefinedvalues from overwriting existing state), please add a comment clarifying that intent.src/common/lib/protocol-config.ts (1)
19-27: Consider adding error handling and async cleanup.The migration to
contractApilooks correct. However, the effect lacks error handling (failures are silently ignored) and doesn't cancel pending promises on unmount, which could lead to state updates on unmounted components.♻️ Suggested improvement
useEffect(() => { + let cancelled = false; + if (configContractId && configViewMethodName) { contractApi({ contractId: configContractId }) .view<{}, ProtocolConfig>(configViewMethodName) .then((config) => { - setConfig(config); + if (!cancelled) setConfig(config); - }); + }) + .catch((error) => { + console.error("Failed to fetch protocol config:", error); + }); } + + return () => { + cancelled = true; + }; }, [configContractId, configViewMethodName]);src/common/contracts/social-db/client.ts (1)
187-194: Redundant async pattern: mixingawaitwith.then().catch().Using
awaitwith.then().catch()is unusual and potentially confusing. IfcallMultiplerejects,awaitwill throw before.then()executes, making the.catch()unreachable for rejection handling.♻️ Suggested fix
- await contractApi() - .callMultiple([buildContract]) - .then((data) => { - console.info(data); - }) - .catch((error) => { - console.error(error); - }); + const data = await contractApi().callMultiple([buildContract]); + console.info(data);The outer
try/catchat line 164 already handles errors, so the inner.catch()is redundant.src/common/contracts/core/campaigns/client.ts (2)
51-77: Third transaction relies on implicitreceiverIdfromcontractApi.The
create_campaigntransaction at lines 71-76 omitsreceiverId, which means it will use thetargetContractIdfrom thecontractApiinstance (CAMPAIGNS_CONTRACT_ACCOUNT_ID). This is correct but differs from the explicitreceiverIdpattern used in the first two transactions.For consistency and clarity, consider adding the explicit
receiverId:♻️ Optional: Add explicit receiverId
{ + receiverId: CAMPAIGNS_CONTRACT_ACCOUNT_ID, method: "create_campaign", args: rest, deposit: floatToYoctoNear(0.021), gas: FULL_TGAS, },
62-69: Non-null assertions onparseNearAmountresults.
parseNearAmountreturnsstring | null, and the non-null assertions (!) assume the inputs are always valid. While these specific inputs ("0.05" and the computeddepositFloat) should always be valid, consider defensive handling to catch unexpected issues:♻️ Optional: Defensive null check
- deposit: parseNearAmount(depositFloat)!, + deposit: parseNearAmount(depositFloat) ?? floatToYoctoNear(0.1),Alternatively, if
floatToYoctoNearis already available and handles the conversion reliably, it could replaceparseNearAmounthere for consistency with line 74.src/common/contracts/core/donation/client.ts (1)
63-68: Consider using a named constant for the gas value.The hardcoded gas value
"100000000000000"(100 TGas) differs fromFULL_TGAS(300 TGas) used elsewhere in this file. Consider extracting this to a named constant for consistency and maintainability.♻️ Suggested refactor
+// In constants.ts or at file top +const STORAGE_DEPOSIT_GAS = "100000000000000"; // 100 TGas export const storage_deposit = (depositAmountYocto: IndivisibleUnits) => donationContractApi.call<{}, IndivisibleUnits>("storage_deposit", { deposit: depositAmountYocto, args: {}, - gas: "100000000000000", + gas: STORAGE_DEPOSIT_GAS, });src/features/matching-pool-contribution/hooks/forms.ts (1)
84-88: Consider providing user feedback on errors.The catch block only logs errors to the console. Users won't be informed if the transaction fails before reaching the wallet confirmation screen.
♻️ Suggested improvement
Consider using a toast notification or error state to inform users:
+import { toast } from "@/common/ui/toast"; // or your toast utility } catch (e) { console.error(e); + toast.error("Failed to process contribution. Please try again."); } finally {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (52)
package.jsonsrc/common/blockchains/near-protocol/client.tssrc/common/contracts/core/campaigns/client.tssrc/common/contracts/core/donation/client.tssrc/common/contracts/core/lists/client.tssrc/common/contracts/core/lists/interfaces.tssrc/common/contracts/core/pot-factory/client.tssrc/common/contracts/core/pot/client.tssrc/common/contracts/core/sybil-resistance/client.tssrc/common/contracts/core/voting/client.tssrc/common/contracts/metapool/liquid-staking/client.tssrc/common/contracts/ref-finance/ref-exchange/client.tssrc/common/contracts/social-db/client.tssrc/common/contracts/sputnikdao2/client.tssrc/common/contracts/tokens/fungible/client.tssrc/common/contracts/tokens/non-fungible/client.tssrc/common/lib/protocol-config.tssrc/common/ui/form/hooks/enhanced.tssrc/common/ui/layout/components/LazyImage.tsxsrc/common/ui/layout/components/atoms/badge.tsxsrc/common/ui/layout/components/atoms/button.tsxsrc/common/ui/layout/components/atoms/filter-chip.tsxsrc/common/ui/layout/components/atoms/select.tsxsrc/common/ui/layout/components/molecules/clipboard-copy-button.tsxsrc/common/ui/layout/components/molecules/social-share.tsxsrc/common/wallet/components/providers.tsxsrc/entities/_shared/account/components/profile-images.tsxsrc/entities/campaign/components/CampaignBanner.tsxsrc/entities/campaign/components/CampaignCard.tsxsrc/entities/campaign/components/CampaignCarouselItem.tsxsrc/entities/campaign/models/effects.tssrc/entities/campaign/models/index.tssrc/entities/list/components/AccountCard.tsxsrc/entities/list/components/ListCard.tsxsrc/entities/list/components/ListDetails.tsxsrc/entities/list/components/ListHero.tsxsrc/entities/list/hooks/useListForm.tssrc/entities/list/models/index.tssrc/entities/post/components/PostCard.tsxsrc/entities/post/components/PostEditor.tsxsrc/features/donation/models/effects/campaign-ft-donation.tssrc/features/donation/models/effects/direct-ft-donation.tssrc/features/matching-pool-contribution/hooks/forms.tssrc/features/pot-application/hooks/forms.tssrc/features/pot-configuration/model/index.tssrc/features/pot-configuration/utils/normalization.tssrc/features/profile-configuration/components/image-upload.tsxsrc/features/profile-configuration/models/effects.tssrc/layout/components/user-menu.tsxsrc/middleware.tssrc/pages/feed/[account]/[block]/index.tsxsrc/pages/profile/[accountId]/feed.tsx
🧰 Additional context used
🧠 Learnings (14)
📚 Learning: 2025-02-14T13:33:35.107Z
Learnt from: carina-akaia
Repo: PotLock/potlock-nextjs-app PR: 341
File: src/common/contracts/core/campaigns/client.ts:36-45
Timestamp: 2025-02-14T13:33:35.107Z
Learning: Error handling for NEAR contract calls should be implemented in the consuming code, not at the contract client level. The contract client layer (`src/common/contracts/core/campaigns/client.ts`) should remain focused on making the calls without error handling logic.
Applied to files:
src/common/contracts/tokens/non-fungible/client.tssrc/common/contracts/core/pot/client.tssrc/common/contracts/core/sybil-resistance/client.tssrc/features/donation/models/effects/campaign-ft-donation.tssrc/common/contracts/tokens/fungible/client.tssrc/common/contracts/core/campaigns/client.tssrc/common/contracts/sputnikdao2/client.ts
📚 Learning: 2025-04-17T14:16:13.402Z
Learnt from: carina-akaia
Repo: PotLock/potlock-nextjs-app PR: 365
File: src/features/donation/models/effects/index.ts:155-174
Timestamp: 2025-04-17T14:16:13.402Z
Learning: The transaction outcome handling in src/features/donation/models/effects/index.ts is temporary and will be replaced with `nearRps.txStatus` in the future, so extensive refinements to the current implementation aren't necessary.
Applied to files:
src/entities/campaign/models/effects.tssrc/common/contracts/core/donation/client.tssrc/features/matching-pool-contribution/hooks/forms.tssrc/features/donation/models/effects/direct-ft-donation.tssrc/features/donation/models/effects/campaign-ft-donation.tssrc/common/contracts/tokens/fungible/client.tssrc/features/profile-configuration/models/effects.tssrc/common/contracts/core/campaigns/client.ts
📚 Learning: 2025-03-12T14:17:45.439Z
Learnt from: carina-akaia
Repo: PotLock/potlock-nextjs-app PR: 357
File: src/common/contracts/core/campaigns/client.ts:52-52
Timestamp: 2025-03-12T14:17:45.439Z
Learning: In the PotLock codebase, using `{}` as a type parameter is acceptable for the `contractApi.call` method in the campaigns contract client, despite the typical TypeScript recommendation to avoid empty object types.
Applied to files:
src/common/lib/protocol-config.tssrc/common/contracts/core/pot/client.tssrc/common/contracts/core/sybil-resistance/client.tssrc/common/contracts/core/campaigns/client.tssrc/common/contracts/core/pot-factory/client.ts
📚 Learning: 2025-05-24T01:18:16.844Z
Learnt from: akaia-shadowfox
Repo: PotLock/potlock-nextjs-app PR: 388
File: src/common/contracts/core/campaigns/client.ts:22-22
Timestamp: 2025-05-24T01:18:16.844Z
Learning: In the PotLock codebase, using `{}` as a type parameter is acceptable for the `contractApi.view` method in the campaigns contract client when the contract method doesn't accept arguments, despite the typical TypeScript recommendation to avoid empty object types.
Applied to files:
src/common/lib/protocol-config.tssrc/common/contracts/core/pot/client.tssrc/common/contracts/core/campaigns/client.tssrc/common/contracts/core/pot-factory/client.ts
📚 Learning: 2025-04-27T20:33:00.401Z
Learnt from: akaia-shadowfox
Repo: PotLock/potlock-nextjs-app PR: 366
File: src/features/donation/models/effects/ft.ts:206-239
Timestamp: 2025-04-27T20:33:00.401Z
Learning: The donation FT multicall function in src/features/donation/models/effects/ft.ts is designed to be executed only in browser environments, making browser-only globals like `atob()` acceptable to use.
Applied to files:
src/common/contracts/core/donation/client.tssrc/features/donation/models/effects/direct-ft-donation.tssrc/features/donation/models/effects/campaign-ft-donation.ts
📚 Learning: 2025-04-17T13:52:53.233Z
Learnt from: carina-akaia
Repo: PotLock/potlock-nextjs-app PR: 365
File: src/features/donation/models/effects/ft.ts:198-205
Timestamp: 2025-04-17T13:52:53.233Z
Learning: When working with NEAR blockchain smart contracts, they will reject transactions with incorrectly formatted amounts rather than silently processing them, providing an additional layer of safety for FT (Fungible Token) transfers.
Applied to files:
src/features/donation/models/effects/direct-ft-donation.tssrc/features/donation/models/effects/campaign-ft-donation.tssrc/common/contracts/tokens/fungible/client.ts
📚 Learning: 2025-04-27T20:37:55.330Z
Learnt from: akaia-shadowfox
Repo: PotLock/potlock-nextjs-app PR: 366
File: src/features/donation/models/effects/ft.ts:37-41
Timestamp: 2025-04-27T20:37:55.330Z
Learning: In the donation flow, message length is already restricted through schema validation with `message: string().max(DONATION_MAX_MESSAGE_LENGTH)` where `DONATION_MAX_MESSAGE_LENGTH = 100`. This validation prevents unbounded messages that could lead to disproportionate storage deposits in the `donationFtMulticall` function.
Applied to files:
src/features/donation/models/effects/direct-ft-donation.ts
📚 Learning: 2025-02-14T13:55:21.902Z
Learnt from: carina-akaia
Repo: PotLock/potlock-nextjs-app PR: 341
File: src/entities/campaign/hooks/forms.ts:94-116
Timestamp: 2025-02-14T13:55:21.902Z
Learning: The toast component from shadcn/ui is imported from "@/hooks/use-toast". For success messages, use a simple toast with description. For error states, use the "destructive" variant.
Applied to files:
src/entities/list/components/AccountCard.tsx
📚 Learning: 2025-06-02T18:44:53.590Z
Learnt from: akaia-shadowfox
Repo: PotLock/potlock-nextjs-app PR: 400
File: src/features/donation/components/modal-confirmation-screen.tsx:83-88
Timestamp: 2025-06-02T18:44:53.590Z
Learning: In src/features/donation/components/modal-confirmation-screen.tsx, the isFeeBypassAllowed flag is temporarily hardcoded to true for urgent shipping, with a TODO comment indicating it should be replaced with campaign permission-based logic using campaign?.allow_fee_avoidance ?? false for campaign donations.
Applied to files:
src/features/donation/models/effects/campaign-ft-donation.ts
📚 Learning: 2025-06-02T18:39:56.068Z
Learnt from: akaia-shadowfox
Repo: PotLock/potlock-nextjs-app PR: 400
File: src/features/donation/components/modal-confirmation-screen.tsx:174-196
Timestamp: 2025-06-02T18:39:56.068Z
Learning: The `useWalletUserSession` hook in this codebase returns a `WalletUserSession` object that is never `undefined`. Even when a user is not signed in, it returns an object with `isSignedIn: false` and the appropriate property values. The hook does not return `undefined` itself.
Applied to files:
src/common/wallet/components/providers.tsx
📚 Learning: 2025-02-26T14:09:01.815Z
Learnt from: carina-akaia
Repo: PotLock/potlock-nextjs-app PR: 352
File: src/features/pot-configuration/hooks/forms.ts:134-137
Timestamp: 2025-02-26T14:09:01.815Z
Learning: In the `useWalletUserSession` hook, when `isSignedIn` is true, it guarantees that `accountId` is available, making additional checks for `accountId` redundant when used alongside `viewer.hasWalletReady && viewer.isSignedIn`.
Applied to files:
src/common/wallet/components/providers.tsx
📚 Learning: 2025-06-25T12:26:52.941Z
Learnt from: akaia-shadowfox
Repo: PotLock/potlock-nextjs-app PR: 418
File: src/layout/profile/components/team.tsx:21-37
Timestamp: 2025-06-25T12:26:52.941Z
Learning: The `useAccountSocialProfile` hook in `src/entities/_shared/account/hooks/social-profile.ts` returns `avatar` and `cover` objects where the `.url` property is guaranteed to be defined. The hook uses `useAccountSocialImageSrc` with fallback URLs (placeholder constants) to ensure these properties are never undefined.
Applied to files:
src/common/contracts/social-db/client.tssrc/entities/post/components/PostEditor.tsxsrc/entities/_shared/account/components/profile-images.tsx
📚 Learning: 2025-06-25T12:26:52.941Z
Learnt from: akaia-shadowfox
Repo: PotLock/potlock-nextjs-app PR: 418
File: src/layout/profile/components/team.tsx:21-37
Timestamp: 2025-06-25T12:26:52.941Z
Learning: The `useAccountSocialProfile` hook in `src/entities/_shared/account/hooks/social-profile.ts` returns `avatar` and `cover` objects where the `.url` property is guaranteed to be defined. The hook uses `useAccountSocialImageSrc` with fallback URLs (placeholder constants) to ensure these properties are never undefined through the fallback chain: `nftMediaSrc ?? rawImageSrc ?? fallbackUrl`.
Applied to files:
src/common/contracts/social-db/client.tssrc/entities/post/components/PostEditor.tsxsrc/entities/_shared/account/components/profile-images.tsx
📚 Learning: 2025-05-13T03:39:29.454Z
Learnt from: akaia-shadowfox
Repo: PotLock/potlock-nextjs-app PR: 370
File: src/features/donation/models/effects/index.ts:60-63
Timestamp: 2025-05-13T03:39:29.454Z
Learning: In the PotLock project, `ByPotId` type ensures that `potId` is a required property of type `AccountId`. The codebase has TypeScript type checking in precommit hooks that prevents pushing code with type mismatches.
Applied to files:
src/common/contracts/core/pot-factory/client.ts
🧬 Code graph analysis (34)
src/entities/campaign/components/CampaignCard.tsx (1)
src/common/ui/layout/components/LazyImage.tsx (1)
LazyImage(6-6)
src/common/contracts/tokens/non-fungible/client.ts (1)
src/common/blockchains/near-protocol/index.ts (1)
nearProtocolClient(8-8)
src/pages/profile/[accountId]/feed.tsx (1)
src/entities/post/components/PostCard.tsx (1)
PostCard(27-141)
src/features/pot-application/hooks/forms.ts (3)
src/common/blockchains/near-protocol/client.ts (2)
contractApi(223-293)accountId(196-198)src/common/contracts/core/pot/client.ts (1)
contractApi(22-25)src/common/constants.ts (2)
MIN_PROPOSAL_DEPOSIT_FALLBACK(151-151)FULL_TGAS(145-145)
src/common/contracts/core/lists/client.ts (2)
src/common/blockchains/near-protocol/client.ts (1)
contractApi(223-293)src/common/contracts/core/pot/client.ts (1)
contractApi(22-25)
src/features/profile-configuration/components/image-upload.tsx (1)
src/common/ui/layout/components/LazyImage.tsx (1)
LazyImage(6-6)
src/common/contracts/core/voting/client.ts (2)
src/common/blockchains/near-protocol/client.ts (1)
contractApi(223-293)src/common/contracts/core/pot/client.ts (1)
contractApi(22-25)
src/entities/list/components/ListDetails.tsx (1)
src/common/ui/layout/components/LazyImage.tsx (1)
LazyImage(6-6)
src/common/lib/protocol-config.ts (2)
src/common/blockchains/near-protocol/client.ts (1)
contractApi(223-293)src/common/contracts/core/pot/client.ts (1)
contractApi(22-25)
src/layout/components/user-menu.tsx (1)
src/common/ui/layout/components/LazyImage.tsx (1)
LazyImage(6-6)
src/common/contracts/core/pot/client.ts (1)
src/common/blockchains/near-protocol/index.ts (1)
nearProtocolClient(8-8)
src/common/contracts/core/donation/client.ts (4)
src/common/blockchains/near-protocol/client.ts (1)
contractApi(223-293)src/common/contracts/core/donation/interfaces.ts (4)
DirectDonationConfig(3-13)DirectDonation(15-26)DirectDonationArgs(28-33)DirectBatchDonationItem(35-38)src/common/types.ts (1)
IndivisibleUnits(92-92)src/common/constants.ts (1)
FULL_TGAS(145-145)
src/entities/campaign/components/CampaignBanner.tsx (1)
src/common/ui/layout/components/LazyImage.tsx (1)
LazyImage(6-6)
src/common/contracts/core/lists/interfaces.ts (1)
src/common/types.ts (1)
ListId(124-124)
src/pages/feed/[account]/[block]/index.tsx (1)
src/common/ui/layout/components/LazyImage.tsx (1)
LazyImage(6-6)
src/common/contracts/core/sybil-resistance/client.ts (2)
src/common/blockchains/near-protocol/client.ts (1)
contractApi(223-293)src/common/contracts/core/pot/client.ts (1)
contractApi(22-25)
src/common/contracts/ref-finance/ref-exchange/client.ts (3)
src/common/blockchains/near-protocol/client.ts (1)
contractApi(223-293)src/common/contracts/core/pot/client.ts (1)
contractApi(22-25)src/common/types.ts (1)
AccountId(45-45)
src/features/matching-pool-contribution/hooks/forms.ts (3)
src/common/blockchains/near-protocol/client.ts (1)
contractApi(223-293)src/common/contracts/core/pot/client.ts (1)
contractApi(22-25)src/common/constants.ts (2)
MIN_PROPOSAL_DEPOSIT_FALLBACK(151-151)FULL_TGAS(145-145)
src/features/donation/models/effects/direct-ft-donation.ts (1)
src/common/blockchains/near-protocol/index.ts (1)
nearProtocolClient(8-8)
src/entities/list/components/AccountCard.tsx (1)
src/common/ui/layout/components/LazyImage.tsx (1)
LazyImage(6-6)
src/features/donation/models/effects/campaign-ft-donation.ts (1)
src/common/blockchains/near-protocol/index.ts (1)
nearProtocolClient(8-8)
src/common/wallet/components/providers.tsx (2)
src/common/blockchains/near-protocol/index.ts (1)
nearProtocolClient(8-8)src/common/blockchains/near-protocol/client.ts (1)
walletSelector(188-190)
src/common/contracts/metapool/liquid-staking/client.ts (3)
src/common/blockchains/near-protocol/client.ts (1)
contractApi(223-293)src/common/contracts/core/pot/client.ts (1)
contractApi(22-25)src/common/types.ts (1)
IndivisibleUnits(92-92)
src/common/contracts/tokens/fungible/client.ts (3)
src/common/blockchains/near-protocol/index.ts (1)
nearProtocolClient(8-8)src/common/contracts/tokens/fungible/interfaces.ts (1)
FungibleTokenMetadata(1-9)src/common/types.ts (2)
ByAccountId(47-49)ByTokenId(117-122)
src/entities/list/hooks/useListForm.ts (2)
src/common/blockchains/near-protocol/client.ts (1)
contractApi(223-293)src/common/contracts/core/pot/client.ts (1)
contractApi(22-25)
src/features/profile-configuration/models/effects.ts (1)
src/common/blockchains/near-protocol/index.ts (1)
nearProtocolClient(8-8)
src/entities/post/components/PostCard.tsx (1)
src/common/ui/layout/components/LazyImage.tsx (1)
LazyImage(6-6)
src/entities/list/components/ListCard.tsx (1)
src/common/ui/layout/components/LazyImage.tsx (1)
LazyImage(6-6)
src/middleware.ts (1)
src/common/lib/id.ts (1)
isTelegramAccountId(9-9)
src/common/contracts/social-db/client.ts (2)
src/common/blockchains/near-protocol/client.ts (1)
contractApi(223-293)src/common/contracts/core/pot/client.ts (1)
contractApi(22-25)
src/features/pot-configuration/utils/normalization.ts (1)
src/common/lib/format.ts (1)
yoctoNearToFloat(25-26)
src/entities/post/components/PostEditor.tsx (1)
src/common/ui/layout/components/LazyImage.tsx (1)
LazyImage(6-6)
src/entities/_shared/account/components/profile-images.tsx (1)
src/common/ui/layout/components/LazyImage.tsx (1)
LazyImage(6-6)
src/entities/list/models/index.ts (1)
src/entities/list/types.ts (1)
ListEditorState(35-52)
🪛 GitHub Actions: Execute all unit tests
src/entities/list/components/ListHero.tsx
[error] 3-3: Cannot access 'vi_import_2' before initialization while mocking module in Vitest. This is likely caused by top-level variables inside a vi.mock factory.
⏰ 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). (1)
- GitHub Check: build (20.x)
🔇 Additional comments (66)
src/features/pot-configuration/utils/normalization.ts (2)
109-109: LGTM! Cleaner conditional syntax.The refactoring simplifies the
conditionalcall by passing the default function directly (() => 0.01) instead of using theconditional.defaultCasewrapper. The default of 0.01 NEAR as minimum matching pool donation is preserved.
149-152: LGTM! Consistent simplification of conditional default case.Same pattern as above — the default case function
() => undefinedis passed directly, maintaining the behavior where nullish input values result inundefinedbeing passed to the contract.src/features/profile-configuration/models/effects.ts (3)
115-171: LGTM - API migration for DAO proposal path.The DAO proposal submission path correctly uses
nearProtocolClient.contractApi().callMultiple(...). The transaction mapping logic (wrapping each transaction in anadd_proposalcall) and the chained promise handling remain unchanged.
1-1: [Your rewritten review comment text here]
[Exactly ONE classification tag]
102-110: LGTM - API migration for non-DAO path.The change from
naxiosInstance.contractApi()tocontractApi()aligns with the PR objective of replacing the naxios-based contract API. ThenearProtocolClient.contractApi()correctly returns an object with acallMultiplemethod that accepts theTransaction<object>[]format. Error handling remains properly positioned in the consuming code via.catch(), and the success/failure response structure is intact.src/middleware.ts (1)
21-29: LGTM!The telegram account ID check is correctly integrated into the existing condition. The rewrite logic for appending
/homeis consistent with the handling of other account types.src/entities/list/components/ListHero.tsx (2)
40-41: LGTM: Line wrapping adjustment.The text content remains identical; this is purely a formatting change with no functional impact.
3-3: Pipeline failure originates from test setup, not this file's changes.The reported TDZ error on line 3 (
import { useRouter } from "next/navigation";) stems from Vitest's module hoisting behavior in the test file_tests/homepage.tests.tsxat line 9:vi.mock("next/navigation", () => NextNavigationMock). Sincevi.mockcalls are hoisted before imports execute, the callback tries to referenceNextNavigationMockbefore it's initialized. Move the mock setup to a Vitest setup file or refactor the factory function to avoid this hoisting conflict.src/common/contracts/tokens/non-fungible/client.ts (1)
26-30: LGTM!Consistent refactor pattern applied to
nft_metadata. The change mirrors thenft_tokenupdate and maintains the existing behavior.src/common/ui/layout/components/atoms/button.tsx (1)
89-92: LGTM!Formatting-only change to collapse the
extendsclause onto a single line. No functional impact.src/features/profile-configuration/components/image-upload.tsx (3)
8-8: LGTM!Migrating to the
LazyImagewrapper for improved TypeScript compatibility. The wrapper re-exportsLazyLoadImagewith proper typing, addressing SSR conflicts mentioned in the PR objectives.
58-62: LGTM!Props are correctly passed to the new
LazyImagecomponent.
101-105: LGTM!Profile image rendering correctly uses
LazyImagewith appropriate props.src/common/ui/layout/components/atoms/filter-chip.tsx (1)
57-62: LGTM!Formatting-only change consistent with the style cleanup applied to other UI atom components.
src/features/pot-configuration/model/index.ts (1)
27-35: LGTM!The updated
handleStepfunction correctly:
- Merges state with updates using
mergeAll- Ensures
currentStepis always set to the providedstep- Preserves
finalOutcomefromstateUpdateif provided, otherwise retains the existing state'sfinalOutcomeThe explicit return type annotation and type assertion improve type safety.
src/features/pot-application/hooks/forms.ts (2)
4-4: Retained@wpdas/naxiosdependency forcalculateDepositByDataSize.The PR objective states removing naxios due to SSR conflicts, but
calculateDepositByDataSizeis still imported from@wpdas/naxios. If the goal is to fully eliminate naxios, consider reimplementing this utility locally or sourcing it from an alternative package.Please verify if this import introduces SSR issues or if
calculateDepositByDataSizeis safe to retain.
82-97: LGTM!The migration from
naxiosInstance.contractApi()to the newcontractApi()is correctly implemented. The call signature withargs,deposit,gas, andcallbackUrlmatches the contract client interface.src/entities/campaign/models/effects.ts (1)
9-11: LGTM!The new
resetStatemethod provides a clean way to reset the campaign editor state by dispatching the reset action.src/common/ui/layout/components/atoms/badge.tsx (1)
34-35: LGTM!Formatting-only change that consolidates the interface declaration to a single line without altering behavior or the public API.
src/common/ui/layout/components/molecules/social-share.tsx (1)
18-20: Acceptable workaround for React 18 compatibility.The
as anycast addresses the known typing issue wherereact-copy-to-clipboardexpects the legacychildrenprop type. This pattern aligns with the similar fix inclipboard-copy-button.tsx.src/entities/list/components/AccountCard.tsx (2)
26-26: LGTM!Import updated to use the
LazyImagewrapper for React 18 compatibility. This aligns with the PR-wide migration pattern.
86-86: LGTM!Usage correctly migrated to
LazyImagewith identical props, maintaining the same rendering behavior.src/entities/list/components/ListCard.tsx (2)
9-9: LGTM!Import correctly updated to use the
LazyImagewrapper.
86-92: LGTM!The
LazyImagecomponent is used correctly with appropriate props for the list cover image.src/entities/post/components/PostEditor.tsx (2)
6-6: LGTM!Import updated to use the
LazyImagewrapper, consistent with the PR-wide migration.
37-43: LGTM!The
LazyImagecomponent is correctly used for the avatar. Based on learnings,avatar.urlis guaranteed to be defined by theuseAccountSocialProfilehook's fallback mechanism.src/entities/post/components/PostCard.tsx (1)
9-9: LGTM! LazyImage migration looks correct.The swap from
LazyLoadImageto the newLazyImagewrapper preserves all necessary props and aligns with the project-wide migration for React 18 typing compatibility.Also applies to: 130-136
src/entities/campaign/components/CampaignCarouselItem.tsx (1)
7-7: LGTM! LazyImage migration is consistent.The component correctly swaps to
LazyImagewhile preserving the same props for the campaign cover image rendering.Also applies to: 24-30
src/common/ui/layout/components/atoms/select.tsx (1)
14-19: LGTM! Formatting-only change.The interface declaration reformatting improves readability without altering functionality.
src/entities/_shared/account/components/profile-images.tsx (1)
1-1: LGTM! Clean migration with proper type import.Good use of
import typeforLazyLoadImageProps(line 1) - this ensures the type is only used at compile time and won't affect the bundle. TheLazyImageswap inAccountProfileCoverpreserves all props includingplaceholderSrc,visibleByDefault, andwrapperClassName.Also applies to: 5-5, 61-70
src/pages/feed/[account]/[block]/index.tsx (1)
9-9: LGTM! LazyImage migration consistent with other components.The swap to
LazyImagefor the IPFS post image is correctly implemented, preserving the same source URL construction and dimensions.Also applies to: 99-105
src/pages/profile/[accountId]/feed.tsx (1)
89-91: LGTM!The multi-line formatting improves readability without any functional change.
src/entities/campaign/components/CampaignBanner.tsx (2)
16-16: LGTM!Import updated to use the new
LazyImagewrapper, consistent with the project-wide migration for TypeScript JSX compatibility.
115-118: LGTM!The
LazyImagecomponent is used correctly with the same props as the previousLazyLoadImageusage.src/layout/components/user-menu.tsx (2)
16-16: LGTM!Import updated to use the
LazyImagewrapper.
68-73: LGTM!The
LazyImagecomponent is correctly used with appropriate props for the registration status icon.src/entities/campaign/components/CampaignCard.tsx (2)
8-8: LGTM!Import updated to use the
LazyImagewrapper.
31-37: LGTM with a note on the hover effect.The
LazyImagecomponent is used correctly. Thehover:scale-150class will scale the image significantly on hover. Ensure the parent container (line 30) hasoverflow-hiddenif you want to contain the scaled image within bounds, otherwise the scaled image may visually overflow.src/entities/list/components/ListDetails.tsx (3)
22-22: LGTM!Import updated to use the
LazyImagewrapper.
282-292: Verify: The conditional logic appears inverted.The condition on lines 285-287 shows
large_default_backdrop.pngwhenlistDetails.cover_image_urlis truthy, andlist_bg_image.pngwhen it's falsy. This seems counterintuitive—typically you'd show a default/fallback image when the cover URL is missing, not when it exists.If this is intentional (e.g., this first
LazyImageserves as a decorative backdrop separate from the actual cover), please disregard. Otherwise, the ternary condition may need to be flipped.
294-300: LGTM!The
LazyImagecomponent correctly displays the list cover image with an appropriate fallback.src/common/contracts/core/voting/client.ts (1)
1-1: Clean migration to the new contractApi pattern.The constructor simplification and type alignment with
ReturnType<typeof contractApi>follow the established pattern across other contract clients in this PR.Also applies to: 22-22, 28-30
src/common/blockchains/near-protocol/client.ts (2)
256-262: Verify behavior for void-returning contract methods.The check throwing when
result === undefinedmay reject valid transactions from contract methods that don't return a value. Some NEAR contract methods legitimately return nothing.Consider whether methods like
pause()andunpause()(which returnPromise<void>) in VotingClient would be affected by this check.
41-80: Comprehensive wallet module setup.The wallet selector configuration includes a good range of wallet options. The commented-out Ethereum wallets and NearSnap with explanatory comments are helpful for maintainability.
src/common/contracts/tokens/fungible/client.ts (1)
6-18: LGTM!The migration to
nearProtocolClient.contractApiis consistent with the broader refactoring pattern. Error suppression with.catch(() => undefined)is preserved, allowing consuming code to handle missing metadata/balance gracefully.src/common/contracts/ref-finance/ref-exchange/client.ts (1)
1-10: LGTM!Clean migration to the new
contractApipattern. The module-levelrefExchangeContractApiinstance is appropriate for a single contract endpoint.src/common/contracts/sputnikdao2/client.ts (1)
6-20: LGTM!Correct approach for DAO contracts where
accountIdvaries per call, requiring dynamiccontractApiinstantiation. The migration maintains the existing API contract.src/common/wallet/components/providers.tsx (2)
79-101: Good defensive handling for walletSelector availability.The early return guard and proper event cleanup ensure safe behavior when
walletSelectoris temporarily unavailable during initialization.
12-13: Consistent defensive access pattern.The
undefinedcheck and optional chaining with fallback tofalseprovide robust handling during the wallet initialization lifecycle.Also applies to: 42-45
src/common/contracts/core/sybil-resistance/client.ts (1)
24-42: LGTM!The migration to
nearProtocolClient.contractApiis correct, and the view methods maintain proper type safety with the existing patterns. Based on learnings, using{}as a type parameter is acceptable for this codebase.src/common/contracts/metapool/liquid-staking/client.ts (1)
1-10: LGTM!Clean migration to the new
contractApifactory. The renamed exportliquidStakingContractApiappropriately distinguishes this contract instance from the generic factory import.src/common/contracts/social-db/client.ts (2)
126-143: LGTM on type improvements.Replacing
anywithRecord<string, unknown>forshared_storageandsetSocialDatadata parameter improves type safety.
108-124: No breaking change regardinguseCacheparameter.The
getSocialProfilefunction signature does not include auseCacheparameter in the current code, and the only caller in the codebase (src/common/contracts/social-db/hooks.ts:20) does not pass this parameter. No references touseCacheexist in the codebase related to this function.src/common/contracts/core/campaigns/client.ts (1)
35-87: LGTM on the batch transaction approach.The refactored
create_campaignfunction properly batches the social profile creation, list registration, and campaign creation into a single multi-transaction call. This is a cleaner approach than sequential calls. Based on learnings, error handling appropriately remains in consuming code.src/common/contracts/core/lists/client.ts (1)
2-18: LGTM! Clean migration to the new contract API pattern.The import aliasing (
contractApi as createContractApi) and local instance creation follow the established pattern seen in other contract clients (e.g.,pot/client.ts). This maintains consistency across the codebase while avoiding naming collisions.src/common/contracts/core/pot-factory/client.ts (2)
4-13: LGTM! Consistent migration pattern.The renaming to
potFactoryContractApiclearly distinguishes this module's contract API instance from the imported factory function, improving readability.
15-24: Verify: Error handling returnsundefinedfor deployment deposit calculation.The
.catchhandler logs the error and returnsundefined. While this aligns with the function's return typePromise<undefined | string>, be aware that callers must handle theundefinedcase—particularlydeploy_pot(line 32) which passes this directly todeposit. Ensure the wallet/contract handlesundefineddeposit gracefully or consider whether this should throw instead.#!/bin/bash # Check how deploy_pot handles undefined deposit from calculate_min_deployment_deposit rg -n -A5 'deploy_pot' --type=ts -g '!**/node_modules/**' | head -50src/common/contracts/core/pot/client.ts (2)
22-25: LGTM! Correct dynamic contract API pattern for pot contracts.Using
nearProtocolClient.contractApi({ contractId: potId })as a factory function is appropriate here since each pot has its own contract address. This differs from static contract clients (like lists or pot-factory) which use a single instance.
1-1: ThecalculateDepositByDataSizeimport is actively used and required.The utility from naxios is correctly used at lines 124, 142, and 163 for calculating deposits. Since
@wpdas/naxioscontinues to be imported and used throughout the codebase (in forms, effects, contracts-section, etc.), there's no indication that removing this dependency is a current goal. This import can remain as-is.src/features/donation/models/effects/direct-ft-donation.ts (1)
35-35: LGTM! Correct migration for dynamic FT token client.The change from
nearProtocolClient.naxiosInstance.contractApitonearProtocolClient.contractApialigns with the broader migration. The dynamictokenIdparameter appropriately uses the factory pattern.src/features/donation/models/effects/campaign-ft-donation.ts (1)
43-43: LGTM! Consistent migration pattern with direct-ft-donation.ts.The change mirrors the migration in
direct-ft-donation.ts, maintaining consistency across the donation effects modules.src/common/contracts/core/donation/client.ts (1)
13-15: LGTM - Clean refactor to new contractApi pattern.The migration from
naxiosInstance.contractApito the directcontractApiwith a local alias (donationContractApi) is clean and maintains the same contract interaction semantics.src/features/matching-pool-contribution/hooks/forms.ts (1)
8-8: LGTM - Consistent migration to contractApi.The migration correctly uses
contractApi({ contractId }).call()pattern matching the new wallet-selector approach. Both DAO proposal and direct donation paths are properly updated.Also applies to: 70-82
package.json (3)
89-89: Clarify naxios retention vs. PR description.The PR description states "Refactored the repository to remove naxios," but
@wpdas/naxiosis still present (upgraded from 2.2.3 to ^2.5.0). Please clarify whether naxios removal is planned for a follow-up PR or if the description should be updated to reflect that naxios is being upgraded rather than removed.
42-42: No action needed. The three packages at v8.10.2 (@near-wallet-selector/mintbase-wallet,@near-wallet-selector/nearfi, and@near-wallet-selector/neth) do not have v10-compatible releases. These packages are correctly pinned at their latest available versions and cannot be upgraded to match the v10.1.4 versions of other wallet-selector packages.Likely an incorrect or invalid review comment.
30-55: Verify compatibility between mixed wallet-selector versions (v8 and v10).While most packages upgraded to v10.1.4, three packages remain at v8.10.2 (mintbase-wallet, nearfi, neth) because v10 releases are not yet available for them. This creates a mixed dependency where both @near-wallet-selector/core@8.10.2 and @near-wallet-selector/core@10.1.4 are installed. Confirm that v10 breaking changes don't affect the v8 packages or cause runtime incompatibilities when both versions coexist.
| setupUnityWallet({ | ||
| projectId: "af5fcece6005cfe70a5d5132ab354e65", | ||
| metadata: { | ||
| name: "Potlock App", | ||
| description: "Bringing public goods funding to the table, built on NEAR", | ||
| url: "https://github.com/near/wallet-selector", | ||
| icons: ["https://avatars.githubusercontent.com/u/37784886"], | ||
| }, | ||
| }), |
There was a problem hiding this comment.
Unity wallet metadata URL points to wallet-selector repo instead of Potlock.
The url field in the metadata appears to be copied from example code and should likely point to the Potlock app URL.
📝 Suggested fix
setupUnityWallet({
projectId: "af5fcece6005cfe70a5d5132ab354e65",
metadata: {
name: "Potlock App",
description: "Bringing public goods funding to the table, built on NEAR",
- url: "https://github.com/near/wallet-selector",
+ url: "https://app.potlock.org",
icons: ["https://avatars.githubusercontent.com/u/37784886"],
},
}),📝 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.
| setupUnityWallet({ | |
| projectId: "af5fcece6005cfe70a5d5132ab354e65", | |
| metadata: { | |
| name: "Potlock App", | |
| description: "Bringing public goods funding to the table, built on NEAR", | |
| url: "https://github.com/near/wallet-selector", | |
| icons: ["https://avatars.githubusercontent.com/u/37784886"], | |
| }, | |
| }), | |
| setupUnityWallet({ | |
| projectId: "af5fcece6005cfe70a5d5132ab354e65", | |
| metadata: { | |
| name: "Potlock App", | |
| description: "Bringing public goods funding to the table, built on NEAR", | |
| url: "https://app.potlock.org", | |
| icons: ["https://avatars.githubusercontent.com/u/37784886"], | |
| }, | |
| }), |
🤖 Prompt for AI Agents
In @src/common/blockchains/near-protocol/client.ts around lines 55 - 63, The
metadata.url in the setupUnityWallet call is copied from the example and should
point to the Potlock app instead; update the metadata.url property in the
setupUnityWallet({...}) invocation (the metadata object) to the correct Potlock
application URL or repository URL (e.g., your live app URL or Potlock repo) so
the wallet metadata accurately represents the app.
| const ensureWallet = async () => { | ||
| await initNear(); | ||
|
|
||
| if (!state.walletSelector) { | ||
| throw new Error("Wallet selector is not initialized."); | ||
| } | ||
|
|
||
| await syncAccounts(); | ||
|
|
||
| if (!state.wallet) { | ||
| state.wallet = await state.walletSelector.wallet(); | ||
| } | ||
|
|
||
| return state.wallet!; | ||
| }; |
There was a problem hiding this comment.
Potential undefined wallet after selector.wallet() call.
If selector.wallet() resolves successfully but returns a falsy value (edge case), the non-null assertion on line 167 would be unsafe.
🔧 Suggested defensive check
const ensureWallet = async () => {
await initNear();
if (!state.walletSelector) {
throw new Error("Wallet selector is not initialized.");
}
await syncAccounts();
if (!state.wallet) {
state.wallet = await state.walletSelector.wallet();
}
+ if (!state.wallet) {
+ throw new Error("No wallet available.");
+ }
+
- return state.wallet!;
+ return state.wallet;
};📝 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 ensureWallet = async () => { | |
| await initNear(); | |
| if (!state.walletSelector) { | |
| throw new Error("Wallet selector is not initialized."); | |
| } | |
| await syncAccounts(); | |
| if (!state.wallet) { | |
| state.wallet = await state.walletSelector.wallet(); | |
| } | |
| return state.wallet!; | |
| }; | |
| const ensureWallet = async () => { | |
| await initNear(); | |
| if (!state.walletSelector) { | |
| throw new Error("Wallet selector is not initialized."); | |
| } | |
| await syncAccounts(); | |
| if (!state.wallet) { | |
| state.wallet = await state.walletSelector.wallet(); | |
| } | |
| if (!state.wallet) { | |
| throw new Error("No wallet available."); | |
| } | |
| return state.wallet; | |
| }; |
🤖 Prompt for AI Agents
In @src/common/blockchains/near-protocol/client.ts around lines 154 - 168, The
ensureWallet function may call state.walletSelector.wallet() and then assert
state.wallet! which is unsafe if the call returns a falsy value; update
ensureWallet to capture the result of await state.walletSelector.wallet(),
verify it is non-null/non-undefined before assigning to state.wallet, and throw
a clear error (or handle appropriately) if the returned wallet is falsy;
reference the ensureWallet function, state.walletSelector.wallet(), and
state.wallet so reviewers can locate and fix the check and assignment.
| export type ApplyToList = { | ||
| list_id: ListId; | ||
| notes?: null | string; | ||
| registrations: Array<{ | ||
| registrations: { | ||
| registrant_id: string; | ||
| status: string; | ||
| submitted_ms: number; | ||
| updated_ms: number; | ||
| notes: string; | ||
| }>; | ||
| } | ||
| }[]; | ||
| } & Record<string, unknown>; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the interfaces.ts file to confirm the code at lines 42-58
head -60 src/common/contracts/core/lists/interfaces.ts | tail -20Repository: PotLock/potlock-nextjs-app
Length of output: 475
🏁 Script executed:
# Search for how ApplyToList and UpdateRegistration are used
rg -n "ApplyToList|UpdateRegistration" --type=ts -C 2 -g '!**/interfaces.ts'Repository: PotLock/potlock-nextjs-app
Length of output: 4578
🏁 Script executed:
# Find RegistrationStatus enum definition
rg -n "enum RegistrationStatus|type RegistrationStatus" --type=tsRepository: PotLock/potlock-nextjs-app
Length of output: 280
🏁 Script executed:
# Search for Registration type to see how it defines status
rg -n "export type Registration|export interface Registration" --type=tsRepository: PotLock/potlock-nextjs-app
Length of output: 239
🏁 Script executed:
# Check how status is actually used in registration processing
rg -n "status" src/common/contracts/core/lists/client.ts -C 2Repository: PotLock/potlock-nextjs-app
Length of output: 833
🏁 Script executed:
# Get the full RegistrationStatus enum definition
sed -n '1,10p' src/common/contracts/core/lists/interfaces.tsRepository: PotLock/potlock-nextjs-app
Length of output: 268
🏁 Script executed:
# Get the Registration interface definition
sed -n '26,35p' src/common/contracts/core/lists/interfaces.tsRepository: PotLock/potlock-nextjs-app
Length of output: 325
🏁 Script executed:
# Check the ApplyToList usage context in the client to understand the data flow
sed -n '89,95p' src/common/contracts/core/lists/client.tsRepository: PotLock/potlock-nextjs-app
Length of output: 298
ApplyToList.registrations[].status should use RegistrationStatus enum instead of string.
The status field type is inconsistent across similar types:
ApplyToList.registrations[].statususesstring(line 47)UpdateRegistration.statususesRegistrationStatusenum (line 56)Registration.status(from contract data) usesRegistrationStatusenum
Since both ApplyToList and UpdateRegistration are inputs to contract calls, and the Registration interface already defines status as an enum, ApplyToList.registrations[].status should use RegistrationStatus enum for consistency and type safety. This prevents invalid status values from being passed to the contract.
🤖 Prompt for AI Agents
In @src/common/contracts/core/lists/interfaces.ts around lines 42 - 52, The
registrations[].status field in the ApplyToList type is currently typed as
string — change it to use the existing RegistrationStatus enum for consistency
and type safety; update the ApplyToList definition (specifically the
registrations property within ApplyToList) to reference RegistrationStatus
instead of string so it matches UpdateRegistration.status and
Registration.status across the contracts.
| export const nft_token = ({ contractAccountId, tokenId }: NonFungibleTokenLookupParams) => | ||
| nearProtocolClient.naxiosInstance | ||
| nearProtocolClient | ||
| .contractApi({ contractId: contractAccountId }) | ||
| .view<NftTokenArgs, NonFungibleToken>("nft_token", { args: { token_id: tokenId } }) | ||
| .catch(() => undefined); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the nearProtocolClient definition and contractApi interface
fd -e ts -e tsx 'index.ts' src/common/blockchains/near-protocol | head -5Repository: PotLock/potlock-nextjs-app
Length of output: 115
🏁 Script executed:
# Search for all .catch() patterns in contract client files to see if this is widespread
rg -n '\.catch\(' src/common/contracts/ -A1 -B1Repository: PotLock/potlock-nextjs-app
Length of output: 2257
🏁 Script executed:
# Check the nft_token and nft_metadata functions specifically
cat -n src/common/contracts/tokens/non-fungible/client.tsRepository: PotLock/potlock-nextjs-app
Length of output: 1231
🏁 Script executed:
# Check the nearProtocolClient and contractApi definition
cat -n src/common/blockchains/near-protocol/index.tsRepository: PotLock/potlock-nextjs-app
Length of output: 410
🏁 Script executed:
# Check the consuming code in hooks to see if error handling is there
cat -n src/common/contracts/tokens/non-fungible/hooks.ts | head -30Repository: PotLock/potlock-nextjs-app
Length of output: 1010
Remove .catch() from the client layer and let errors propagate to consuming code.
The refactoring from naxiosInstance.contractApi to nearProtocolClient.contractApi is technically sound and the interface remains compatible. However, the .catch(() => undefined) at lines 21 and 30 violates the error handling principle: error handling should be implemented in consuming code (like the hooks), not at the contract client level. The client layer should remain focused on making calls without suppressing errors. Since consuming code like src/common/contracts/tokens/non-fungible/hooks.ts already handles errors (line 21), the client-level catch handlers are redundant and should be removed.
🤖 Prompt for AI Agents
In @src/common/contracts/tokens/non-fungible/client.ts around lines 17 - 21, The
client currently swallows errors by appending .catch(() => undefined) to
contract calls; remove the .catch(...) from the nft_token function (and the
other similar call in this file) so that
nearProtocolClient.contractApi(...).view<NftTokenArgs,
NonFungibleToken>("nft_token", { args: { token_id: tokenId } }) can throw to
callers; leave the call signatures and return types unchanged and rely on
consuming code (e.g., hooks) to handle errors.
| import { type ComponentType } from "react"; | ||
|
|
||
| import { LazyLoadImage, type LazyLoadImageProps } from "react-lazy-load-image-component"; | ||
|
|
||
| // Wrapper to satisfy React 18 JSX element typing for LazyLoadImage | ||
| export const LazyImage = LazyLoadImage as unknown as ComponentType<LazyLoadImageProps>; | ||
|
|
||
| export type { LazyLoadImageProps } from "react-lazy-load-image-component"; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
react-lazy-load-image-component React 18 compatibility maintained
💡 Result:
Short answer: The package does not appear actively maintained and there’s no official, up-to-date guarantee of React 18 support. It presently installs and is used with React 18 by many projects, but there are open issues and no recent maintainer activity or releases explicitly addressing React 18. [1][2][3]
Sources:
- npm (latest release v1.6.3, published ~9 months ago). [1]
- GitHub issues showing active/open problems and little recent maintainer activity. [2]
- Snyk package page flags low maintenance / inactivity. [3]
If you need a stable, actively maintained option for React 18, consider built-in browser lazy-loading (loading="lazy"), react-lazyload (actively maintained fork/community), or framework-specific solutions (e.g., next/image). [4][5]
Reconsider using react-lazy-load-image-component — it's not actively maintained.
The library has no official React 18 support guarantee and shows little recent maintainer activity (last release ~9 months ago). The double-cast workaround here is a symptom of relying on an outdated, unmaintained dependency. Consider migrating to:
- Native browser lazy-loading (
loading="lazy") - Actively maintained alternatives (e.g., react-lazyload)
- Framework-specific solutions (e.g., next/image)
🤖 Prompt for AI Agents
In @src/common/ui/layout/components/LazyImage.tsx around lines 1 - 8, The
current LazyImage wrapper around LazyLoadImage (symbols: LazyImage,
LazyLoadImage, LazyLoadImageProps) relies on an unmaintained library and uses an
unsafe double-cast; replace this by removing the wrapper and migrating consumers
to a maintained approach: either use native browser lazy-loading by
returning/using a plain img with loading="lazy" and preserving
LazyLoadImageProps mappings, or switch to an actively maintained package (e.g.,
react-lazyload) and adapt consumers to that API (wrap images with the new
provider/component and adjust prop names). Update exports so LazyImage and
LazyLoadImageProps no longer reference the old package, and modify any
components importing LazyImage to use the chosen replacement API.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.