Finalize farcaster support - #57
Draft
L03TJ3 wants to merge 8 commits into
Draft
Conversation
#14) * Feat: Installed farcaster package * fix: build errors * feat: Add Farcaster miniapp support for identity flow with build fixes * rm eslintrc * refactor: Address code review feedback with improved architecture across utilities * fix: Address code review complaints * fix: working on resolving the reviews made * fix: removed dynamic import, and reverted some configs to the default and added farcaster/miniapp-sdk as dependency to citizen-sdk * pulled update * fix: Addressed all outstanding code review feedback. * fix: resolved all code changes requested * feat: implement shared build config utility for Node.js built-ins and also created packages/build-config/index.ts with dynamic Node.js built-ins generation * revert: removed the shared build config changes and other changes * fc:frame meta tag * fix: refined the codebase * fix:patching up the codebase * Revert "fix:patching up the codebase" This reverts commit 8616d89. * revert: reverted all changes requested * revert: reverted all changes requested * revert: reverted all changes requested * fix: revert changes requested * fix: revert changes requested * fix: revert changes affecting mobile * feat: implemented the universal link using farcaster docs * fix: resolving previous issues * fix: resolved redirection concerns and fixed bugs * fix: build id working well now * fix: remove the unused interface of VerifyButtonProps * fix: removed the used import { createUniversalLinkCallback } from ../utils/auth * fix: revert apps/demo-identity-app/src/App.tsx and verifybutton.tsx to original state * fix: update auth.ts to use chainConfigs instead of contractAddresses After merging upstream/main, the contract address structure changed from contractAddresses to chainConfigs. Updated isAddressWhitelisted function to use the new chainConfigs structure. * refactor: apply engineering principles to Farcaster implementation - Remove excessive console.log statements from auth utilities and SDKs - Simplify error handling by removing redundant try-catch blocks - Remove verbose JSDoc comments that state the obvious - Consolidate duplicate navigation functions - Improve code readability and maintainability - Follow KISS, DRY, and clarity over cleverness principles * refactor: clean up demo app following engineering principles - Remove unnecessary console.log/console.error statements - Remove unused onVerificationSuccess prop from VerifyButton - Simplify error handling in demo components - Rename callBackExample to txConfirm for clarity - Follow DRY and KISS principles * fix: address code review feedback - Restore original fvRedirect behavior in ClaimSDK (window.location.href) - Use fvDefaultChain instead of hardcoded chainId - Restore error handling in submitAndWait - Restore removed JSDoc comments - Remove unused handleVerificationResponse functions - Fix VerifyButton error handling - Add FarcasterAppConfigs documentation to README - Add JSDoc to createVerificationCallbackUrl * fix: address round 3 code review comments (restore comments, fix fvRedirect, restore error handling) * fix: address PR review feedback from L03TJ3 - Restore try-catch in getWhitelistedRoot with meaningful error message - Re-throw original error in submitAndWait to preserve error type - Simplify createVerificationCallbackUrl (remove /verify path enforcement) - Add parseVerificationResult helper for verified query param parsing - Restore console.error in App.tsx catch block - Remove unused navigateToFaceVerification method - Remove commit-style comment in fvRedirect * fix: address round 2 PR review feedback from sirpy - Restore all original JSDoc, try-catch, and error handling patterns - Add resolveCallbackUrl to IdentitySDK for Farcaster/URL consolidation - Remove duplicate Farcaster logic from ClaimSDK (delegate to IdentitySDK) - Remove FarcasterAppConfigs (apps don't have envs in Farcaster) - Remove redundant isAddressWhitelisted from auth utils - Move @farcaster/miniapp-sdk to peerDependencies - Restore VerifyButton original props and chain ID * feat: add navigateToFaceVerification with Farcaster-aware redirect - Add navigateToFaceVerification() to IdentitySDK for Farcaster-aware FV navigation - ClaimSDK.fvRedirect() delegates to IdentitySDK instead of using window.location.href - Refactor FarcasterAppConfig: domain is now required, appId/appSlug optional - Support domain-based Farcaster deep links for redirect-back flow - Pass farcasterConfig through useIdentitySDK and useClaimSDK hooks - VerifyButton uses navigateToFaceVerification() for correct navigation * refactor: remove Farcaster config coupling from SDK internals - Removed `farcasterConfig` from `IdentitySDK` options and class state - Removed `farcasterConfig` param from `useIdentitySDK` and `useClaimSDK` hooks - Reverted `generateFVLink` back to using `callbackUrl` directly without internal interception - Removed `resolveCallbackUrl` helper method - Reverted `ClaimSDK.fvRedirect()` to use `window.location.href` - Added explicit `generateFarcasterCallback` method to `IdentitySDK` - Updated `VerifyButton` to handle Farcaster context explicitly and generate callback URL locally --------- Co-authored-by: Lewis B <lewis@ikigaistudios.eu>
|
QA Report Tests
Bugs: 0 confirmed. 1 flagged as unconfirmed/needs reproduction — see note below. Note for maintainers: Wallet connection reverted to disconnected shortly after a successful Sign in with Farcaster, with no clear trigger identified in this pass (possibly in-app browser session behaviour in Warpcast rather than the app itself — not filing as a confirmed bug). Recommend a follow-up QA pass focused specifically on wallet-connection persistence, since it's currently the blocker preventing redirect-handling verification for this PR. |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Following original implementation with finalization fixes and QA review.
currently resolved: