[#36] Add Farcaster Mini App manifest and SDK setup#146
Conversation
- Install @farcaster/miniapp-sdk and @farcaster/miniapp-wagmi-connector - Serve manifest at /.well-known/farcaster.json via Next.js route handler - Add FarcasterMiniApp component that detects mini app context and calls sdk.actions.ready() - Wire farcasterMiniApp() connector into wagmi config Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The PR is currently blocked at dependency resolution and cannot pass CI as submitted. npm ci fails before lint/typecheck because the chosen Farcaster wagmi connector has an incompatible peer dependency on wagmi v2, while this repo uses wagmi v3.
Findings
- [high] PR introduces an unresolvable dependency conflict with the repo's wagmi version
- File:
package.json - Suggestion:
@farcaster/miniapp-wagmi-connector@1.1.1peers on@wagmi/core@^2.14.1, but this project installswagmi@^3.5.0/@wagmi/core@3.4.0. CI fails atnpm ciwithERESOLVEfor that mismatch, so the repo cannot be installed cleanly. Use a connector/package version compatible with wagmi v3, or avoid introducing the incompatible package until the stack is aligned.
- File:
Decision
Request changes because the PR does not currently build/install in CI and therefore cannot be merged safely.
project7-interns
left a comment
There was a problem hiding this comment.
T2b Review: REQUEST CHANGES
Blocking
- Iframe detection too broad —
window.parent !== windowinFarcasterMiniApp.tsxtriggerssdk.actions.ready()in ANY iframe (analytics preview, link unfurling, etc.), not just Farcaster. This causes unnecessary SDK loading and confusing behavior. Remove the iframe fallback, or replace withsdk.contextvalidation (see below).
Strong recommendations
- Use
sdk.contextinstead of manual heuristics — Rather than checking query params and iframe status, dynamic-import the SDK and callsdk.contextfirst. If it returns a valid context, callready(). This eliminates false positives and the manual detection entirely. Also add.catch()for the import chain.
import("@farcaster/miniapp-sdk").then(async ({ sdk }) => {
if (cancelled) return;
const context = await sdk.context;
if (context && !cancelled) sdk.actions.ready();
}).catch(() => {});- Add TODO for wagmi v2/v3 peer dep mismatch —
@farcaster/miniapp-wagmi-connectorrequires@wagmi/core ^2.14.1, project uses wagmi v3. Works with--legacy-peer-depsbut could break on connector internals. Add a TODO comment inlib/wagmi.tsto track upgrading when a v3-compatible release is available.
Low
- Empty
accountAssociation— acceptable as scaffold placeholder, but consider returning 503 or omitting the field when values are empty, so it fails explicitly instead of serving an invalid manifest.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The dependency issue is fixed and lint-and-typecheck passes, but the mini app detection logic is still too broad and does not reliably check for a real Farcaster client context.
Findings
- [high]
FarcasterMiniApptreats any iframe as Farcaster context viawindow.parent !== window, then callssdk.actions.ready()unconditionally. That will fire inside arbitrary embeds, not just Farcaster, which does not satisfy issue #36's requirement to check whether the app is running inside a Farcaster client before callingready().- File:
src/components/FarcasterMiniApp.tsx:14 - Suggestion: remove the generic iframe and URL heuristics and gate
sdk.actions.ready()on actual Farcaster SDK context validation instead.
- File:
Decision
Requesting changes because the PR still mis-detects non-Farcaster iframe contexts as Farcaster mini apps.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The PR now cleanly adds the Farcaster mini app manifest and gates sdk.actions.ready() on real Farcaster SDK context. The earlier wagmi connector incompatibility and broad iframe detection issues are fixed.
Findings
- [info] No blocking findings.
Decision
Approving because the implementation now matches issue #36 and lint-and-typecheck passed.
project7-interns
left a comment
There was a problem hiding this comment.
T2b Re-review: APPROVE
All blocking issues fixed:
- Iframe detection replaced with proper
sdk.contextvalidation — no more false positives .catch()added for non-Farcaster environments- Cleanup via
cancelledflag prevents state updates after unmount
Non-blocking: no TODO comment for wagmi v2/v3 peer dep mismatch in lib/wagmi.ts — track separately when @farcaster/miniapp-wagmi-connector releases a v3-compatible version.
Summary
Fixes #36
@farcaster/miniapp-sdkand@farcaster/miniapp-wagmi-connector/.well-known/farcaster.jsonvia a Next.js route handler, withhomeUrlderived fromNEXT_PUBLIC_APP_URLFarcasterMiniAppcomponent that detects mini app context and callssdk.actions.ready()on mount to dismiss the splash screenfarcasterMiniApp()connector into the wagmi config alongside the existinginjected()connectorNotes
accountAssociationfields in the manifest are empty placeholders — must be populated after registering the app at the Farcaster developer portal--legacy-peer-depsdue to a peer dependency mismatch (package requires wagmi v2, project uses v3). Connector functionality should be verified at integration time.packages/sdk/dist/are unrelated to this PRTest plan
npm run typecheckpassesnpm run lintshows no new errorsGET /.well-known/farcaster.jsonreturns valid manifest JSON with correcthomeUrlsdk.actions.ready()fires when loaded inside Farcaster client context🤖 Generated with Claude Code