-
Notifications
You must be signed in to change notification settings - Fork 46
feat: deploy Noblocks as Farcaster Mini App using MiniKit #211
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: main
Are you sure you want to change the base?
Conversation
…m provider and improve error handling during connection attempts.
WalkthroughThis PR introduces comprehensive Farcaster Mini App integration alongside enhanced Base App support. It adds Farcaster manifest files, SDK initialization logic, a new BaseAppContext for environment detection, wallet priority handling across multiple components, expanded configuration for app metadata, and new OnchainKit/Wagmi dependencies for blockchain interactions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as App (Next.js)
participant EarlyReady
participant FarcasterSDK as Farcaster SDK
participant BaseAppContext
participant WalletContext
participant Provider as Provider Tree
User->>App: Load app in Farcaster mini app
App->>EarlyReady: Render at layout root (early)
EarlyReady->>FarcasterSDK: Call sdk.ready()
FarcasterSDK-->>EarlyReady: Signal ready or error
EarlyReady->>EarlyReady: Set __farcasterMiniAppReady flag
App->>BaseAppContext: Initialize provider
BaseAppContext->>FarcasterSDK: isInMiniApp(), getContext()
FarcasterSDK-->>BaseAppContext: Return environment info
BaseAppContext->>BaseAppContext: Detect isBaseApp, isFarcaster, get baseAppWallet
App->>WalletContext: Initialize InjectedWalletContext
WalletContext->>FarcasterSDK: Attempt sdk.wallet.getEthereumProvider()
alt Farcaster SDK Provider Available
FarcasterSDK-->>WalletContext: Return provider
else Fallback to window.ethereum
WalletContext->>WalletContext: Use window.ethereum
end
WalletContext->>WalletContext: Delay 100ms, request accounts
App->>Provider: Render wrapped components
Provider->>Provider: Wallet priority: BaseApp > Farcaster > Injected > Smart
Provider-->>User: App ready with active wallet
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
app/api/v1/transactions/[id]/route.ts (1)
22-24: Validate and sanitize request body; avoid writing undefined/null and enforce allowed status values.Currently txHash/timeSpent/status are unchecked and may upsert nulls/invalid states. Validate and only update defined fields (and trim strings). You can reuse withValidProperties.
Apply this diff:
import { NextRequest, NextResponse } from "next/server"; import { supabaseAdmin } from "@/app/lib/supabase"; import { withRateLimit } from "@/app/lib/rate-limit"; +import { withValidProperties } from "@/app/lib/manifest-utils"; @@ - const body = await request.json(); - const { txHash, timeSpent, status } = body; + const body = await request.json(); + const { txHash, timeSpent, status } = body as { + txHash?: string; + timeSpent?: number; + status?: "pending" | "confirmed" | "failed"; + }; + + // Basic validation + if (txHash && !/^0x[0-9a-fA-F]{64}$/.test(txHash)) { + return NextResponse.json({ success: false, error: "Invalid txHash" }, { status: 400 }); + } + if (typeof timeSpent !== "undefined" && (typeof timeSpent !== "number" || timeSpent < 0)) { + return NextResponse.json({ success: false, error: "Invalid timeSpent" }, { status: 400 }); + } + if (typeof status !== "undefined" && !["pending", "confirmed", "failed"].includes(status)) { + return NextResponse.json({ success: false, error: "Invalid status" }, { status: 400 }); + } + + const updates = withValidProperties({ + tx_hash: txHash?.trim(), + time_spent: timeSpent, + status, + updated_at: new Date().toISOString(), + }); @@ - const { data, error } = await supabaseAdmin + const { data, error } = await supabaseAdmin .from("transactions") - .update({ - tx_hash: txHash, - time_spent: timeSpent, - status: status, - updated_at: new Date().toISOString(), - }) + .update(updates) .eq("id", id) .eq("wallet_address", walletAddress) .select() .single();Also applies to: 41-53
app/utils.ts (3)
559-607: Farcaster Mini App detection wrongly gated on window.ethereum; tighten heuristics and allow SDK wallet.In many Farcaster mini-app contexts, a provider may exist via SDK/OnchainKit without populating window.ethereum. Current logic returns false in that case and may disable the injected flow. Also, the generic iframe check can cause false positives.
Refactor to:
- Detect any viable provider (window.ethereum OR SDK wallet).
- Remove the broad iframe heuristic.
- Accept more values for the injected flag.
Apply:
export function shouldUseInjectedWallet( searchParams: URLSearchParams, ): boolean { const injectedParam = searchParams.get("injected"); - const hasEthereum = typeof window !== "undefined" && (window as any).ethereum; + const ethereumProvider = + typeof window !== "undefined" ? (window as any).ethereum : undefined; // Detect if running inside Farcaster Mini App environment const isLikelyFarcasterMiniApp = (() => { if (typeof window === "undefined") return false; try { const w = window as any; const ua = navigator.userAgent || ""; - const ref = document.referrer || ""; - const url = window.location.href || ""; + const ref = document.referrer || ""; + const url = window.location.href || ""; - // More aggressive detection for Farcaster Mini App - return ( - Boolean(w.__farcasterMiniAppReady) || - Boolean(w.farcaster) || - Boolean(w.warpcast) || - Boolean(w.farcasterMiniApp) || - /Farcaster|Warpcast/i.test(ua) || - /warpcast\.com/i.test(ref) || - /warpcast\.com/i.test(url) || - /farcaster\.xyz/i.test(url) || - /farcaster\.xyz/i.test(ref) || - // Check if we're in an iframe or embedded context - window !== window.top || - // Check for Farcaster-specific query parameters - searchParams.has("farcaster") || - searchParams.has("warpcast") || - searchParams.has("miniapp") || - // Check for Farcaster SDK availability - (typeof w !== "undefined" && w.sdk && w.sdk.wallet) - ); + // Heuristics focused on Farcaster/Warpcast contexts and SDK presence + return ( + Boolean(w.__farcasterMiniAppReady) || + Boolean(w.farcaster) || + Boolean(w.warpcast) || + Boolean(w.farcasterMiniApp) || + /Farcaster|Warpcast/i.test(ua) || + /(warpcast\.com|farcaster\.xyz)/i.test(ref + url) || + searchParams.has("farcaster") || + searchParams.has("warpcast") || + searchParams.has("miniapp") || + (w?.sdk && w?.sdk?.wallet) + ); } catch { return false; } })(); - // Treat Farcaster Mini App as injected flow if a provider exists - if (isLikelyFarcasterMiniApp && hasEthereum) return true; + const hasFarcasterSdkWallet = + typeof window !== "undefined" && + !!(window as any)?.sdk && + !!(window as any)?.sdk?.wallet; + + // Treat Farcaster Mini App as injected flow if any provider exists + if (isLikelyFarcasterMiniApp && (ethereumProvider || hasFarcasterSdkWallet)) { + return true; + } - return Boolean(injectedParam === "true" && hasEthereum); + const injectedFlag = + (injectedParam || "").toLowerCase() === "true" || + injectedParam === "1" || + (injectedParam || "").toLowerCase() === "yes"; + return Boolean(injectedFlag && (ethereumProvider || hasFarcasterSdkWallet)); }
335-378: Ensure getNetworkTokens always returns an array and set cache timestamp at completion.The function promises Token[] but can return undefined on fallback. Also, lastTokenFetch should reflect completion time.
Apply:
- lastTokenFetch = now; + lastTokenFetch = Date.now(); ... - // Return fallback tokens if API fails - return FALLBACK_TOKENS[network]; + // Return fallback tokens if API fails + return FALLBACK_TOKENS[network] || [];
702-754: setSelectedNetwork/onSuccess fire before the switch completes; await the promise.UI can reflect a switched network even if wallet_switchEthereumChain fails.
Apply:
- try { - toast.promise( - (async () => { + try { + const switchPromise = (async () => { try { // First try to switch to the network await (window.ethereum as any).request({ method: "wallet_switchEthereumChain", params: [{ chainId }], }); } catch (switchError: any) { // This error code indicates that the chain has not been added to MetaMask. if (switchError.code === 4902) { await (window.ethereum as any).request({ method: "wallet_addEthereumChain", params: [getAddChainParameters(network)], }); } else { throw switchError; } } - })(), - { + })(); + await toast.promise(switchPromise, { loading: `Switching to ${network.chain.name}...`, success: `Successfully switched to ${network.chain.name}`, error: (err) => err.message, - }, - ); + }); + await switchPromise; setSelectedNetwork(network); onSuccess();app/types.ts (2)
249-280: Fix typos in Config type and update corresponding usages in config loader
- In app/types.ts, rename the misspelled keys:
appSubstitle→appSubtitleappSpashImage→appSplashImagefarcasterSignatuure→farcasterSignature- In app/lib/config.ts (around lines 19–27), update the same three property names to match the type:
appSubstitle→appSubtitleappSpashImage→appSplashImagefarcasterSignatuure→farcasterSignature- Run
to confirm no remaining references.rg -nP 'appSubstitle|appSpashImage|farcasterSignatuure'
249-280: Remove Supabase service_role key from the client bundlesupabaseRoleKey in app/lib/config.ts (process.env.SUPABASE_SERVICE_ROLE_KEY) is exported to the frontend — the service_role key bypasses RLS and grants full database access. Replace with the anon/public key (NEXT_PUBLIC_SUPABASE_ANON_KEY) in client code, and load the service_role key only in server‐only contexts (API routes or server components). Split into PublicConfig (client) and ServerConfig (backend) and update app/lib/config.ts accordingly.
🧹 Nitpick comments (30)
vercel.json (1)
4-6: Extract hosted-manifest ID to an environment variable for redirects
Move this redirect intonext.config.js(or handle it in an app route) so the destination comes fromprocess.envand you avoid code changes whenever the manifest ID rotates. Note that invercel.json,"permanent": falsealready issues an HTTP 307 redirect, so you don’t need a separatestatusCode.app/api/v1/transactions/[id]/route.ts (1)
55-66: Surface domain errors clearly to clients (optional).Optionally map Supabase errors (e.g., unique violation on tx_hash) to 409/422 instead of generic 500 for better client UX.
package.json (1)
16-25: Add a type-check script to CI.Helpful for catching SDK integration regressions.
"scripts": { "dev": "next dev --turbopack", "build": "next build", "start": "next start", "lint": "next lint", + "typecheck": "tsc --noEmit", "sanity:dev": "sanity dev --studio studio",app/api/og/route.tsx (2)
3-4: Run on the Edge and add caching headers for the OG image.Edge runtime and cache headers reduce cold starts and recompute.
export async function GET() { - return new ImageResponse( + const res = new ImageResponse( @@ - { - width: 1200, - height: 630, - }, + { width: 1200, height: 630 }, ); + res.headers.set("Cache-Control", "public, s-maxage=86400, stale-while-revalidate=604800"); + return res; } + +export const runtime = "edge";
20-44: Consider pulling copy from config to avoid divergence.If you already added appOgTitle/appOgDescription, render them here to keep OG consistent with metadata.
app/early-ready.tsx (2)
12-13: Guard SDK call to avoid throwing outside Farcaster.Be defensive in non-miniapp contexts and older SDKs.
- // Notify Farcaster SDK - sdk.actions.ready(); + // Notify Farcaster SDK + try { + // @ts-expect-error: older SDKs may not type optional chaining + sdk?.actions?.ready?.(); + } catch { /* no-op */ }
23-25: Limit postMessage target if you can infer origin (optional).Using "*" is fine for readiness pings, but if the SDK exposes the parent origin, prefer that for stricter security.
app/lib/manifest-utils.ts (1)
1-22: Add basic unit tests for edge cases (empty strings, arrays with blanks, false booleans).This helper is easy to regress; a small test will lock behavior.
public/farcaster-manifest.json (3)
12-18: Parametrize asset URLs (icon/splash) to avoid drift across environments.Tie these to config so staging/production assets don’t get out of sync with the app.url, and to keep a single source of truth with the hosted manifest endpoint you added.
- "icon": { - "light": "https://noblocks-ialc.vercel.app/icons/android-chrome-192x192.png", - "dark": "https://noblocks-ialc.vercel.app/icons/android-chrome-192x192.png" - }, + "icon": { + "light": "<from config>", + "dark": "<from config>" + }, - "splash_screen": { - "light": "https://noblocks-ialc.vercel.app/screenshots/desktop-wide.png", - "dark": "https://noblocks-ialc.vercel.app/screenshots/desktop-wide.png" - }, + "splash_screen": { + "light": "<from config>", + "dark": "<from config>" + },
23-24: Confirm versioning strategy.If this is the first release of the mini app,
10.0.0looks accidental. Consider aligning manifest version with your app/package versioning.
30-34: Ensure domain association triple matches the deployed domain.Verify that:
- payload.domain equals the exact host users load (no www vs apex mismatch),
- signature matches the header/payload, and
- header key is the intended signer.
If you’ll rotate these, prefer pulling from env at build time to avoid frequent commits.
Would you like a small server route that emits this manifest from env and validates base64 structure before serving?
next.config.mjs (3)
23-27: Constrain frame-ancestors to the minimal set and consider reporting.The CSP is correct in principle. Consider:
- Keeping only the domains truly needed (least privilege).
- Adding a report-to/report-uri for visibility on violations.
- value: - "frame-ancestors 'self' https://warpcast.com https://*.warpcast.com https://*.farcaster.xyz https://farcaster.xyz https://auth.privy.io https://client.warpcast.com;", + value: + "frame-ancestors 'self' https://warpcast.com https://*.warpcast.com https://*.farcaster.xyz; report-to=csp-endpoint;",
39-49: Service worker headers look fine; consider text/javascript for broadest compatibility.application/javascript usually works, but text/javascript is the de-facto for SW scripts.
- value: "application/javascript; charset=utf-8", + value: "text/javascript; charset=utf-8",
29-35: X-XSS-Protection is obsolete.Modern browsers ignore it. Safe to remove to reduce header noise.
- { - key: "X-XSS-Protection", - value: "1; mode=block", - },app/context/InjectedWalletContext.tsx (6)
14-14: Lazy-load the Farcaster SDK to avoid bundling it for non-miniapp users.Dynamically import the SDK only when you’ve determined you should use the injected wallet. This reduces bundle size and avoids initializing the SDK unnecessarily.
-import { sdk } from "@farcaster/miniapp-sdk";+ // Inside the shouldUse branch: + const { sdk } = await import("@farcaster/miniapp-sdk");
44-62: Provider selection fallback is solid. Minor: add a timeout around SDK provider fetch.If
sdk.wallet.getEthereumProvider()hangs, you’ll stall init. Wrap it with a short timeout and fall back to window.ethereum deterministically.- const farcasterProvider = await sdk.wallet.getEthereumProvider(); + const farcasterProvider = await Promise.race([ + sdk.wallet.getEthereumProvider(), + new Promise<null>((resolve) => setTimeout(() => resolve(null), 800)), + ]);
71-77: Avoid arbitrary sleeps; prefer readiness signals.Instead of a fixed 100ms delay, await an explicit provider-ready event if available (or the timeout-wrapped getEthereumProvider above). Fixed sleeps can be flaky under load.
74-83: Type the provider and pass the client back to context.Use EIP1193Provider typing and consider exposing the viem client in context for downstream code.
-import { createWalletClient, custom } from "viem"; +import { createWalletClient, custom, type EIP1193Provider } from "viem"; @@ - const [injectedProvider, setInjectedProvider] = useState<any | null>(null); + const [injectedProvider, setInjectedProvider] = useState<EIP1193Provider | null>(null); + const [client, setClient] = useState<ReturnType<typeof createWalletClient> | null>(null); @@ - client = createWalletClient({ + client = createWalletClient({ transport: custom(ethereumProvider as any), }); + setClient(client);
79-90: Listen for provider events to keep state in sync.Handle accountsChanged, chainChanged, and disconnect, and clean up listeners on unmount.
+ const onAccountsChanged = (accounts: string[]) => { + setInjectedAddress(accounts?.[0] ?? null); + setInjectedReady(Boolean(accounts?.length)); + }; + const onDisconnect = () => { + setInjectedReady(false); + setInjectedAddress(null); + }; + (ethereumProvider as any).on?.("accountsChanged", onAccountsChanged); + (ethereumProvider as any).on?.("disconnect", onDisconnect); + // In cleanup (use the cancelled guard block above), also: + // (ethereumProvider as any).removeListener?.("accountsChanged", onAccountsChanged); + // (ethereumProvider as any).removeListener?.("disconnect", onDisconnect);
94-108: Error handling duplication.You already handle code 4001; the string “User rejected” branch largely duplicates behavior. Consider collapsing into a single branch keyed by 4001 or by a helper predicate.
app/utils.ts (1)
397-409: Avoid precision/overflow by using formatUnits instead of Number(BigInt).Apply:
- const balanceInWei = await client.readContract({ + const balanceInWei = await client.readContract({ address: token.address as `0x${string}`, abi: erc20Abi, functionName: "balanceOf", args: [address as `0x${string}`], }); - const balance = Number(balanceInWei) / Math.pow(10, token.decimals); + const balance = parseFloat(formatUnits(balanceInWei as bigint, token.decimals));And update imports:
- import { erc20Abi } from "viem"; + import { erc20Abi, formatUnits } from "viem";app/types.ts (1)
249-280: Stronger typing for env-like fields.nodeEnv and noIndex are strings; consider:
- nodeEnv: "development" | "production" | "test"
- noIndex: boolean
Example:
- nodeEnv: string; - noIndex: string; + nodeEnv: "development" | "production" | "test"; + noIndex: boolean;Gate conversion in app/lib/config.ts when reading process.env.
app/head.tsx (2)
6-11: Guard against empty appUrl and consider adding SEO/OG tags from config.If appUrl is empty, replace() will throw. Also, you already have OG fields in config; adding them here improves share previews.
Apply:
- const baseUrl = appUrl.replace(/\/$/, ""); + const safeUrl = appUrl || "https://noblocks.xyz"; + const baseUrl = safeUrl.replace(/\/$/, "");Optionally add:
+ <meta property="og:title" content={config.appOgTitle} /> + <meta property="og:description" content={config.appOgDescription} /> + <meta property="og:image" content={config.publicAppOGImage} /> + <meta name="twitter:card" content="summary_large_image" />
20-24: Remove legacy Frame meta tag
fc:frame:button:1:target is part of the old Frame spec and isn’t required for Mini Apps; Mini Apps use only the fc:miniapp embed meta (containing version, imageUrl, and button.title + button.action) (miniapps.farcaster.xyz, docs.farcaster.xyz)providers/MiniKitProvider.tsx (2)
7-23: Prefer official chain constants over hand-rolled objects.Import the Base chain from viem to reduce drift and maintenance.
Apply this diff:
+import { base } from "viem/chains"; -const baseChain = { - id: 8453, - name: "Base Mainnet", - network: "base", - nativeCurrency: { name: "ETH", symbol: "ETH", decimals: 18 }, - rpcUrls: { - default: { http: ["https://mainnet.base.org"] }, - public: { http: ["https://mainnet.base.org"] }, - }, - blockExplorers: { - default: { name: "BaseScan", url: "https://basescan.org" }, - baseScan: { name: "BaseScan", url: "https://basescan.org" }, - }, - contracts: {}, - testnet: false, - blockTime: 12, -}; +// use viem's canonical Base chain config- <MiniKitProvider apiKey={cdpApiKey} chain={baseChain}> + <MiniKitProvider apiKey={cdpApiKey} chain={base}>Also applies to: 38-38
28-35: Strengthen the missing-key path.Consider surfacing an explicit error in production (e.g., console.error or report to telemetry) so silent degradation doesn’t mask misconfigurations.
if (!cdpApiKey) { - if (nodeEnv !== "production") { - console.warn( - "MiniKitContextProvider: NEXT_PUBLIC_CDP_API_KEY is not set. Rendering without MiniKitProvider.", - ); - } + const msg = + "MiniKitContextProvider: NEXT_PUBLIC_CDP_API_KEY is not set. Rendering without MiniKitProvider."; + nodeEnv !== "production" ? console.warn(msg) : console.error(msg); return <>{children}</>; }If you expect MiniKit features in production, please confirm that NEXT_PUBLIC_CDP_API_KEY is set in Vercel env for the production project.
app/lib/supabase.ts (1)
6-16: Optional: expose a getter that throws with a clear message when misconfigured.Avoid passing null around; fail fast where used.
export function getSupabaseAdmin(): SupabaseClient { if (!supabaseAdmin) { throw new Error("Supabase admin client is not configured (supabaseUrl/supabaseRoleKey missing)."); } return supabaseAdmin; }app/lib/config.ts (3)
20-20: Give publicUrl a sensible default to avoid empty strings.Align with appUrl’s behavior.
- publicUrl: process.env.NEXT_PUBLIC_URL || "", + publicUrl: process.env.NEXT_PUBLIC_URL || "https://noblocks.xyz",
45-45: Treat noIndex as a boolean, not a string.Prevents truthy string bugs in meta/robots logic.
- noIndex: process.env.NEXT_PUBLIC_NOINDEX || "", + noIndex: process.env.NEXT_PUBLIC_NOINDEX === "true",Ensure the Config type reflects boolean.
14-14: Minor: appUrl/publicUrl duplication.You’re sourcing both from NEXT_PUBLIC_URL; consider documenting their intended use or consolidating to reduce confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
app/api/og/route.tsx(1 hunks)app/api/v1/transactions/[id]/route.ts(1 hunks)app/context/InjectedWalletContext.tsx(3 hunks)app/early-ready.tsx(1 hunks)app/head.tsx(1 hunks)app/layout.tsx(3 hunks)app/lib/config.ts(1 hunks)app/lib/manifest-utils.ts(1 hunks)app/lib/supabase.ts(1 hunks)app/types.ts(2 hunks)app/utils.ts(2 hunks)next.config.mjs(2 hunks)package.json(3 hunks)providers/MiniKitProvider.tsx(1 hunks)public/.well-known/farcaster.json(1 hunks)public/.well-known/route.ts(1 hunks)public/farcaster-manifest.json(1 hunks)vercel.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-23T07:30:23.720Z
Learnt from: jeremy0x
PR: paycrest/noblocks#177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:30:23.720Z
Learning: The `normalizeNetworkName` function in app/utils.ts has been updated to be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces and proper handling of acronyms like "BNB".
Applied to files:
app/utils.ts
🧬 Code graph analysis (2)
app/layout.tsx (2)
app/early-ready.tsx (1)
EarlyReady(5-34)providers/MiniKitProvider.tsx (1)
MiniKitContextProvider(25-42)
app/lib/supabase.ts (1)
app/lib/sanity-client.ts (1)
client(7-10)
🔇 Additional comments (6)
app/api/v1/transactions/[id]/route.ts (2)
9-9: LGTM: fix to sync destructure of params.This corrects the unnecessary await.
11-21: Normalize wallet addresses consistently at write-time and query-time.You lower-case the header value; ensure wallet_address is stored lower-cased in DB (or use ilike) to avoid mismatches. Add a unit/integration test covering mixed-case inputs.
package.json (1)
27-28: Verify peerDependency warnings on install
Runpnpm install --reporter=append-onlyand confirm there are no peer warnings for Next.js 15, React 19, wagmi/@wagmi/core, @wagmi/chains, viem, @coinbase/onchainkit or @farcaster/miniapp-sdk. Applies to lines 27–28, 43–45 and 71–73.public/.well-known/farcaster.json (1)
30-34: Ensure accountAssociation domain & signature consistency
File:public/.well-known/farcaster.json(lines 30–34)
Decoded payload domain:noblocks-ialc.vercel.app. Confirm this matches your live deployment domain—if you’ve switched domains, update the payload accordingly and regenerate its signature.app/layout.tsx (2)
17-18: Imports for EarlyReady and MiniKit provider look good.No issues. These are correctly isolated client components used from a server layout.
247-268: Provider composition and EarlyReady placement LGTM.Early-ready signal is emitted early in the body, and MiniKit wraps Providers without altering layout order. Looks correct for in-app launch.
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: 6
♻️ Duplicate comments (3)
app/context/InjectedWalletContext.tsx (1)
79-90: Guard state updates if unmounted; avoid setState after unmountUse the existing cancelled flag to gate updates inside the async flow.
- if (address) { - setInjectedProvider(ethereumProvider); - setInjectedAddress(address); - setInjectedReady(true); - console.log("Successfully connected to wallet:", address); - } else { - console.warn("No address returned from injected wallet."); - toast.error( - "Couldn't connect to your wallet. Please check your wallet connection.", - ); - setIsInjectedWallet(false); - } + if (cancelled) return; + if (address) { + setInjectedProvider(ethereumProvider); + setInjectedAddress(address); + setInjectedReady(true); + console.log("Successfully connected to wallet:", address); + } else { + console.warn("No address returned from injected wallet."); + toast.error("Couldn't connect to your wallet. Please check your wallet connection."); + setIsInjectedWallet(false); + }app/lib/config.ts (1)
81-81: Good move: supabaseRoleKey isolated to serverConfig.This addresses the earlier concern about leaking the service role key via shared config.
app/lib/supabase.ts (1)
1-3: Mark server-only and add typing to avoid accidental client import and any.Prevents bundling in client and improves type safety.
-import { createClient } from "@supabase/supabase-js"; +import "server-only"; +import { createClient, type SupabaseClient } from "@supabase/supabase-js"; import config from "./config"; import { serverConfig } from "./config"; @@ -let client: any = null; +let client: SupabaseClient | null = null; @@ -export const supabaseAdmin: any = client; +export const supabaseAdmin: SupabaseClient | null = client;Also applies to: 10-10, 20-20
🧹 Nitpick comments (6)
app/context/InjectedWalletContext.tsx (2)
67-69: Simplify client creation; avoid unused mutable var- let client; - ... - client = createWalletClient({ + const client = createWalletClient({ transport: custom(ethereumProvider as any), });Also applies to: 47-48
44-66: Optional: type the provider and reduce console noise in productionType ethereumProvider as EIP1193Provider from viem and gate console logs behind NODE_ENV !== "production".
Want a quick diff to add the type import and a tiny logger?
app/types.ts (2)
250-251: Tighten types for env-driven fields (prevents misuse).Prefer precise types for better safety.
- nodeEnv: string; - noIndex: string; + nodeEnv: "development" | "test" | "production"; + noIndex: boolean;Companion change suggested in app/lib/config.ts Line 44 to parse the env into a boolean.
278-279: Align cdpApiKey optionality with actual config usage.config always supplies a string (possibly empty). Make it required to simplify consumers.
- cdpApiKey?: string; + cdpApiKey: string;app/lib/config.ts (2)
19-19: Use a consistent fallback for publicUrl (matches appUrl).Avoid empty string; mirror appUrl’s default.
- publicUrl: process.env.NEXT_PUBLIC_URL || "", + publicUrl: process.env.NEXT_PUBLIC_URL || "https://noblocks.xyz",
44-45: Parse NOINDEX env to boolean (and align type).If you adopt the boolean type in app/types.ts, parse here.
- noIndex: process.env.NEXT_PUBLIC_NOINDEX || "", + noIndex: /^(1|true|yes)$/i.test(process.env.NEXT_PUBLIC_NOINDEX || ""),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/.well-known/farcaster.json(1 hunks)app/.well-known/route.ts(1 hunks)app/context/InjectedWalletContext.tsx(3 hunks)app/layout.tsx(3 hunks)app/lib/config.ts(2 hunks)app/lib/manifest-utils.ts(1 hunks)app/lib/supabase.ts(1 hunks)app/types.ts(2 hunks)public/farcaster-manifest.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/lib/manifest-utils.ts
- public/farcaster-manifest.json
- app/layout.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/lib/supabase.ts (2)
app/lib/config.ts (1)
serverConfig(76-82)app/lib/sanity-client.ts (1)
client(7-10)
🔇 Additional comments (3)
app/.well-known/farcaster.json (2)
12-18: Icon vs splash_screen paths inconsistent; likely 404 riskThe icon points to /icons/... while splash_screen points to root /. Ensure both URLs exist and meet recommended sizes.
- "light": "https://noblocks.xyz/android-chrome-192x192.png", - "dark": "https://noblocks.xyz/android-chrome-192x192.png" + "light": "https://noblocks.xyz/icons/android-chrome-192x192.png", + "dark": "https://noblocks.xyz/icons/android-chrome-192x192.png"
23-26: Manifest schema fields—confirm spec compliance and asset sizesprimaryCategory/tags vs categories have changed across spec versions; also verify required icon/splash dimensions.
If you want, I’ll add a small checker that validates keys and image dimensions via HEAD/Content-Type/Size.
app/context/InjectedWalletContext.tsx (1)
121-128: Nice: timeout + cancellation cleanup implemented
app/.well-known/route.ts
Outdated
| export async function GET() { | ||
| const hostedManifestUrl = process.env.NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL!; | ||
| return new Response(null, { | ||
| status: 307, | ||
| headers: { | ||
| Location: hostedManifestUrl, | ||
| "Cache-Control": "public, s-maxage=300, stale-while-revalidate=86400", | ||
| }, | ||
| }); | ||
| } |
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
Handle missing/invalid hosted manifest URL; improve caching
Non-null assertion on env can yield an invalid Location at runtime. Add a guard and validate as a URL. Also add max-age for browsers (s-maxage applies to CDNs only).
export async function GET() {
- const hostedManifestUrl = process.env.NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL!;
- return new Response(null, {
+ const hostedManifestUrl = process.env.NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL;
+ if (!hostedManifestUrl) {
+ return new Response("Missing NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL", { status: 500 });
+ }
+ try {
+ new URL(hostedManifestUrl);
+ } catch {
+ return new Response("Invalid NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL", { status: 500 });
+ }
+ return new Response(null, {
status: 307,
headers: {
Location: hostedManifestUrl,
- "Cache-Control": "public, s-maxage=300, stale-while-revalidate=86400",
+ "Cache-Control": "public, max-age=300, s-maxage=300, stale-while-revalidate=86400",
},
});
}
+
+export async function HEAD() {
+ return GET();
+}📝 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 async function GET() { | |
| const hostedManifestUrl = process.env.NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL!; | |
| return new Response(null, { | |
| status: 307, | |
| headers: { | |
| Location: hostedManifestUrl, | |
| "Cache-Control": "public, s-maxage=300, stale-while-revalidate=86400", | |
| }, | |
| }); | |
| } | |
| export async function GET() { | |
| const hostedManifestUrl = process.env.NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL; | |
| if (!hostedManifestUrl) { | |
| return new Response("Missing NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL", { status: 500 }); | |
| } | |
| try { | |
| new URL(hostedManifestUrl); | |
| } catch { | |
| return new Response("Invalid NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL", { status: 500 }); | |
| } | |
| return new Response(null, { | |
| status: 307, | |
| headers: { | |
| Location: hostedManifestUrl, | |
| "Cache-Control": "public, max-age=300, s-maxage=300, stale-while-revalidate=86400", | |
| }, | |
| }); | |
| } | |
| export async function HEAD() { | |
| return GET(); | |
| } |
🤖 Prompt for AI Agents
In app/.well-known/route.ts around lines 1–10, remove the non-null assertion and
add a guard that checks process.env.NEXT_PUBLIC_FC_HOSTED_MANIFEST_URL exists
and is a valid URL (e.g., try new URL(value) and catch errors); if
missing/invalid, return a 400 (or 500) Response with a clear message instead of
performing the redirect; when valid, use the parsed URL.href as the Location
header and update Cache-Control to include a browser-facing max-age as well as
s-maxage and stale-while-revalidate (for example: "public, max-age=60,
s-maxage=300, stale-while-revalidate=86400").
💡 Verification agent
🧩 Analysis chain
Route path doesn’t intercept /.well-known/farcaster.json
This route handles /.well-known, not /.well-known/farcaster.json. If you intend to redirect the JSON path via code (instead of vercel.json), move this file under app/.well-known/farcaster.json/route.ts. Otherwise, delete this route and rely on the Vercel redirect only.
I can script a curl check to confirm the live behavior for both /.well-known and /.well-known/farcaster.json.
🏁 Script executed:
#!/bin/bash
# Locate vercel.json
echo "=== vercel.json path ==="
fd -t f vercel.json || echo "vercel.json not found"
# Show content of vercel.json if exists
if [ -f vercel.json ]; then
echo "=== vercel.json content ==="
sed -n '1,200p' vercel.json
fi
# Locate next.config.js or next.config.ts
echo "=== next.config.js or ts ==="
fd -t f next.config.js next.config.ts || echo "next.config.js/ts not found"
# Show redirect config in next.config.js
if [ -f next.config.js ]; then
echo "=== next.config.js redirects ==="
grep -R "redirects" -n next.config.js | sed 's/^/ /'
fi
# Show redirect config in next.config.ts
if [ -f next.config.ts ]; then
echo "=== next.config.ts redirects ==="
grep -R "redirects" -n next.config.ts | sed 's/^/ /'
fi
# Show app/.well-known/route.ts content
echo "=== app/.well-known/route.ts content ==="
if [ -f app/.well-known/route.ts ]; then
sed -n '1,200p' app/.well-known/route.ts
else
echo "app/.well-known/route.ts not found"
fiLength of output: 781
Route file only matches /.well-known, not /.well-known/farcaster.json
Move app/.well-known/route.ts to app/.well-known/farcaster.json/route.ts if you intend to handle the JSON redirect in code; otherwise delete this route and rely on the existing vercel.json redirect for the .well-known/farcaster.json path.
🤖 Prompt for AI Agents
In app/.well-known/route.ts around lines 1 to 10, the route currently only
matches /.well-known; either move this file to
app/.well-known/farcaster.json/route.ts so it handles requests to
/.well-known/farcaster.json (keeping the same GET handler) or delete this route
file entirely and rely on the existing vercel.json redirect for the
/.well-known/farcaster.json path; after moving, ensure the file path and exports
remain the same and update any imports or references if necessary.
| } else if ((error as any)?.message?.includes("User rejected")) { | ||
| toast.error("Wallet connection was cancelled by user.", { | ||
| description: "You can try connecting again later.", | ||
| }); | ||
| setIsInjectedWallet(false); | ||
| } else { | ||
| toast.error( | ||
| "Failed to connect to wallet. Please refresh and try again.", | ||
| { | ||
| description: "If the problem persists, try restarting the app.", | ||
| }, | ||
| ); | ||
| setIsInjectedWallet(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
Reset all injected state on any failure path
The “User rejected” and generic error branches leave stale provider/address/ready.
- } else if ((error as any)?.message?.includes("User rejected")) {
- toast.error("Wallet connection was cancelled by user.", {
- description: "You can try connecting again later.",
- });
- setIsInjectedWallet(false);
+ } else if ((error as any)?.message?.includes("User rejected")) {
+ toast.error("Wallet connection was cancelled by user.", {
+ description: "You can try connecting again later.",
+ });
+ setIsInjectedWallet(false);
+ setInjectedProvider(null);
+ setInjectedAddress(null);
+ setInjectedReady(false);
} else {
toast.error(
"Failed to connect to wallet. Please refresh and try again.",
{
description: "If the problem persists, try restarting the app.",
},
);
- setIsInjectedWallet(false);
+ setIsInjectedWallet(false);
+ setInjectedProvider(null);
+ setInjectedAddress(null);
+ setInjectedReady(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.
| } else if ((error as any)?.message?.includes("User rejected")) { | |
| toast.error("Wallet connection was cancelled by user.", { | |
| description: "You can try connecting again later.", | |
| }); | |
| setIsInjectedWallet(false); | |
| } else { | |
| toast.error( | |
| "Failed to connect to wallet. Please refresh and try again.", | |
| { | |
| description: "If the problem persists, try restarting the app.", | |
| }, | |
| ); | |
| setIsInjectedWallet(false); | |
| } | |
| } else if ((error as any)?.message?.includes("User rejected")) { | |
| toast.error("Wallet connection was cancelled by user.", { | |
| description: "You can try connecting again later.", | |
| }); | |
| setIsInjectedWallet(false); | |
| setInjectedProvider(null); | |
| setInjectedAddress(null); | |
| setInjectedReady(false); | |
| } else { | |
| toast.error( | |
| "Failed to connect to wallet. Please refresh and try again.", | |
| { | |
| description: "If the problem persists, try restarting the app.", | |
| }, | |
| ); | |
| setIsInjectedWallet(false); | |
| setInjectedProvider(null); | |
| setInjectedAddress(null); | |
| setInjectedReady(false); | |
| } |
🤖 Prompt for AI Agents
In app/context/InjectedWalletContext.tsx around lines 103 to 116, the error
branches for "User rejected" and the generic failure currently only
setIsInjectedWallet(false) and leave stale provider/address/ready state; update
both failure branches to fully reset all injected-wallet state (e.g., set
provider to null/undefined, clear connected address, mark ready/connected flags
false, and setIsInjectedWallet(false)) — or call an existing resetInjectedState
helper that does that — so no provider/address/ready values remain after any
failure path.
…m provider and improve error handling during connection attempts.
…ns and visibility based on mini mode. Updated aria attributes for accessibility and improved rendering logic for wallet and network details.
…ng mini mode condition for wallet connection button.
….json to reflect new branding for Paycrest. Fix URLs in config.ts to remove duplicate protocol prefixes.
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: 1
♻️ Duplicate comments (4)
next.config.mjs (1)
20-27: Align framing headers with modern CSP usage and keep origins maintainableYou’re correctly relying on
Content-Security-Policy’sframe-ancestorsto allow embedding from Warpcast/Farcaster and related origins, which matches the mini‑app use case. However,X-Frame-Options: ALLOWALLis non‑standard and effectively redundant when CSPframe-ancestorsis present; it can be confusing for future readers.Consider:
- Dropping the
X-Frame-Optionsheader entirely and lettingframe-ancestorsbe the single source of truth.- Optionally factoring the allowed frame origins into a small helper/constant or env-driven list so it’s easier to update if Farcaster adds more domains.
This is non-blocking and mostly about clarity and long‑term maintainability.
app/blog/[id]/page.tsx (1)
28-35: Alignparams/searchParamstypes and mini-mode redirect with App Router conventionsIn the App Router,
paramsandsearchParamsare passed in as plain objects, not Promises. Typing them as Promises and awaiting them works at runtime (sinceawaiton a non-Promise just returns the value) but fights the canonical Next.js types and makes the API harder to work with.You can simplify both the types and the control flow here (and in
generateMetadata) by treating them as plain objects and makingsearchParamsoptional:-export async function generateMetadata({ - params, -}: { - params: Promise<{ id: string }>; -}): Promise<Metadata> { - const { id } = await params; +export async function generateMetadata({ + params, +}: { + params: { id: string }; +}): Promise<Metadata> { + const { id } = params; const post = await getPost(id); if (!post) return {}; // ... } export default async function BlogPostDetailPage({ params, searchParams, }: { - params: Promise<{ id: string }>; - searchParams: Promise<{ mini?: string }>; + params: { id: string }; + searchParams?: { mini?: string }; }) { - // Redirect to home page if in mini mode - const resolvedSearchParams = await searchParams; - if (resolvedSearchParams.mini === "true") { - redirect("/?mini=true"); - } - - const { id } = await params; + // Redirect to home page if in mini mode + if (searchParams?.mini === "true") { + redirect("/?mini=true"); + } + + const { id } = params; const post = await getPost(id); if (!post) notFound(); const recent = await getRecentPosts(post.slug.current); return <DetailClient post={post} recent={recent} />; }This keeps the behavior you want (redirecting mini-mode blog detail requests back to
/?mini=true) while:
- Matching Next.js’ canonical App Router types for
params/searchParams.- Removing unnecessary
awaits and an intermediateresolvedSearchParamsvariable.- Making the code safer if
searchParamsis ever omitted or undefined.Also, just to confirm product intent: in mini mode, this will always bounce users from an individual blog post back to the home mini view. If you instead want to support reading posts inside mini mode, we’d need a different handling strategy (e.g., adapting the layout instead of redirecting). Happy to help sketch that if needed.
Also applies to: 110-125
app/components/Navbar.tsx (1)
135-144: LGTM! Dropdown toggle properly gated in mini mode.The button's ARIA attributes and click handler correctly reflect the disabled state in mini mode, and the dropdown arrow is appropriately hidden.
Note: A past review comment suggests additional accessibility improvements (adding
disabledandaria-disabledattributes, functional state updates, and properidon the dropdown element). Consider implementing those enhancements for better accessibility.Also applies to: 160-172
app/layout.tsx (1)
40-44: Critical: Missing import forconfigobject.Lines 41 and 44 reference
config.publicUrl, but theconfigobject is not imported. This will cause a runtime error.Add the missing import at the top of the file:
+import { config } from "./config"; import "./globals.css"; import React from "react";Additionally, past review comments have already flagged concerns about:
- Using hardcoded preview URLs instead of config-driven values
- Potential duplicate Farcaster frame metadata across files
Please address those comments as well.
🧹 Nitpick comments (1)
app/types.ts (1)
250-250: Consider using a union type fornodeEnvfor better type safety.The
nodeEnvfield is currently typed asstring, but it typically has a fixed set of values. Using a union type would provide better type safety and IDE autocomplete support.Apply this diff to improve type safety:
- nodeEnv: string; + nodeEnv: "development" | "production" | "test";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
app/blog/[id]/page.tsx(2 hunks)app/blog/page.tsx(2 hunks)app/components/HomePage.tsx(4 hunks)app/components/MainPageContent.tsx(4 hunks)app/components/Navbar.tsx(8 hunks)app/layout.tsx(3 hunks)app/lib/config.ts(2 hunks)app/types.ts(1 hunks)app/utils.ts(2 hunks)next.config.mjs(3 hunks)package.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- app/utils.ts
- app/components/MainPageContent.tsx
- package.json
- app/lib/config.ts
- app/blog/page.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T16:44:32.125Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 244
File: app/components/CopyAddressWarningModal.tsx:48-52
Timestamp: 2025-10-10T16:44:32.125Z
Learning: In the CopyAddressWarningModal component (app/components/CopyAddressWarningModal.tsx), selectedNetwork from useNetwork() is always defined and does not require null safety checks when accessing its properties like selectedNetwork.chain.name.
Applied to files:
app/components/Navbar.tsx
🧬 Code graph analysis (2)
app/components/HomePage.tsx (1)
app/hooks/useMiniMode.ts (1)
useMiniMode(10-16)
app/components/Navbar.tsx (6)
app/hooks/useMiniMode.ts (1)
useMiniMode(10-16)app/utils.ts (3)
classNames(23-25)IS_MAIN_PRODUCTION_DOMAIN(730-732)getNetworkImageUrl(863-868)app/components/WalletDetails.tsx (1)
WalletDetails(41-433)app/components/NetworksDropdown.tsx (1)
NetworksDropdown(23-128)app/components/SettingsDropdown.tsx (1)
SettingsDropdown(28-296)app/components/MobileDropdown.tsx (1)
MobileDropdown(26-299)
🪛 Biome (2.1.2)
app/components/Navbar.tsx
[error] 178-178: expected ) but instead found {
Remove {
(parse)
🔇 Additional comments (12)
next.config.mjs (2)
52-52: Whitespace-only change; no behavioral impactThis blank-line change is cosmetic only and has no effect on behavior; fine to keep as-is.
90-90: Whitespace-only change; no behavioral impactSame here: whitespace-only adjustment, safe and non-functional.
app/types.ts (2)
268-268: > Likely an incorrect or invalid review comment.
265-267: The fieldsfarcasterHeader,farcasterPayload, andfarcasterSignaturedo not exist in the codebase. A comprehensive search found zero occurrences of these fields in app/types.ts or anywhere else in the repository. The actual content at lines 265-267 contains different configuration fields (aggregatorUrl, privyAppId, thirdwebClientId, mixpanelToken, hotjarSiteId, googleVerificationCode). The only Farcaster references in the codebase are UI components for social sharing with Farcaster icons, not authentication configuration fields.Likely an incorrect or invalid review comment.
app/blog/[id]/page.tsx (1)
3-3: Importingredirectfor server-side navigation looks correctUsing
redirectfromnext/navigationis the right primitive for this new mini-mode redirect behavior in an App Router page. No issues here.app/components/HomePage.tsx (2)
18-18: LGTM! Clean integration of mini mode detection.The
useMiniModehook is correctly imported and used to determine the current mode.Also applies to: 53-54
149-441: LGTM! Marketing content appropriately hidden in mini mode.The conditional rendering correctly hides all additional content (video walkthrough, use cases, rates, FAQs, liquidity provider section, and mobile app download) when in mini mode, aligning with the simplified UI requirements for the Farcaster Mini App integration.
app/layout.tsx (2)
8-18: LGTM! Layout component imports properly organized.The new imports for the restructured layout (Providers, MainContent, navigation components, and MiniKitContextProvider) are properly organized and follow Next.js conventions.
258-279: LGTM! New layout structure properly implements Farcaster Mini App integration.The restructured layout correctly introduces:
EarlyReadycomponent for Farcaster readiness signalingMiniKitContextProviderwrapper for Farcaster SDK integration- Proper nesting of providers and layout components
- Conditional
NoticeBannerrenderingThe component composition follows React best practices.
Note: Line 265 also uses
config.noticeBannerText, which requires the missingconfigimport flagged in the earlier comment.app/components/Navbar.tsx (3)
30-30: LGTM! Mini mode hook properly integrated.The
useMiniModehook is correctly imported and initialized.Also applies to: 41-41
121-121: LGTM! Dropdown hover behavior correctly disabled in mini mode.The
onMouseEnterhandler properly prevents the dropdown from opening when in mini mode.
201-213: LGTM! Wallet controls and navigation correctly hidden in mini mode.The implementation correctly:
- Shows the Swap link only on blog routes when not in mini mode
- Hides the entire dropdown menu in mini mode
- Hides all wallet controls (WalletDetails, NetworksDropdown, SettingsDropdown, and mobile wallet button) in mini mode
This creates the simplified UI appropriate for the Farcaster Mini App integration.
Also applies to: 215-245, 270-309
…to pr-211-farcaster
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/Navbar.tsx (1)
175-208: Critical: Duplicate Blog link will render twice.Lines 175-185 and 198-208 are identical Blog link blocks with the same condition
!pathname.startsWith("/blog") && !isMiniMode. This will render two "Blog" links side by side in the navbar.Remove one of the duplicate blocks:
{/* Blog Link - Desktop Only - Hidden in mini mode */} {!pathname.startsWith("/blog") && !isMiniMode && ( <div className="hidden items-center sm:flex"> <Link href="/blog" className={` ${IS_MAIN_PRODUCTION_DOMAIN ? "" : "-mt-[3px]"} text-sm font-medium text-gray-700 transition-colors hover:text-gray-900 dark:text-white/80 dark:hover:text-white`} > Blog </Link> </div> )} {/* Home Link */} {pathname !== "/" && ( <div className="hidden items-center sm:flex"> <Link href="/" className={` ${IS_MAIN_PRODUCTION_DOMAIN ? "" : "-mt-[3px]"} text-sm font-medium text-gray-700 transition-colors hover:text-gray-900 dark:text-white/80 dark:hover:text-white`} > Home </Link> </div> )} - {/* Blog Link - Desktop Only - Hidden in mini mode */} - {!pathname.startsWith("/blog") && !isMiniMode && ( - <div className="hidden items-center sm:flex"> - <Link - href="/blog" - className={` ${IS_MAIN_PRODUCTION_DOMAIN ? "" : "-mt-[3px]"} text-sm font-medium text-gray-700 transition-colors hover:text-gray-900 dark:text-white/80 dark:hover:text-white`} - > - Blog - </Link> - </div> - )} -Also note: The Home link (lines 186-196) doesn't check
isMiniMode. Verify if this is intentional—if Home should also be hidden in mini mode, add&& !isMiniModeto its condition.
♻️ Duplicate comments (3)
app/layout.tsx (1)
51-55: Consider consolidating Farcaster frame meta withapp/head.tsx.Past review flagged potential duplication of
fc:framemeta tags between this file andapp/head.tsx. Verify that only one location emits these tags to avoid conflicting frame targets.#!/bin/bash # Check for fc:frame meta tag locations across the codebase rg -n 'fc:frame' -g '*.tsx' -g '*.ts' -C2app/lib/config.ts (1)
51-51: RemovesupabaseRoleKeyfrom client config — exposes server secret to browser.
SUPABASE_SERVICE_ROLE_KEYis a privileged server-side credential that should never be bundled into client code. It's already correctly placed inserverConfigat line 126.app/components/Navbar.tsx (1)
133-144:aria-controlsreferences missingidon dropdown element.The button sets
aria-controls="navbar-dropdown"(line 137) but the dropdownmotion.divat line 227 lacksid="navbar-dropdown". Add the id to complete the ARIA relationship:<motion.div + id="navbar-dropdown" + role="menu" initial={{ opacity: 0, y: -10 }}
🧹 Nitpick comments (1)
app/components/Navbar.tsx (1)
245-253: Redundant!isMiniModecheck.This condition is already inside the
isDropdownOpen && !isMiniModeblock (line 223), so the!isMiniModecheck here is unnecessary.- {!pathname.startsWith("/blog") && !isMiniMode && ( + {!pathname.startsWith("/blog") && (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/components/MainPageContent.tsx(7 hunks)app/components/Navbar.tsx(9 hunks)app/layout.tsx(3 hunks)app/lib/config.ts(2 hunks)app/types.ts(1 hunks)package.json(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-13T14:37:45.409Z
Learnt from: sundayonah
Repo: paycrest/noblocks PR: 250
File: app/components/AppLayout.tsx:33-46
Timestamp: 2025-10-13T14:37:45.409Z
Learning: In TypeScript/JavaScript/React codebases, JSX whitespace expressions like `{" "}` between elements should be classified as style/nitpick issues, not "Potential issues" or problems that affect functionality. These are purely cosmetic concerns about code cleanliness.
Applied to files:
app/components/Navbar.tsx
📚 Learning: 2025-10-10T16:44:32.125Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 244
File: app/components/CopyAddressWarningModal.tsx:48-52
Timestamp: 2025-10-10T16:44:32.125Z
Learning: In the CopyAddressWarningModal component (app/components/CopyAddressWarningModal.tsx), selectedNetwork from useNetwork() is always defined and does not require null safety checks when accessing its properties like selectedNetwork.chain.name.
Applied to files:
app/components/Navbar.tsx
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is false (verification succeeded), the recipient safety alert should display when: !isRecipientNameEditable && recipientName && !recipientNameError. The !errors.recipientName check is unnecessary because in non-editable mode the recipient name is displayed as read-only text (not an input field), so form validation errors cannot occur.
Applied to files:
app/components/MainPageContent.tsx
📚 Learning: 2025-07-23T07:30:23.720Z
Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:30:23.720Z
Learning: The `normalizeNetworkName` function in app/utils.ts has been updated to be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces and proper handling of acronyms like "BNB".
Applied to files:
app/types.ts
🧬 Code graph analysis (3)
app/components/Navbar.tsx (4)
app/hooks/useMiniMode.ts (1)
useMiniMode(10-16)app/utils.ts (2)
classNames(23-25)IS_MAIN_PRODUCTION_DOMAIN(730-732)app/components/WalletDetails.tsx (1)
WalletDetails(41-433)app/components/NetworksDropdown.tsx (1)
NetworksDropdown(23-128)
app/layout.tsx (6)
app/early-ready.tsx (1)
EarlyReady(5-34)providers/MiniKitProvider.tsx (1)
MiniKitContextProvider(25-42)app/components/Navbar.tsx (1)
Navbar(32-333)app/components/LayoutWrapper.tsx (1)
LayoutWrapper(11-22)app/components/Footer.tsx (1)
Footer(56-222)app/components/PWAInstallManager.tsx (1)
PWAInstall(5-37)
app/components/MainPageContent.tsx (4)
app/utils.ts (1)
getBannerPadding(1129-1132)app/types.ts (1)
STEPS(29-33)app/hooks/useMiniMode.ts (1)
useMiniMode(10-16)app/hooks/useBlockFestReferral.ts (1)
useBlockFestReferral(5-28)
🪛 Biome (2.1.2)
app/layout.tsx
[error] 19-19: Shouldn't redeclare 'Providers'. Consider to delete it or rename it.
'Providers' is defined here:
(lint/suspicious/noRedeclare)
[error] 20-20: Shouldn't redeclare 'MainContent'. Consider to delete it or rename it.
'MainContent' is defined here:
(lint/suspicious/noRedeclare)
[error] 22-22: Shouldn't redeclare 'Footer'. Consider to delete it or rename it.
'Footer' is defined here:
(lint/suspicious/noRedeclare)
[error] 23-23: Shouldn't redeclare 'Navbar'. Consider to delete it or rename it.
'Navbar' is defined here:
(lint/suspicious/noRedeclare)
[error] 24-24: Shouldn't redeclare 'LayoutWrapper'. Consider to delete it or rename it.
'LayoutWrapper' is defined here:
(lint/suspicious/noRedeclare)
[error] 25-25: Shouldn't redeclare 'PWAInstall'. Consider to delete it or rename it.
'PWAInstall' is defined here:
(lint/suspicious/noRedeclare)
[error] 26-26: Shouldn't redeclare 'NoticeBanner'. Consider to delete it or rename it.
'NoticeBanner' is defined here:
(lint/suspicious/noRedeclare)
[error] 28-28: Shouldn't redeclare 'EarlyReady'. Consider to delete it or rename it.
'EarlyReady' is defined here:
(lint/suspicious/noRedeclare)
[error] 29-29: Shouldn't redeclare 'MiniKitContextProvider'. Consider to delete it or rename it.
'MiniKitContextProvider' is defined here:
(lint/suspicious/noRedeclare)
app/components/MainPageContent.tsx
[error] 41-41: Shouldn't redeclare 'useMiniMode'. Consider to delete it or rename it.
'useMiniMode' is defined here:
(lint/suspicious/noRedeclare)
[error] 121-121: Shouldn't redeclare 'isMiniMode'. Consider to delete it or rename it.
'isMiniMode' is defined here:
(lint/suspicious/noRedeclare)
app/lib/config.ts
[error] 12-12: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 13-13: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 14-14: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 15-15: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 16-16: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 17-17: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 18-18: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 19-19: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 20-20: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 21-21: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 22-24: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 25-27: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 28-28: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 29-29: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 30-32: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 33-34: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 35-36: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 37-39: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 40-42: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 43-43: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
[error] 44-44: This property is later overwritten by an object member with the same name.
Overwritten with this property.
If an object property with the same name is defined multiple times (except when combining a getter with a setter), only the last definition makes it into the object and previous definitions are ignored.
Unsafe fix: Remove this property.
(lint/suspicious/noDuplicateObjectKeys)
package.json
[error] 74-74: expected , but instead found "tls"
Remove "tls"
(parse)
[error] 73-73: The key wagmi was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
🔇 Additional comments (4)
app/components/Navbar.tsx (4)
30-41: LGTM!The
useMiniModehook is correctly imported and used to conditionally adjust the Navbar behavior based on theminiquery parameter.
160-172: LGTM!Correctly hides the dropdown caret in mini mode, preventing user confusion about non-functional UI elements.
210-221: LGTM!The Swap link correctly appears only on blog pages and is properly hidden in mini mode.
277-316: LGTM!Cleanly wraps all wallet, network, and settings controls in a single
!isMiniModecondition. This appropriately hides these controls in the Farcaster mini app context where the host app manages wallet functionality.
package.json
Outdated
| "wagmi": "^2.16.9" | ||
| "tls": "^0.0.1", | ||
| "viem": "^2.31.0" | ||
| "viem": "^2.31.0", | ||
| "wagmi": "^2.16.9" | ||
| }, |
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.
Fix JSON syntax error and remove duplicate wagmi entry — breaks package installation.
Line 73 is missing a trailing comma, and wagmi is declared twice (lines 73 and 76). This will cause pnpm install to fail.
Apply this diff:
"thirdweb": "^5.102.6",
- "wagmi": "^2.16.9"
"tls": "^0.0.1",
- "viem": "^2.31.0",
+ "viem": "^2.31.0",
"wagmi": "^2.16.9"Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 74-74: expected , but instead found "tls"
Remove "tls"
(parse)
[error] 73-73: The key wagmi was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
🤖 Prompt for AI Agents
In package.json around lines 73 to 77, the dependencies block has a JSON syntax
error (missing trailing comma after the entry on line 73) and a duplicate
"wagmi" entry (lines 73 and 76); fix by removing the duplicate "wagmi"
declaration and ensure proper JSON punctuation by adding the missing comma where
needed so the dependencies object is valid for pnpm install.
…and adding supabaseRoleKey to the Config type. Update package.json to remove duplicate wagmi entry.
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: 0
🧹 Nitpick comments (1)
app/types.ts (1)
249-281: TightenConfigtyping and separate public vs server-only fields.Centralizing config here is good, but a few small tweaks would improve safety and type‑checking:
nodeEnvandnoIndexare better modeled as narrower types, e.g.:
nodeEnv: "development" | "test" | "production";noIndex: boolean;(convert from the env string inapp/lib/config.tsonce, then use as a boolean everywhere).- Sensitive fields like
supabaseRoleKeyandcdpApiKeyare mixed with clearly public fields (publicUrl, OG/meta fields, etc.) in a singleConfigshape. Consider splitting into something likePublicConfigandServerConfig(orPrivateConfig) so that server‑only values cannot be accidentally exposed to client code by reusing the wrong config object.These are non-breaking refactors but will make misuse of config much harder over time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/lib/config.ts(2 hunks)app/types.ts(1 hunks)package.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/lib/config.ts
- package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-23T07:30:23.720Z
Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 177
File: app/utils.ts:179-196
Timestamp: 2025-07-23T07:30:23.720Z
Learning: The `normalizeNetworkName` function in app/utils.ts has been updated to be dynamic and scalable, converting any network identifier to sentence case with hyphens replaced by spaces and proper handling of acronyms like "BNB".
Applied to files:
app/types.ts
🔇 Additional comments (1)
app/types.ts (1)
283-291:Network.imageUrlunion shape looks good.The
imageUrlunion (stringor{ light; dark }) is clear and flexible for theming; the reformatting here doesn’t introduce new behavior and the type remains sound for current and future callers.
…tection using Farcaster SDK
…ntext and improve mini app detection logic
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: 2
♻️ Duplicate comments (3)
app/lib/config.ts (1)
48-48: Security:supabaseRoleKeyshould not be in client-accessible config.The
SUPABASE_SERVICE_ROLE_KEYis a sensitive server-side secret that grants admin access to your Supabase database. Including it in the sharedconfigobject risks exposing it in client bundles.This was flagged in a previous review. Move it exclusively to
serverConfig(which you already have at line 97) and remove it from the main config object.blockfestEndDate: process.env.NEXT_PUBLIC_BLOCKFEST_END_DATE || "2025-10-11T23:59:00+01:00", - supabaseRoleKey: process.env.SUPABASE_SERVICE_ROLE_KEY || "", baseBuilderOwnerAddress: process.env.BASE_BUILDER_OWNER_ADDRESS || "",Then import
serverConfig.supabaseRoleKeyonly in server-side code.app/context/InjectedWalletContext.tsx (2)
88-110: Reset all injected wallet state in every error branch (avoid stale provider/address).The
code === 4001branch correctly clearsisInjectedWallet,injectedProvider,injectedAddress, andinjectedReady. However:
- The
"User rejected"message branch only callssetIsInjectedWallet(false).- The generic error branch also only calls
setIsInjectedWallet(false).If the user had previously connected successfully (or a prior attempt partially initialized state), these branches will leave a stale provider/address/ready flag in context, which can confuse downstream consumers.
You already have the full reset pattern in the 4001 branch; mirror it in all failure paths, or factor out a small helper:
- } else if ((error as any)?.message?.includes("User rejected")) { - toast.error("Wallet connection was cancelled by user.", { - description: "You can try connecting again later.", - }); - setIsInjectedWallet(false); - } else { + } else if ((error as any)?.message?.includes("User rejected")) { + toast.error("Wallet connection was cancelled by user.", { + description: "You can try connecting again later.", + }); + setIsInjectedWallet(false); + setInjectedProvider(null); + setInjectedAddress(null); + setInjectedReady(false); + } else { toast.error( "Failed to connect to wallet. Please refresh and try again.", { description: "If the problem persists, try restarting the app.", }, ); - setIsInjectedWallet(false); + setIsInjectedWallet(false); + setInjectedProvider(null); + setInjectedAddress(null); + setInjectedReady(false); }Or:
const resetInjectedState = () => { setIsInjectedWallet(false); setInjectedProvider(null); setInjectedAddress(null); setInjectedReady(false); };and call
resetInjectedState()in all error branches.
37-43: Guard async state updates with thecancelledflag to avoid setState after unmount.You added a
cancelledflag and only scheduleinitInjectedWalletif!cancelled, which protects against the effect firing after unmount before the timeout elapses. However, onceinitInjectedWallethas started, unmounting the component doesn’t prevent the async flow from continuing and callingsetInjectedProvider,setInjectedAddress,setInjectedReady, etc.To fully avoid setState on an unmounted component:
- Capture
cancelledinsideinitInjectedWallet(it already closes over it).- Check
if (cancelled) return;before any state updates.- Optionally early‑return after
shouldUseInjectedWallet(...)if cancelled.For example:
const initInjectedWallet = async () => { const shouldUse = shouldUseInjectedWallet(searchParams); - setIsInjectedWallet(shouldUse); + if (cancelled) return; + setIsInjectedWallet(shouldUse); // ... after async calls - if (address) { + if (cancelled) return; + if (address) { setInjectedProvider(ethereumProvider); setInjectedAddress(address); setInjectedReady(true); } // In each error/failure branch: - setIsInjectedWallet(false); - setInjectedProvider(null); - setInjectedAddress(null); - setInjectedReady(false); + if (!cancelled) { + setIsInjectedWallet(false); + setInjectedProvider(null); + setInjectedAddress(null); + setInjectedReady(false); + } }; let cancelled = false; const timeoutId = setTimeout(() => !cancelled && initInjectedWallet(), 200); return () => { cancelled = true; clearTimeout(timeoutId); };This keeps the async wallet initialization safe even if the provider is slow and the user navigates away mid‑flow.
Also applies to: 65-67, 73-77, 88-97, 114-121
🧹 Nitpick comments (7)
app/context/BaseAppContext.tsx (2)
38-41: Remove unusedisMountedstate.
isMountedis set totrueon Line 41 but never read. This is dead code that should be removed.Apply this diff:
const [sdk, setSdk] = useState<any | null>(null); - const [isMounted, setIsMounted] = useState(false); useEffect(() => { - setIsMounted(true); - const initBaseApp = async () => {
45-58: Consolidate duplicate URL parameter parsing.The code parses the same query parameters twice: once via
searchParamshook (Lines 45-48) and again viawindow.location.search(Lines 55-58). Consider consolidating to reduce duplication and potential divergence.const initBaseApp = async () => { - // Check for query params for local testing (including Farcaster pattern) - const baseAppParam = searchParams?.get("baseApp"); - const miniParam = searchParams?.get("mini"); - const miniAppParam = searchParams?.get("miniApp"); - const shouldUseForTesting = baseAppParam === "true" || miniParam === "true" || miniAppParam === "true"; - if (typeof window === "undefined") { return; } // Check URL query param and pathname (Farcaster pattern: /mini pathname) const urlParams = new URLSearchParams(window.location.search); const urlPathname = window.location.pathname; - const hasBaseAppParam = urlParams.get("baseApp") === "true" || urlParams.get("mini") === "true" || urlParams.get("miniApp") === "true"; + const shouldUseForTesting = urlParams.get("baseApp") === "true" || urlParams.get("mini") === "true" || urlParams.get("miniApp") === "true"; const hasMiniPathname = urlPathname.startsWith("/mini"); // Farcaster docs patternapp/.well-known/farcaster.json/route.ts (2)
1-1: Remove unused import.
NextRequestis imported but never used in this route handler.-import { NextRequest, NextResponse } from "next/server"; +import { NextResponse } from "next/server";
7-7: Consider adding a type for the manifest object.Using
anyloses TypeScript benefits. Define an interface for the Farcaster manifest structure to improve type safety and documentation.- const manifest: any = { + interface FarcasterManifest { + accountAssociation: { + header: string; + payload: string; + signature: string; + }; + baseBuilder: { + ownerAddress: string; + }; + miniapp: { + version: string; + name: string; + homeUrl: string; + iconUrl: string; + imageUrl: string; + splashImageUrl: string; + splashBackgroundColor: string; + subtitle: string; + description: string; + screenshotUrls: string[]; + primaryCategory: string; + tags: string[]; + heroImageUrl: string; + tagline: string; + ogTitle: string; + ogDescription: string; + ogImageUrl: string; + noindex: boolean; + }; + } + + const manifest: FarcasterManifest = {app/pages/TransactionPreview.tsx (1)
120-133: Wallet objects includetypeproperty here but not in TransactionForm.The
baseAppWalletObjandinjectedWallethere include atypeproperty, while the equivalent objects inTransactionForm.tsxdo not. Consider aligning the structure across both files for consistency and to enable consistent analytics tracking.app/pages/TransactionStatus.tsx (1)
354-375: Balance and wallet type logic is correct.The balance priority and wallet type derivation follow the established pattern from other components.
Minor inconsistency: The
walletTypefor injected wallets is"Injected"here (line 364), but"Injected wallet"inTransactionPreview.tsx(line 281). Consider aligning these for consistent analytics data.const walletType = isBaseApp ? "Base App wallet" : isFarcaster ? "Farcaster wallet" : isInjectedWallet - ? "Injected" + ? "Injected wallet" : "Smart wallet";app/types.ts (1)
249-275: Consider tighteningConfigandNetworktypes for stronger compile‑time guarantees.The new fields on
Configand the widenedNetwork.imageUrlunion look consistent with the app’s config needs, but you might get more mileage from stricter typing:
nodeEnvandnoIndexare plainstring; consider:
nodeEnv: "development" | "production" | "test"(or whatever you actually support).noIndex: boolean(or"true" | "false"if it mirrors env vars directly).- Fields like
appPrimaryCategoryand Farcaster/Base App metadata could be restricted to known literals if they are constrained by external APIs (e.g., Farcaster categories).- For
Network.imageUrl, the union is fine; just ensure all renderers handle bothstringand{ light, dark }branches (a small helper likegetNetworkImageUrl(network, theme)can keep that logic centralized).These are non‑blocking, but they’ll catch misconfigured deployments earlier in TypeScript instead of at runtime.
Also applies to: 282-282, 287-295
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
app/.well-known/farcaster.json/route.ts(1 hunks)app/components/AppLayout.tsx(3 hunks)app/components/HomePage.tsx(1 hunks)app/components/MainPageContent.tsx(2 hunks)app/components/Navbar.tsx(1 hunks)app/context/BaseAppContext.tsx(1 hunks)app/context/InjectedWalletContext.tsx(3 hunks)app/context/index.ts(1 hunks)app/early-ready.tsx(1 hunks)app/layout.tsx(3 hunks)app/lib/config.ts(2 hunks)app/pages/TransactionForm.tsx(7 hunks)app/pages/TransactionPreview.tsx(6 hunks)app/pages/TransactionStatus.tsx(8 hunks)app/providers.tsx(3 hunks)app/types.ts(1 hunks)next.config.mjs(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/components/Navbar.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- app/components/MainPageContent.tsx
- app/components/HomePage.tsx
- next.config.mjs
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-12-05T02:22:28.063Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 312
File: app/page.tsx:7-11
Timestamp: 2025-12-05T02:22:28.063Z
Learning: In Next.js 15 with React 19, components using useSearchParams() cannot be statically prerendered. When app/layout.tsx has `export const dynamic = "force-static"`, MainPageContent must be dynamically imported with ssr: false to avoid "Element type is invalid" errors during build. The dynamic import pattern `dynamic(() => import("./components/MainPageContent").then(mod => ({ default: mod.MainPageContent })), { ssr: false })` is the correct solution for this scenario in the noblocks repository.
Applied to files:
app/context/BaseAppContext.tsxapp/components/AppLayout.tsxapp/layout.tsx
📚 Learning: 2025-10-10T16:44:32.125Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 244
File: app/components/CopyAddressWarningModal.tsx:48-52
Timestamp: 2025-10-10T16:44:32.125Z
Learning: In the CopyAddressWarningModal component (app/components/CopyAddressWarningModal.tsx), selectedNetwork from useNetwork() is always defined and does not require null safety checks when accessing its properties like selectedNetwork.chain.name.
Applied to files:
app/pages/TransactionPreview.tsxapp/pages/TransactionForm.tsx
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is false (verification succeeded), the recipient safety alert should display when: !isRecipientNameEditable && recipientName && !recipientNameError. The !errors.recipientName check is unnecessary because in non-editable mode the recipient name is displayed as read-only text (not an input field), so form validation errors cannot occur.
Applied to files:
app/pages/TransactionForm.tsxapp/pages/TransactionStatus.tsx
📚 Learning: 2025-11-06T07:37:39.036Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:539-552
Timestamp: 2025-11-06T07:37:39.036Z
Learning: In RecipientDetailsForm (app/components/recipient/RecipientDetailsForm.tsx), when isRecipientNameEditable is true (verification failed/returned "Ok"), the recipient safety alert should display when: isRecipientNameEditable && recipientName && !errors.recipientName && !recipientNameError. The !isFetchingRecipientName check is redundant because recipientName is cleared at fetch start and only populated after fetching completes or when the user manually enters it.
Applied to files:
app/pages/TransactionForm.tsxapp/context/InjectedWalletContext.tsxapp/pages/TransactionStatus.tsx
📚 Learning: 2025-11-06T07:08:54.593Z
Learnt from: Dprof-in-tech
Repo: paycrest/noblocks PR: 231
File: app/components/recipient/RecipientDetailsForm.tsx:271-280
Timestamp: 2025-11-06T07:08:54.593Z
Learning: In the RecipientDetailsForm component (app/components/recipient/RecipientDetailsForm.tsx), Mobile Money institutions always return "Ok" from the fetchAccountName verification endpoint. Therefore, checking for accountName.toLowerCase() === "ok" is sufficient to handle both verification failures and Mobile Money institutions without needing explicit institution type checks in the frontend.
Applied to files:
app/pages/TransactionStatus.tsx
📚 Learning: 2025-10-13T14:59:46.723Z
Learnt from: jeremy0x
Repo: paycrest/noblocks PR: 251
File: app/components/blockfest/BlockFestCashbackComponent.tsx:191-191
Timestamp: 2025-10-13T14:59:46.723Z
Learning: In app/components/blockfest/BlockFestCashbackComponent.tsx, the social share URLs intentionally mention "2% cashback" even though the component's cashbackPercentage prop defaults to "1%". The 2% represents the total potential cashback (1% instant + 1% bonus from showing the post at the booth), while the prop represents only the automatic portion.
Applied to files:
app/pages/TransactionStatus.tsx
🧬 Code graph analysis (6)
app/early-ready.tsx (1)
public/sw.js (1)
url(54-54)
app/context/BaseAppContext.tsx (1)
app/context/index.ts (2)
BaseAppProvider(15-15)useBaseApp(15-15)
app/providers.tsx (2)
app/context/BaseAppContext.tsx (1)
BaseAppProvider(174-180)app/context/index.ts (1)
BaseAppProvider(15-15)
app/pages/TransactionForm.tsx (5)
app/context/BaseAppContext.tsx (1)
useBaseApp(182-182)app/context/index.ts (1)
useBaseApp(15-15)app/components/index.ts (2)
AnimatedComponent(15-15)slideInOut(19-19)app/components/AnimatedComponents.tsx (2)
AnimatedComponent(172-188)slideInOut(28-32)app/utils.ts (1)
formatNumberWithCommas(56-60)
app/pages/TransactionStatus.tsx (4)
app/context/BaseAppContext.tsx (1)
useBaseApp(182-182)app/context/index.ts (1)
useBaseApp(15-15)app/components/index.ts (2)
AnimatedComponent(15-15)slideInOut(19-19)app/components/AnimatedComponents.tsx (2)
AnimatedComponent(172-188)slideInOut(28-32)
app/layout.tsx (2)
app/early-ready.tsx (1)
EarlyReady(10-60)app/components/AppLayout.tsx (1)
AppLayout(16-63)
🪛 ast-grep (0.40.0)
app/layout.tsx
[warning] 257-257: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
app/early-ready.tsx
[error] 31-31: Unexpected constant condition.
(lint/correctness/noConstantCondition)
app/context/BaseAppContext.tsx
[error] 141-141: This branch can never execute. Its condition is a duplicate or covered by previous conditions in the if-else-if chain.
(lint/suspicious/noDuplicateElseIf)
app/layout.tsx
[error] 258-258: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (15)
app/context/index.ts (1)
15-15: LGTM!The new export follows the established barrel pattern and correctly exposes the
BaseAppProvideranduseBaseAppfrom the new context module.app/layout.tsx (2)
28-41: Hardcoded production URLs in fc:miniapp metadata.The
fc:miniapppayload uses hardcodedhttps://noblocks.xyzURLs. This is appropriate for the production canonical URL, but consider whether preview/staging deployments need different URLs. If so, derive from a config value.Confirm that all Farcaster deployments should point to the canonical production URL, or update to use
config.publicUrlfor environment-specific behavior.
253-262: LGTM!The placement of
EarlyReadyat the top of the body ensures SDK readiness signals fire early. WrappingAppLayoutwithProviderscorrectly establishes the context hierarchy. ThedangerouslySetInnerHTMLfor JSON-LD (Line 258) is safe sincejsonLdis a static constant with no user input.app/components/AppLayout.tsx (2)
17-26: LGTM — correct hydration-safe pattern for conditional UI.The
isMountedstate withuseEffectensures the mini app check only applies after hydration, preventing server/client mismatch. The full UI renders on the server, then conditionally hides on the client when in a mini app context.
31-43: Appropriate UI simplification for mini app contexts.Hiding Navbar, NoticeBanner, Footer, and PWAInstall when embedded in Farcaster/Base App provides a cleaner, more integrated experience without redundant navigation or install prompts.
app/providers.tsx (1)
90-108: BaseAppProvider integration looks correct.The
BaseAppProvideris appropriately placed as the outermost wrapper withinContextProviders, ensuring the base app context is available to all nested providers and components.Minor formatting note: The child elements (lines 91-107) lack indentation under the
<BaseAppProvider>wrapper, which slightly hurts readability.app/.well-known/farcaster.json/route.ts (1)
42-52: Review caching and X-Frame-Options headers for production.Two concerns:
- Cache-Control: The comment says "during testing" but this aggressive no-cache policy will persist in production. Consider using environment-based caching:
const cacheControl = process.env.NODE_ENV === "production" ? "public, max-age=3600" : "no-cache, no-store, must-revalidate, max-age=0";
- X-Frame-Options: Setting it to an empty string sends an invalid header rather than removing it. If you need to allow framing, simply omit the header or use
ALLOWALL(non-standard). For proper control, useContent-Security-Policy: frame-ancestorsinstead.app/pages/TransactionForm.tsx (2)
115-133: Wallet and balance priority logic is well-structured.The priority chain (Base App → Injected → Smart Wallet) is correctly implemented, and the balance selection appropriately uses
injectedWalletBalancefor Base App/Farcaster contexts since they use the injected provider.Minor observation: The wallet objects have slightly inconsistent shapes—
baseAppWalletObjandinjectedWalletObjare plain objects with onlyaddress, whilesmartWalletObjcomes fromlinkedAccountsand may have additional properties. This works because onlyactiveWallet?.addressis accessed, but consider unifying the shape for clarity.
687-690: Formatting change only—no functional impact.The className template literal restructuring maintains the same conditional logic.
app/pages/TransactionPreview.tsx (2)
200-204: Unified wallet flow for Base App, Farcaster, and Injected wallets looks correct.The condition
(isBaseApp || isFarcaster || isInjectedWallet) && injectedProvidercorrectly routes all external wallet types through the same code path since they all use the injected provider.
279-282: Analytics tracking correctly distinguishes wallet types.The ternary chain properly identifies "Base App wallet", "Farcaster wallet", or "Injected wallet" for event tracking.
app/pages/TransactionStatus.tsx (2)
507-514: StatusIndicator className formatting refactored.The nested ternary in the className is functionally equivalent to the previous version—just reformatted.
859-869: BlockFest cashback component rendering unchanged.The JSX indentation was adjusted but the rendering logic and props remain the same.
app/lib/config.ts (1)
49-52: The referenced code does not exist in the current codebase.The fields
baseBuilderOwnerAddress,baseAppHeader,baseAppPayload, andbaseAppSignaturementioned in the review are not present in app/lib/config.ts. Lines 49-52 of the current file contain Sanity configuration, not Base App configuration. A search of the entire codebase confirms these fields are not defined anywhere.Likely an incorrect or invalid review comment.
app/context/InjectedWalletContext.tsx (1)
44-60: The review is based on code that does not exist in the file.The code snippet shown in the review comment (lines 44-60) references Farcaster SDK integration (
sdk.isInMiniApp(),sdk.wallet.getEthereumProvider()), asetTimeoutmechanism, and acancelledflag—none of which are present in the actual file. The current implementation is much simpler and only useswindow.ethereumdirectly.While the underlying concerns about state consistency are valid, they apply to the actual code in a different way:
- Line 41 sets
isInjectedWallet = shouldUseunconditionally, then line 43 gates the initialization on bothshouldUse && window.ethereum. IfshouldUseis true butwindow.ethereumis undefined, the state ends up inconsistent (isInjectedWallet=true with null provider/address/ready).- Lines 75–79 reset only
isInjectedWalleton generic errors, unlike the 4001 error case (lines 71–74), which fully resets provider/address/ready.Please clarify: Is this review intended for a different branch or version of the code, or is it a suggested refactoring to add Farcaster SDK support?
Likely an incorrect or invalid review comment.
| } else if (shouldUseForTesting) { | ||
| // For testing mode, default to Base App | ||
| setIsBaseAppEnv(true); | ||
| setBaseAppReady(true); | ||
| } |
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.
Unreachable code: this branch can never execute.
The condition at Line 141 (shouldUseForTesting) can never be true here. If shouldUseForTesting were true, the if at Line 104 (isInMiniApp || shouldUseForTesting) would have been taken instead. Remove this dead branch.
Apply this diff:
}
} catch (error) {
console.warn("[BaseApp/Farcaster] Failed to get context:", error);
// Default to Base App for testing
if (shouldUseForTesting) {
setIsBaseAppEnv(true);
}
}
- } else if (shouldUseForTesting) {
- // For testing mode, default to Base App
- setIsBaseAppEnv(true);
- setBaseAppReady(true);
}📝 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.
| } else if (shouldUseForTesting) { | |
| // For testing mode, default to Base App | |
| setIsBaseAppEnv(true); | |
| setBaseAppReady(true); | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 141-141: This branch can never execute. Its condition is a duplicate or covered by previous conditions in the if-else-if chain.
(lint/suspicious/noDuplicateElseIf)
🤖 Prompt for AI Agents
In app/context/BaseAppContext.tsx around lines 141 to 145, the else-if branch
checking shouldUseForTesting is unreachable because the earlier if at line ~104
already handles isInMiniApp || shouldUseForTesting; remove the dead else-if
block (and its setIsBaseAppEnv/setBaseAppReady calls) so there is no redundant
branch, and if any logic from that block is required under different conditions,
move it into the appropriate reachable branch or consolidate it into the earlier
if to preserve intended behavior.
| const isMini = | ||
| url.pathname.startsWith("/mini") || | ||
| url.searchParams.get("miniApp") === "true" || | ||
| url.searchParams.get("mini") === "true" || | ||
| url.searchParams.get("baseApp") === "true" || | ||
| !!(window as any).minikit || // Base App MiniKit | ||
| window.navigator?.userAgent?.toLowerCase().includes("baseapp") || | ||
| window.navigator?.userAgent?.toLowerCase().includes("farcaster") || | ||
| window.navigator?.userAgent?.toLowerCase().includes("warpcast") || | ||
| window.top !== window.self; // In iframe | ||
|
|
||
|
|
||
| if (isMini || true) { // Always call for Base App compatibility |
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 | 🟠 Major
Remove dead code: isMini is never used due to || true.
The isMini || true condition (Line 31) always evaluates to true, making the entire isMini computation (Lines 19-28) dead code. If the intent is to always call ready() for Base App compatibility, simplify by removing the unused logic.
Apply this diff:
- // Check for mini app conditions (Farcaster docs pattern + Base App)
- const url = new URL(window.location.href);
- const isMini =
- url.pathname.startsWith("/mini") ||
- url.searchParams.get("miniApp") === "true" ||
- url.searchParams.get("mini") === "true" ||
- url.searchParams.get("baseApp") === "true" ||
- !!(window as any).minikit || // Base App MiniKit
- window.navigator?.userAgent?.toLowerCase().includes("baseapp") ||
- window.navigator?.userAgent?.toLowerCase().includes("farcaster") ||
- window.navigator?.userAgent?.toLowerCase().includes("warpcast") ||
- window.top !== window.self; // In iframe
-
-
- if (isMini || true) { // Always call for Base App compatibility
+ // Always call ready() for Base App compatibility
+ {
try {Alternatively, if the detection logic should be preserved for future conditional behavior, remove the || true:
- if (isMini || true) { // Always call for Base App compatibility
+ if (isMini) {📝 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 isMini = | |
| url.pathname.startsWith("/mini") || | |
| url.searchParams.get("miniApp") === "true" || | |
| url.searchParams.get("mini") === "true" || | |
| url.searchParams.get("baseApp") === "true" || | |
| !!(window as any).minikit || // Base App MiniKit | |
| window.navigator?.userAgent?.toLowerCase().includes("baseapp") || | |
| window.navigator?.userAgent?.toLowerCase().includes("farcaster") || | |
| window.navigator?.userAgent?.toLowerCase().includes("warpcast") || | |
| window.top !== window.self; // In iframe | |
| if (isMini || true) { // Always call for Base App compatibility | |
| const isMini = | |
| url.pathname.startsWith("/mini") || | |
| url.searchParams.get("miniApp") === "true" || | |
| url.searchParams.get("mini") === "true" || | |
| url.searchParams.get("baseApp") === "true" || | |
| !!(window as any).minikit || // Base App MiniKit | |
| window.navigator?.userAgent?.toLowerCase().includes("baseapp") || | |
| window.navigator?.userAgent?.toLowerCase().includes("farcaster") || | |
| window.navigator?.userAgent?.toLowerCase().includes("warpcast") || | |
| window.top !== window.self; // In iframe | |
| if (isMini) { |
🧰 Tools
🪛 Biome (2.1.2)
[error] 31-31: Unexpected constant condition.
(lint/correctness/noConstantCondition)
🤖 Prompt for AI Agents
In app/early-ready.tsx around lines 19–31, the condition `if (isMini || true)`
makes the `isMini` computation dead code; either remove the unused detection
logic and call ready() unconditionally (delete the `isMini` declaration and the
`if` wrapping so ready() is invoked directly), or preserve the detection logic
by removing the `|| true` so the `if (isMini)` check behaves as intended—pick
one approach and update the code accordingly.
Description
This PR completes the deployment and integration of the Noblocks mini app (“Noblocks Mini App”) on Farcaster.
-Configured and deployed the Noblocks project on Vercel.
-Created and set up packaster.json with correct frame metadata (name, icons, images, categories, etc.).
-Wrapped the app with MiniKit for Farcaster mini app compatibility.
-Configured domain association (accountAssociation) for app verification on Farcaster.
-Added redirect for /.well-known/farcaster.json to the hosted manifest URL.
-Verified the manifest and confirmed the app is now visible under “My MiniApps” on the Farcaster Developer portal.
-Ensured the app opens in-app on Farcaster instead of in an external browser.
Testing
Log into Farcaster and Search for "Noblocks", it opens up the Noblocks Miniapp.
Checklist
mainReference
Closes #307 #182
By submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.