P-1114 Add Formo + Turnkey embedded wallet integration example#7
P-1114 Add Formo + Turnkey embedded wallet integration example#7
Conversation
Adds a new example demonstrating Formo Analytics SDK integration with Turnkey embedded wallets. Includes a custom wagmi connector wrapping @turnkey/viem for signing, TurnkeyProvider for passkey auth, and FormoAnalyticsProvider for automatic wallet event tracking. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
yosriady
changed the title
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new example application that demonstrates the powerful synergy between Formo Analytics and Turnkey embedded wallets. It provides a practical, working model for developers to understand how to integrate passkey-based authentication with web3 wallet interactions, ensuring that all relevant wallet activities are automatically captured for analytics purposes. The example highlights a robust architecture for modern web3 applications, focusing on user experience and data insights. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive example application for integrating Formo Analytics with Turnkey embedded wallets. The code is well-structured and provides a clear demonstration of the integration. I've identified a couple of minor issues on the main page: an unused variable and an incorrect GitHub link in the footer. My review includes suggestions to address these points for improved code quality and correctness.
with-turnkey/src/app/page.tsx
Outdated
| }); | ||
|
|
||
| // Register the connector with wagmi and connect | ||
| const augmentedConfig = wagmiConfig; |
There was a problem hiding this comment.
The augmentedConfig variable is declared but is not used anywhere in the function. It can be removed to improve code clarity.
with-turnkey/src/app/page.tsx
Outdated
| </p> | ||
| <p> | ||
| <a | ||
| href="https://github.com/getformo/formo-example-turnkey" |
There was a problem hiding this comment.
The "View on GitHub" link points to a non-existent repository. Since this example is part of the getformo/examples repository, the link should be updated to point to the correct directory within that repository.
| href="https://github.com/getformo/formo-example-turnkey" | |
| href="https://github.com/getformo/examples/tree/main/with-turnkey" |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| async getProvider(): Promise<EIP1193Provider> { | ||
| // Return a minimal EIP-1193 provider that delegates to viem | ||
| // For most wagmi usage the connector methods above are sufficient | ||
| return undefined as unknown as EIP1193Provider; | ||
| }, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Implement proper EIP-1193 provider in turnkey-connector.ts that handles personal_sign, eth_signTypedData_v4, eth_sendTransaction, and forwards unknown RPC calls to the chain's public endpoint - Remove unused augmentedConfig variable and wagmiConfig import - Fix GitHub link to point to getformo/examples/tree/main/with-turnkey https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Update @turnkey/sdk-browser to ^5.15.2, @turnkey/sdk-react to ^5.5.6, @turnkey/viem to ^0.14.26 (^3.0.0 doesn't exist on npm) - Replace getCurrentUser() with getSession() (v5 API change) - Replace getActiveClient() with client from useTurnkey() (v5 API change) - Fix wagmi v3 connect() withCapabilities type compatibility - Fix viem TransactionSerializable union type compatibility https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The provider returned by getProvider() was missing on(), removeListener(), and emit() methods required by the EIP-1193 standard. Wagmi calls these to subscribe to accountsChanged and other provider events, causing a TypeError at runtime. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Previously each getProvider() call created a new provider with fresh event listeners, causing wagmi's event subscriptions to be silently lost. Now the provider and its listeners map are cached at the connector level and cleared on disconnect. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
Rewrites the connector to use Turnkey's official EIP-1193 provider package, following the approach from docs.turnkey.com/wallets/wagmi. This eliminates ~150 lines of hand-rolled provider code (signing, transaction building, gas estimation, nonce management, RPC forwarding, event emitter) and replaces it with the battle-tested @turnkey/eip-1193-provider which handles all of this out of the box. Changes: - Replace @turnkey/viem with @turnkey/eip-1193-provider - Connector now accepts walletId instead of signWith address - Add switchChain support via wallet_switchEthereumChain - Remove manual wallet account fetching from page.tsx - Update README to reference the official Turnkey wagmi docs https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The fire-and-forget connect() mutate function doesn't throw on failure, so the existing try/catch never caught wagmi connection errors and setTurnkeyUser ran before connection was confirmed. Switching to connectAsync ensures errors propagate to the catch block and setTurnkeyUser only runs on successful connection. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Formo's autocapture (enabled in providers.tsx) already monitors wagmi state changes and fires disconnect events. The manual useEffect bridge watching isConnected caused duplicate disconnect events. Removed the bridge and the now-unused prevAuthRef/useRef import. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive example application for integrating Formo with Turnkey embedded wallets. The code is well-structured and includes good documentation. My review identifies a critical bug in the custom turnkeyConnector where it fails to correctly extract the EIP-1193 provider, which would prevent the connector from functioning. I've also included suggestions to improve the display of wallet balances to avoid precision loss, to refactor the main page component for better maintainability, and to reconsider the use of force-dynamic which has performance implications. Addressing these points will make the example more robust and a better reference for developers.
| const provider = await createEIP1193Provider({ | ||
| walletId: walletId as UUID, | ||
| organizationId: organizationId as UUID, | ||
| turnkeyClient: client, | ||
| chains, | ||
| }); | ||
|
|
||
| cachedProvider = provider as unknown as EIP1193Provider; | ||
| return cachedProvider; |
There was a problem hiding this comment.
The createEIP1193Provider function from Turnkey returns an object containing the provider on the eip1193Provider property, not the provider itself. The current code incorrectly assigns the entire returned object to cachedProvider. This will cause runtime errors when methods like request are called on it. You should destructure eip1193Provider from the result, as shown in the official Turnkey documentation.
| const provider = await createEIP1193Provider({ | |
| walletId: walletId as UUID, | |
| organizationId: organizationId as UUID, | |
| turnkeyClient: client, | |
| chains, | |
| }); | |
| cachedProvider = provider as unknown as EIP1193Provider; | |
| return cachedProvider; | |
| const { eip1193Provider } = await createEIP1193Provider({ | |
| walletId: walletId as UUID, | |
| organizationId: organizationId as UUID, | |
| turnkeyClient: client, | |
| chains, | |
| }); | |
| cachedProvider = eip1193Provider as unknown as EIP1193Provider; | |
| return cachedProvider; |
with-turnkey/src/app/layout.tsx
Outdated
| import { Providers } from "./providers"; | ||
| import "./globals.css"; | ||
|
|
||
| export const dynamic = "force-dynamic"; |
There was a problem hiding this comment.
Using export const dynamic = "force-dynamic"; opts the entire route out of caching and forces it to be dynamically rendered on the server for every request. While this might be acceptable for a demo, it has significant performance implications for a production application. Since the main page is a client component ("use client" in page.tsx), the primary interactions are client-side. You might not need to disable all server-side caching so aggressively. Please consider if this is truly necessary or if a more granular caching strategy (or removing this line to use Next.js defaults) would be more appropriate for a real-world scenario this example might inspire.
|
|
||
| import { turnkeyConnector } from "@/config/turnkey-connector"; | ||
|
|
||
| export default function Home() { |
There was a problem hiding this comment.
This Home component is over 400 lines long, handling state, logic, and rendering for multiple distinct sections (authentication, wallet status, analytics actions, etc.). For better maintainability and reusability, consider breaking it down into smaller, more focused components. For example, WalletConnection, WalletStatus, and AnalyticsActions could each be their own component. While this is an example, demonstrating good component structure would be beneficial for developers using this as a reference.
with-turnkey/src/app/page.tsx
Outdated
| ? `${parseFloat( | ||
| formatUnits(balance.value, balance.decimals) | ||
| ).toFixed(4)} ${balance.symbol}` |
There was a problem hiding this comment.
Using parseFloat on values representing cryptocurrency amounts is risky as it can lead to loss of precision due to floating-point arithmetic limitations. viem's formatUnits correctly returns a string to maintain precision. It's safer to perform string manipulation to truncate the value for display purposes, rather than converting to a floating-point number.
For example:
const formatted = formatUnits(balance.value, balance.decimals);
const dotIndex = formatted.indexOf('.');
const displayValue = dotIndex === -1 ? formatted : formatted.slice(0, dotIndex + 5); // 1 for dot, 4 for decimals
// ... then use displayValue- Use string slicing instead of parseFloat for balance display to avoid floating-point precision loss with cryptocurrency amounts - Remove force-dynamic from layout.tsx since client components don't need server-side dynamic rendering forced on every request https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive example for integrating Formo with Turnkey embedded wallets. The code is well-structured and the example is feature-rich. I've found a critical issue regarding a server-side module being imported in client code, which will cause a runtime error. I've also included a couple of medium-severity suggestions to improve documentation accuracy and code readability. Overall, great work on this example application.
| import { getAddress, type Address, type EIP1193Provider } from "viem"; | ||
| import type { TurnkeyBrowserClient } from "@turnkey/sdk-browser"; | ||
| import type { AddEthereumChainParameter } from "viem"; | ||
| import type { UUID } from "crypto"; |
There was a problem hiding this comment.
The crypto module is a Node.js built-in and is not available in the browser. Importing UUID from it will cause a runtime error in this client-side code (due to 'use client'). To resolve this, you can replace the import with a local type alias for UUID, as the underlying type expected by the Turnkey SDK is a string.
| import type { UUID } from "crypto"; | |
| type UUID = string; |
with-turnkey/src/app/page.tsx
Outdated
| ? `${(() => { | ||
| const formatted = formatUnits(balance.value, balance.decimals); | ||
| const dot = formatted.indexOf("."); | ||
| return dot === -1 ? formatted : formatted.slice(0, dot + 5); | ||
| })()} ${balance.symbol}` |
There was a problem hiding this comment.
This immediately-invoked function expression (IIFE) for formatting the balance adds complexity within the JSX, making it harder to read and maintain. It's better to extract this logic into a helper function for improved clarity and separation of concerns.
For example, you could define a helper function:
const formatDisplayBalance = (balance: { value: bigint; decimals: number; }) => {
const formatted = formatUnits(balance.value, balance.decimals);
const dotIndex = formatted.indexOf('.');
// Truncate to 4 decimal places without rounding
if (dotIndex === -1) {
return formatted;
}
return formatted.slice(0, dotIndex + 5);
};And then use it in your component like this:
{balance ? `${formatDisplayBalance(balance)} ${balance.symbol}` : "Loading..."}
with-turnkey/README.md
Outdated
| - `wagmiConfig.subscribe()` for wallet connect/disconnect/chain events | ||
| - `queryClient.getMutationCache().subscribe()` for signature and transaction events | ||
|
|
||
| **Note:** Turnkey's logout doesn't trigger wagmi's disconnect status, so the demo includes a `useEffect` that manually tracks disconnect events when the connection state changes. |
There was a problem hiding this comment.
This note mentions a useEffect for manually tracking disconnect events when the connection state changes. However, the implementation in page.tsx only shows a handleDisconnect function that is manually triggered by a button click. This part of the documentation seems to be inconsistent with the code. Please update the README to accurately describe the implemented disconnect behavior.
…ADME
- Replace `import type { UUID } from "crypto"` with a local type alias
to avoid importing from a Node.js built-in in client-side code
- Extract IIFE balance formatting into a named helper function
- Remove stale README note about manual disconnect useEffect bridge
https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive example for integrating Formo with Turnkey embedded wallets. The implementation is well-structured, with a clear separation of concerns between UI, configuration, and the custom wagmi connector. The code is generally of high quality. I've pointed out a couple of areas for improvement in page.tsx related to handling side effects and ensuring robust state management for user sessions.
| return dot === -1 ? formatted : formatted.slice(0, dot + 5); | ||
| } | ||
|
|
||
| export default function Home() { |
There was a problem hiding this comment.
The pull request description mentions: "Includes a useEffect hook to manually track Turnkey logout events since Turnkey's logout doesn't trigger wagmi's disconnect status". However, this functionality does not appear to be implemented in the current code. Without it, the application's state could become inconsistent if a user logs out of Turnkey through other means (e.g., another browser tab). Consider implementing a mechanism, perhaps using useEffect to poll turnkey.getSession(), to detect external logout events and synchronize the application state.
with-turnkey/src/app/page.tsx
Outdated
| }); | ||
|
|
||
| setCustomEventSent(true); | ||
| setTimeout(() => setCustomEventSent(false), 2000); |
There was a problem hiding this comment.
Using setTimeout without a cleanup function can lead to memory leaks and attempts to set state on an unmounted component if the component unmounts before the timeout completes. It's a best practice to manage this kind of side effect within a useEffect hook, which provides a cleanup mechanism.
Consider removing this line and adding a useEffect hook to manage the state reset for customEventSent:
useEffect(() => {
if (customEventSent) {
const timer = setTimeout(() => {
setCustomEventSent(false);
}, 2000);
return () => clearTimeout(timer);
}
}, [customEventSent]);The bare setTimeout in handleTrackCustomEvent could attempt to set state on an unmounted component. Moving it to a useEffect with a cleanup function ensures the timer is cleared if the component unmounts. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
|
/gemini review |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Without cleanup, if the Turnkey provider is kept alive by internal SDK references, old listeners could fire and interfere with subsequent connections via the shared wagmi config emitter. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive example application for integrating Formo with Turnkey embedded wallets. The code is well-structured and provides a clear demonstration of the integration. I've added a few suggestions to improve the documentation clarity and developer experience for those running the example.
with-turnkey/README.md
Outdated
| pnpm install | ||
| # or | ||
| npm install |
There was a problem hiding this comment.
The package.json file specifies pnpm as the package manager. Suggesting npm as an alternative can be confusing as it will produce warnings. It's better to stick to pnpm for consistency. The same applies to the npm run dev suggestion below.
| pnpm install | |
| # or | |
| npm install | |
| pnpm install |
with-turnkey/README.md
Outdated
| pnpm dev | ||
| # or | ||
| npm run dev |
There was a problem hiding this comment.
The package.json file specifies pnpm as the package manager. To avoid confusion and warnings from using npm, it's best to only document the pnpm command.
| pnpm dev | |
| # or | |
| npm run dev | |
| pnpm dev |
| }; | ||
|
|
||
| export function Providers({ children }: { children: ReactNode }) { | ||
| const formoWriteKey = process.env.NEXT_PUBLIC_FORMO_WRITE_KEY; |
There was a problem hiding this comment.
While the app correctly handles a missing formoWriteKey by not rendering FormoAnalyticsProvider, it does so silently. For a better developer experience in this example app, consider adding a console.warn if formoWriteKey is not set to inform the developer that analytics are disabled. This would be consistent with the explicit error handling for the missing Turnkey Organization ID.
For example:
if (!formoWriteKey) {
console.warn("Missing NEXT_PUBLIC_FORMO_WRITE_KEY. Formo analytics will be disabled.");
}- Remove npm install/dev alternatives from README since package.json specifies pnpm as the package manager - Add console.warn when NEXT_PUBLIC_FORMO_WRITE_KEY is missing so developers know analytics are disabled https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive example application for integrating Formo Analytics with Turnkey embedded wallets. The implementation is well-structured, covering passkey authentication, a custom wagmi connector, and both automatic and manual event tracking. My review focuses on improving code maintainability and consistency by refining how configuration is handled and ensuring documentation accuracy. The changes I've suggested will make the example more robust and easier for other developers to follow.
with-turnkey/README.md
Outdated
|
|
||
| ## Prerequisites | ||
|
|
||
| - Node.js 18+ and pnpm 9.15+ |
There was a problem hiding this comment.
The package.json specifies a fixed version of pnpm (pnpm@9.15.4). For consistency and to ensure a reproducible setup, it would be better to specify the exact version in the prerequisites instead of a range.
| - Node.js 18+ and pnpm 9.15+ | |
| - Node.js 18+ and pnpm 9.15.4 |
with-turnkey/src/app/page.tsx
Outdated
| const organizationId = | ||
| process.env.NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID ?? ""; |
There was a problem hiding this comment.
Instead of accessing process.env.NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID directly within the component, it's better to use the organizationId from the turnkey object provided by the useTurnkey hook. This improves component encapsulation and relies on the configuration provided by the TurnkeyProvider, making the code more maintainable. Since you've already confirmed the turnkey object exists when you check for session, you can safely access turnkey.organizationId.
| const organizationId = | |
| process.env.NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID ?? ""; | |
| const organizationId = turnkey.organizationId; |
with-turnkey/src/app/providers.tsx
Outdated
| const turnkeyConfig = { | ||
| apiBaseUrl: "https://api.turnkey.com", | ||
| defaultOrganizationId: process.env.NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID ?? "", | ||
| rpId: process.env.NEXT_PUBLIC_TURNKEY_RP_ID ?? "localhost", | ||
| iframeUrl: "https://auth.turnkey.com", | ||
| serverSignUrl: process.env.NEXT_PUBLIC_TURNKEY_SERVER_SIGN_URL, | ||
| }; | ||
|
|
||
| export function Providers({ children }: { children: ReactNode }) { | ||
| const formoWriteKey = process.env.NEXT_PUBLIC_FORMO_WRITE_KEY; | ||
| const [queryClient] = useState(() => new QueryClient()); |
There was a problem hiding this comment.
The turnkeyConfig object is defined at the module level and depends on process.env. While this works in Next.js with client components, it's a better practice to define such configuration objects within the component body. This ensures that the configuration is created in the client-side rendering context and avoids potential issues with server-side rendering or bundlers in different environments. I'd suggest moving it inside the Providers component.
| const turnkeyConfig = { | |
| apiBaseUrl: "https://api.turnkey.com", | |
| defaultOrganizationId: process.env.NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID ?? "", | |
| rpId: process.env.NEXT_PUBLIC_TURNKEY_RP_ID ?? "localhost", | |
| iframeUrl: "https://auth.turnkey.com", | |
| serverSignUrl: process.env.NEXT_PUBLIC_TURNKEY_SERVER_SIGN_URL, | |
| }; | |
| export function Providers({ children }: { children: ReactNode }) { | |
| const formoWriteKey = process.env.NEXT_PUBLIC_FORMO_WRITE_KEY; | |
| const [queryClient] = useState(() => new QueryClient()); | |
| export function Providers({ children }: { children: ReactNode }) { | |
| const turnkeyConfig = { | |
| apiBaseUrl: "https://api.turnkey.com", | |
| defaultOrganizationId: process.env.NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID ?? "", | |
| rpId: process.env.NEXT_PUBLIC_TURNKEY_RP_ID ?? "localhost", | |
| iframeUrl: "https://auth.turnkey.com", | |
| serverSignUrl: process.env.NEXT_PUBLIC_TURNKEY_SERVER_SIGN_URL, | |
| }; | |
| const formoWriteKey = process.env.NEXT_PUBLIC_FORMO_WRITE_KEY; | |
| const [queryClient] = useState(() => new QueryClient()); |
- Fix pnpm version in README to exact 9.15.4 instead of range - Use turnkey.config.defaultOrganizationId instead of direct env var access - Move turnkeyConfig inside Providers component for proper client-side context https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
Previously the login handler only checked for an existing Turnkey session and showed an error if none was found. Now it uses passkeyClient.login() to initiate a passkey-based session, then proceeds with wallet connection. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
New users can now create a sub-organization with a passkey and embedded wallet directly from the demo app. The flow prompts WebAuthn to create a passkey, creates a Turnkey sub-org with an Ethereum wallet, then logs in and connects automatically. Existing users use 'Log In with Passkey'. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
- Add email input field for account creation (Turnkey requires a valid email for root users) - Store sub-org ID in localStorage after account creation so subsequent logins can scope the passkey auth to the correct sub-organization - Use session.organizationId instead of the parent org config for wallet fetching, supporting both parent-org and sub-org users https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The handleCreateAccount callback captured the initial empty signupEmail value because it was missing from the dependency array. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
The client-side passkeyClient.createSubOrganization() fails because it stamps the request with a WebAuthn assertion, but the newly created passkey isn't registered with Turnkey yet (chicken-and-egg problem). Fix by adding a /api/create-sub-org Next.js API route that uses @turnkey/sdk-server with API key authentication to create the sub-org. The client now sends the passkey attestation to this route, which creates the sub-org server-side without needing WebAuthn auth. Requires TURNKEY_API_PUBLIC_KEY and TURNKEY_API_PRIVATE_KEY env vars. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
Replace the wagmi-based architecture with direct EIP-1193 provider usage: - Remove wagmi, @tanstack/react-query dependencies - Remove custom turnkey-connector.ts and wagmi.ts config - Use createEIP1193Provider from @turnkey/eip-1193-provider directly - Pass the EIP-1193 provider to Formo SDK for autocapture (Formo wraps provider.request() to track signatures, transactions) - Manage wallet state (address, chainId, balance) via React context - Call provider.request() directly for sign/send actions Page bundle size reduced from 60.9kB to 31.6kB. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…Id overwritten with null After passkeyClient.login(), the turnkeyClient from useTurnkey() is still undefined because React hasn't re-rendered. Fix by creating a fresh client via turnkey.indexedDbClient() when the hook value is stale. Also fix both handlePasskeyLogin and handleCreateAccount overwriting the chainId (correctly set by connectWallet) with the stale walletState.chainId (null) from the closure. https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new example application for integrating Formo with Turnkey embedded wallets. The implementation is comprehensive, covering authentication, wallet interactions, and analytics tracking. My review focuses on several key areas for improvement:
- Documentation Accuracy: The README file contains significant discrepancies regarding the use of
wagmi, which appears to be absent from the actual implementation. Aligning the documentation with the code is critical for user understanding. - Code Structure: The main page component is very large and would benefit from being broken down into smaller, more manageable components to improve readability and maintainability.
- Type Safety: There are several instances of
as anytype assertions, which bypass TypeScript's type checking. These should be resolved to ensure type safety and prevent potential runtime errors. - Best Practices: I've also suggested minor improvements, such as using utility functions from existing dependencies and safer handling of environment variables.
with-turnkey/README.md
Outdated
|
|
||
| - **Turnkey Authentication**: Login with passkeys to access embedded wallets — no browser extensions needed | ||
| - **Formo Analytics Integration**: Track wallet events and custom analytics | ||
| - **Wagmi Integration**: Custom wagmi connector wrapping Turnkey's `@turnkey/eip-1193-provider` |
There was a problem hiding this comment.
The documentation repeatedly mentions wagmi integration, but the project does not seem to use wagmi. The package.json doesn't include it as a dependency, and the implementation in page.tsx uses @turnkey/eip-1193-provider directly rather than through wagmi hooks. Key files mentioned, such as src/config/turnkey-connector.ts and src/config/wagmi.ts, are also missing from the pull request. Please either add the wagmi integration as described or update the documentation to reflect the current implementation to avoid confusion.
| ```env | ||
| NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID=your-turnkey-organization-id | ||
| NEXT_PUBLIC_FORMO_WRITE_KEY=your-formo-write-key | ||
| ``` |
There was a problem hiding this comment.
The environment variable setup instructions are incomplete. The create-sub-org functionality requires server-side Turnkey API keys (TURNKEY_API_PUBLIC_KEY and TURNKEY_API_PRIVATE_KEY), but they are not mentioned here. Please add them to this section to ensure users can set up the example correctly.
Here is the complete set of variables that should be in the .env file:
NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID=your-turnkey-organization-id
TURNKEY_API_PUBLIC_KEY=your-turnkey-api-public-key
TURNKEY_API_PRIVATE_KEY=your-turnkey-api-private-key
NEXT_PUBLIC_FORMO_WRITE_KEY=your-formo-write-key| const turnkeyServer = new Turnkey({ | ||
| apiBaseUrl: "https://api.turnkey.com", | ||
| apiPrivateKey: process.env.TURNKEY_API_PRIVATE_KEY!, | ||
| apiPublicKey: process.env.TURNKEY_API_PUBLIC_KEY!, | ||
| defaultOrganizationId: process.env.NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID!, | ||
| }); |
There was a problem hiding this comment.
Using the non-null assertion operator (!) for environment variables can hide configuration issues and lead to runtime errors. It's safer to validate these variables when the application starts and provide a clear error message if they are missing. This ensures the server fails fast with a descriptive error rather than at an unpredictable point during a request.
const apiPrivateKey = process.env.TURNKEY_API_PRIVATE_KEY;
const apiPublicKey = process.env.TURNKEY_API_PUBLIC_KEY;
const defaultOrganizationId = process.env.NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID;
if (!apiPrivateKey || !apiPublicKey || !defaultOrganizationId) {
throw new Error(
'Missing required Turnkey server environment variables. Please check your .env file.'
);
}
const turnkeyServer = new Turnkey({
apiBaseUrl: 'https://api.turnkey.com',
apiPrivateKey,
apiPublicKey,
defaultOrganizationId,
});| return dot === -1 ? formatted : formatted.slice(0, dot + 5); | ||
| } | ||
|
|
||
| export default function Home() { |
There was a problem hiding this comment.
The Home component is very large (nearly 700 lines), which can make it difficult to read, maintain, and test. Consider refactoring it by extracting UI sections into smaller, dedicated components. For example, you could create components for WalletConnectionCard, WalletStatusCard, FormoAnalyticsActionsCard, etc. This would improve modularity and code organization.
| method: "eth_chainId", | ||
| })) as string; | ||
|
|
||
| setProvider(eip1193 as any); |
There was a problem hiding this comment.
Using as any here suppresses type checking and can hide potential bugs. It seems there might be a type incompatibility between the provider returned by @turnkey/eip-1193-provider and the EIP1193Provider type from viem used in your WalletContext. Please investigate the type mismatch and use a proper type assertion or an adapter function instead of as any.
| setProvider(eip1193 as any); | |
| setProvider(eip1193); |
with-turnkey/src/app/page.tsx
Outdated
| if (!provider || !address) return; | ||
| setIsSignPending(true); | ||
| try { | ||
| await (provider as any).request({ |
There was a problem hiding this comment.
The as any cast on the provider should be avoided as it bypasses type safety and may hide issues. The provider state is guarded against being null, so the issue is likely with its type definition not satisfying the request method's signature as expected by TypeScript. Please resolve the underlying type issue to remove this cast.
with-turnkey/src/app/page.tsx
Outdated
| await (provider as any).request({ | ||
| method: "personal_sign", | ||
| params: [ | ||
| ("0x" + Buffer.from("Hello from Formo + Turnkey Demo!").toString("hex")) as `0x${string}`, |
There was a problem hiding this comment.
Instead of using Buffer to convert a string to its hex representation, you can use the toHex utility from viem, which is already a dependency. This will make the code cleaner and more consistent with the project's tooling.
You'll need to import it first: import { toHex } from 'viem';
| ("0x" + Buffer.from("Hello from Formo + Turnkey Demo!").toString("hex")) as `0x${string}`, | |
| toHex("Hello from Formo + Turnkey Demo!"), |
with-turnkey/src/app/providers.tsx
Outdated
| // Formo options: when a provider is available, pass it for EIP-1193 autocapture. | ||
| // When provider changes from null -> value, the optionsKey changes and Formo reinitializes. | ||
| const formoOptions = { | ||
| ...(provider ? { provider: provider as any } : {}), |
There was a problem hiding this comment.
Casting provider to any here hides a potential type mismatch between the viem provider type and what the Formo SDK expects. This should be resolved by ensuring the types are compatible or by using a proper type guard or adapter, rather than suppressing the type error with as any.
| ...(provider ? { provider: provider as any } : {}), | |
| ...(provider ? { provider } : {}), |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Remove all wagmi references from README since the project uses
@turnkey/eip-1193-provider directly, not wagmi
- Add server-side env vars (TURNKEY_API_PUBLIC_KEY, TURNKEY_API_PRIVATE_KEY)
to the README setup instructions
- Update project structure in README to match actual file layout
- Validate required Turnkey env vars at module load with a clear error
message instead of using non-null assertions
- Replace Buffer.from().toString("hex") with viem's toHex() utility
- Fix duplicate parseInt(chainIdHex, 16) in connectWallet
https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a well-structured example application demonstrating the integration of Formo Analytics with Turnkey embedded wallets. The code is generally clean and includes helpful documentation. My review focuses on a few areas for improvement: enhancing type safety by addressing several as any type assertions, increasing the robustness of unique name generation in an API route, and clarifying parts of the documentation to ensure a smoother setup experience for developers.
| ```env | ||
| NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID=your-turnkey-organization-id | ||
| TURNKEY_API_PUBLIC_KEY=your-turnkey-api-public-key | ||
| TURNKEY_API_PRIVATE_KEY=your-turnkey-api-private-key | ||
| NEXT_PUBLIC_FORMO_WRITE_KEY=your-formo-write-key | ||
| ``` |
There was a problem hiding this comment.
The environment variable example in the Quick Start section is missing NEXT_PUBLIC_TURNKEY_RP_ID, which is present in the .env.example file. This inconsistency could lead to confusion and setup issues for users following the guide. Please update the documentation to include it.
| ```env | |
| NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID=your-turnkey-organization-id | |
| TURNKEY_API_PUBLIC_KEY=your-turnkey-api-public-key | |
| TURNKEY_API_PRIVATE_KEY=your-turnkey-api-private-key | |
| NEXT_PUBLIC_FORMO_WRITE_KEY=your-formo-write-key | |
| ``` | |
| ```env | |
| NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID=your-turnkey-organization-id | |
| NEXT_PUBLIC_TURNKEY_RP_ID=localhost | |
| TURNKEY_API_PUBLIC_KEY=your-turnkey-api-public-key | |
| TURNKEY_API_PRIVATE_KEY=your-turnkey-api-private-key | |
| NEXT_PUBLIC_FORMO_WRITE_KEY=your-formo-write-key |
with-turnkey/README.md
Outdated
|
|
||
| ## Supported Chains | ||
|
|
||
| The app is configured for Sepolia (testnet). Additional chains can be added by modifying the `chainParam` configuration in `page.tsx`. |
There was a problem hiding this comment.
The statement that additional chains can be added just by modifying chainParam is an oversimplification. The application is currently hardcoded for a single network (Sepolia). Supporting multiple chains would require more substantial changes, such as implementing a chain-switching UI and dynamically configuring the EIP-1193 provider. It would be beneficial to clarify this to manage user expectations.
| const apiClient = turnkeyServer.apiClient(); | ||
|
|
||
| const result = await apiClient.createSubOrganization({ | ||
| subOrganizationName: `Sub-Org ${Date.now()}`, |
There was a problem hiding this comment.
Using Date.now() for generating a subOrganizationName is not guaranteed to be unique, especially if multiple requests are processed in the same millisecond. While this may be acceptable for a demo, appending a random string would make it more robust.
| subOrganizationName: `Sub-Org ${Date.now()}`, | |
| subOrganizationName: `Sub-Org ${Date.now()}-${Math.random().toString(36).slice(2)}`, |
|
|
||
| const parsedChainId = parseInt(chainIdHex, 16); | ||
|
|
||
| setProvider(eip1193 as any); |
There was a problem hiding this comment.
Casting eip1193 to any when calling setProvider weakens type safety. It would be better to investigate if there's a type mismatch between the provider from @turnkey/eip-1193-provider and the EIP1193Provider type from viem. If the types are compatible, the cast is unnecessary. If not, consider creating a type adapter to ensure type correctness.
with-turnkey/src/app/page.tsx
Outdated
| if (!provider || !address) return; | ||
| setIsSignPending(true); | ||
| try { | ||
| await (provider as any).request({ |
There was a problem hiding this comment.
The provider is unnecessarily cast to any here. The type is already narrowed to EIP1193Provider by the check on line 299, so the request method should be available with proper typing. This is also inconsistent with handleSendTransaction on line 321, which correctly calls provider.request() without a cast. Please remove the as any for better type safety and consistency.
| await (provider as any).request({ | |
| await provider.request({ |
| // When provider changes from null -> value, Formo reinitializes to attach autocapture. | ||
| const formoOptions = useMemo( | ||
| () => ({ | ||
| ...(provider ? { provider: provider as any } : {}), |
There was a problem hiding this comment.
The use of as any for the provider in formoOptions reduces type safety. While the provider option in @formo/analytics might be typed as any, it's a good practice to maintain strict typing within the application. Please verify if this cast is necessary and remove it if the EIP1193Provider type is compatible with what Formo's provider expects.
- Add missing NEXT_PUBLIC_TURNKEY_RP_ID to README Quick Start env example - Clarify Supported Chains section about multi-chain requirements - Add random suffix to sub-org names for better uniqueness - Remove unnecessary `as any` cast in handleSignMessage https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS
| const formoOptions = useMemo( | ||
| () => ({ | ||
| ...(provider ? { provider: provider as any } : {}), | ||
| autocapture: true, | ||
| tracking: true, | ||
| logger: { | ||
| enabled: true, | ||
| levels: ["info", "warn", "error"] as ("info" | "warn" | "error")[], | ||
| }, | ||
| }), | ||
| [provider] | ||
| ); |
There was a problem hiding this comment.
🟡 React Rules of Hooks violation: useMemo called after conditional early return
The formoOptions useMemo hook on line 81 is called after the conditional early return on lines 58–73. React requires that hooks are called in the same order on every render. When defaultOrganizationId is empty, the component returns early and only hooks 1–3 (the two useState calls and the first useMemo) execute. When it's non-empty, hook 4 (formoOptions useMemo) also executes. This violates React's rules of hooks.
In practice, process.env.NEXT_PUBLIC_TURNKEY_ORGANIZATION_ID is a Next.js build-time constant, so the condition is stable across renders and won't cause a runtime crash today. However, this pattern is fragile: any refactor that makes the condition dynamic would cause a hard-to-debug hooks-ordering crash. The sibling example with-privy/src/app/providers.tsx correctly places all hooks before conditional returns.
Prompt for agents
In with-turnkey/src/app/providers.tsx, move the `formoOptions` useMemo (lines 81-92) to BEFORE the conditional early return on line 58. All hooks (useState, useMemo) must be called unconditionally and in the same order on every render, per React's rules of hooks. The formoOptions useMemo should be placed right after the walletContextValue useMemo (after line 56), and before the `if (!turnkeyConfig.defaultOrganizationId)` check on line 58. This matches the pattern used in with-privy/src/app/providers.tsx where all hooks are called before any conditional returns.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary
This PR adds a complete example application demonstrating integration between Formo Analytics SDK and Turnkey embedded wallets. The example shows how to authenticate users with passkeys, connect to embedded wallets via wagmi, and automatically track wallet events with Formo.
Key Changes
Main demo page (
src/app/page.tsx): Full-featured UI with wallet connection, status display, and event testing capabilitiesCustom wagmi connector (
src/config/turnkey-connector.ts): Wraps Turnkey's@turnkey/viemto integrate with wagmiuseAccount,useSignMessage, etc.)Provider setup (
src/app/providers.tsx): Configures provider nesting orderConfiguration files:
src/config/wagmi.ts: Wagmi config with support for Ethereum, Sepolia, Polygon, Arbitrum, Optimism, and Basesrc/app/layout.tsx: Root layout with metadatasrc/app/globals.css: Tailwind CSS setupnext.config.ts: Next.js configurationpostcss.config.mjs: PostCSS configurationtsconfig.json: TypeScript configurationpackage.json: Dependencies and scripts.env.example: Environment variable templateDocumentation (
README.md): Comprehensive guide covering setup, features, project structure, and integration detailsNotable Implementation Details
autocapture: trueuseEffecthook to manually track Turnkey logout events since Turnkey's logout doesn't trigger wagmi's disconnect status@ts-ignorecomments for@turnkey/sdk-reactwhich is built for React 18https://claude.ai/code/session_012cKNjKPGBnVZQwNDAL3iKS