WIP: feat(clerk-js,react,shared): Add sign in with Solana functionality to clerk-js ui components #7293
Conversation
Signed-off-by: Kenton Duprey <kenton@clerk.dev>
Core Changes - Add `AuthenticateWithSolanaParams` type for Solana-specific authentication - Extend `authenticateWithWeb3` to support Solana wallets with `walletName` parameter - Add `authenticateWithSolana` method to `SignInResource` for dedicated Solana auth - Require `walletName` for Solana provider to enable multi-wallet selection - Split Ethereum and Solana provider detection into separate modules Updated types - Add `web3_solana_signature` to `Web3Strategy` union type - Add `walletName` field to `Web3SignatureFactor` type - Update `AuthenticateWithWeb3Params` to include optional `walletName` - Removed unused Wallet Standard types in `web3.ts` (moved to library imports) Dependencies - Add `@solana/wallet-standard@^1.1.4` for Solana wallet types - Add `@wallet-standard/core@^1.1.1` for wallet registry - Add `@wallet-standard/react@^1.0.1` for React integration Signed-off-by: Kenton Duprey <kenton@clerk.dev>
… selectable wallet support - Adds `web3_solana_signature` strategy for Sign In and Sign Up, introduces a Solana wallet selection route (`choose-wallet`), and integrates Solana wallet-standard APIs for identifier and signature generation. - Add `@solana/wallet-standard`, `@solana/wallet-adapter-react`, `@solana/wallet-adapter-base`, `bs58` libs - Implement `injectedWeb3SolanaProviders.ts` singleton with feature/type guards within `solana:signMessage`, connect flow - Update `web3.ts` to fetch Solana identifier via `StandardConnect` and sign messages via `SolanaSignMessage`. - Add `authenticateWithSolana` to `SignIn`, `SignUp`, and expose through `IsomorphicClerk` & `Clerk` interface. - Extend `SignInResource`, `SignUpResource`, introduce `SignUpAuthenticateWithSolanaParams`, thread optional `walletName`. - Support `web3_solana_signature` in sign-in and sign-up start machines. - Add `SignInFactorOneSolanaWalletsCard` component and `choose-wallet` route; extend `FlowMetadata` with `choose-wallet`. - Add `getSolanaIdentifier`, `generateSignatureWithSolana`, propagate `walletName` into web3 auth flow - if named wallet not found, attempt `window.solana` before failing gracefully. Signed-off-by: Kenton Duprey <kenton@clerk.dev>
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds Solana wallet authentication: new runtime dependencies, Solana-aware web3 utilities and providers, public methods for authenticateWithSolana, UI wallet-selection components/routes for sign-in/sign-up, and shared type updates to propagate walletName. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Clerk UI
participant Core as Clerk Core
participant Detect as Wallet Detector
participant Wallet as Solana Wallet
participant Backend as Clerk Backend
User->>UI: choose Solana auth
UI->>Core: authenticateWithSolana()
Core->>UI: navigate to choose-wallet
UI->>Detect: listInstalledSolanaWallets()
Detect-->>UI: available wallets
User->>UI: select wallet (walletName)
UI->>Core: getSolanaIdentifier(walletName)
Core->>Wallet: connect() / request public key
Wallet-->>Core: public key
Core->>Wallet: signMessage(nonce)
Wallet-->>Core: signature (bs58)
Core->>Backend: authenticateWithWeb3(identifier, signature, strategy=web3_solana_signature)
Backend-->>Core: auth result / session
Core-->>UI: complete / redirect
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/types/signInFuture.ts (1)
243-256: Consider using a discriminated union for stronger type safety.The JSDoc states that
walletNameis required whenprovideris'solana', but TypeScript doesn't enforce this constraint. This could lead to runtime errors if developers forget to providewalletNamefor Solana authentication.Consider refactoring to a discriminated union type to enforce this at compile time:
export type SignInFutureWeb3Params = | { strategy: Web3Strategy; provider: Exclude<Web3Provider, 'solana'>; walletName?: never; } | { strategy: Extract<Web3Strategy, 'web3_solana_signature'>; provider: 'solana'; walletName: string; };This approach ensures type safety and prevents runtime issues.
🧹 Nitpick comments (7)
packages/clerk-js/src/ui/components/SignIn/SignInSocialButtons.tsx (1)
61-63: LGTM! Solana flow correctly routes to wallet selection.The early return for the Solana strategy correctly navigates to the wallet selection route before authentication, which aligns with the PR's objectives for Solana wallet detection and selection.
Optional: Extract magic strings to constants.
Consider defining constants for the strategy and route strings to improve maintainability:
const WEB3_SOLANA_STRATEGY = 'web3_solana_signature'; const CHOOSE_WALLET_ROUTE = 'choose-wallet';Then use them:
-if (strategy === 'web3_solana_signature') { - return navigate(`choose-wallet?strategy=${strategy}`); +if (strategy === WEB3_SOLANA_STRATEGY) { + return navigate(`${CHOOSE_WALLET_ROUTE}?strategy=${strategy}`);packages/shared/src/types/clerk.ts (1)
872-875: Consider enhancing the JSDoc comment.The JSDoc for
authenticateWithSolanacould be more descriptive. Consider documenting:
- The optional
walletNameparameter and when it should be provided- Return type details (e.g., what the promise resolves to)
- Example usage
Compare to other authentication methods for consistency in documentation style.
packages/clerk-js/src/core/resources/SignUp.ts (2)
271-320: Add a Solana-specific walletName guard inauthenticateWithWeb3For sign-up,
walletNameis effectively required whenstrategyisweb3_solana_signature(the Solana utils cannot resolve a provider without it), butauthenticateWithWeb3does not enforce this. If a caller bypassesauthenticateWithSolanaand invokes this method directly with a Solana strategy and nowalletName, you’ll end up with empty identifiers/signatures and a less obvious backend error.Consider failing fast for Solana here, similar to the SignIn flow:
): Promise<SignUpResource> => { const { generateSignature, identifier, unsafeMetadata, strategy = 'web3_metamask_signature', legalAccepted, walletName, } = params || {}; - const provider = strategy.replace('web3_', '').replace('_signature', '') as Web3Provider; + const provider = strategy.replace('web3_', '').replace('_signature', '') as Web3Provider; + + if (provider === 'solana' && !walletName) { + clerkMissingOptionError('walletName'); + }
382-397: Fail fast on missingwalletNameinauthenticateWithSolana
getSolanaIdentifierandgenerateSignatureWithSolanaboth expect a concrete wallet selection; calling this method withoutwalletNamewill currently hit those utilities withundefined, produce console warnings, and then fail later in the flow.To make the API clearer and consistent with the SignIn/Future Solana paths, validate
walletNameat the entry point:public authenticateWithSolana = async ( params?: SignUpAuthenticateWithWeb3Params & { walletName?: string; legalAccepted?: boolean; }, ): Promise<SignUpResource> => { - const identifier = await getSolanaIdentifier({ walletName: params?.walletName }); + if (!params?.walletName) { + clerkMissingOptionError('walletName'); + } + + const identifier = await getSolanaIdentifier({ walletName: params.walletName }); return this.authenticateWithWeb3({ identifier, generateSignature: generateSignatureWithSolana, unsafeMetadata: params?.unsafeMetadata, strategy: 'web3_solana_signature', legalAccepted: params?.legalAccepted, walletName: params?.walletName, }); };packages/clerk-js/src/core/resources/SignIn.ts (1)
471-479: Avoid unnecessary Solana identifier lookup whenwalletNameis missing
authenticateWithSolanacurrently callsgetSolanaIdentifier({ walletName: params?.walletName })even whenwalletNameis absent, and only later willauthenticateWithWeb3throw due to the Solana walletName guard. To keep errors cleaner and avoid extra provider lookups, you can fail early here:public authenticateWithSolana = async (params?: AuthenticateWithSolanaParams): Promise<SignInResource> => { - const identifier = await getSolanaIdentifier({ walletName: params?.walletName }); + if (!params?.walletName) { + clerkMissingOptionError('walletName'); + } + + const identifier = await getSolanaIdentifier({ walletName: params.walletName }); return this.authenticateWithWeb3({ identifier, generateSignature: generateSignatureWithSolana, strategy: 'web3_solana_signature', walletName: params?.walletName, }); };packages/clerk-js/src/core/clerk.ts (1)
2267-2382: Add SolanawalletNameguard inauthenticateWithWeb3to mirror SignIn behavior
authenticateWithWeb3now:
- Derives
providerfromstrategy.- Calls
getWeb3Identifier({ provider, walletName }).- Chooses
generateSignatureWithSolanawhenprovider === 'solana'.- Forwards
walletNameinto bothsignIn.authenticateWithWeb3andsignUp.authenticateWithWeb3.That wiring is good, but when
strategyisweb3_solana_signatureandwalletNameis missing,getWeb3Identifierwill already be invoked with an undefined wallet name, causing a console warning and an empty identifier before SignIn/SignUp eventually throw.To provide a clearer error and avoid the extra Solana provider lookup, add a guard similar to the one in
SignIn.authenticateWithWeb3:public authenticateWithWeb3 = async ({ redirectUrl, signUpContinueUrl, customNavigate, unsafeMetadata, strategy, legalAccepted, secondFactorUrl, walletName, }: ClerkAuthenticateWithWeb3Params): Promise<void> => { @@ - const provider = strategy.replace('web3_', '').replace('_signature', '') as Web3Provider; - const identifier = await getWeb3Identifier({ provider, walletName }); + const provider = strategy.replace('web3_', '').replace('_signature', '') as Web3Provider; + + if (provider === 'solana' && !walletName) { + throw new ClerkRuntimeError('walletName is required for solana web3 authentication', { + code: 'missing_wallet_name', + }); + } + + const identifier = await getWeb3Identifier({ provider, walletName });This keeps the contract “walletName is required for Solana” enforced at the highest entry point and avoids surprising warnings downstream.
packages/shared/src/types/web3Wallet.ts (1)
28-49: Solana typings look sound; consider tightening the discriminated unionThe changes here (adding
walletName?: stringtoAuthenticateWithWeb3Params/GenerateSignatureParams, and introducingGenerateSolanaSignatureParamswithprovider: 'solana'and requiredwalletName) are structurally backward compatible and give Solana a more precise payload shape.If you want stronger compile-time guarantees, you could simplify
GenerateSignatureto a proper discriminated union keyed byproviderinstead ofGenerateSignatureParams | GenerateSolanaSignatureParams(which is effectively just the base type since the Solana variant extends it). This is purely a type-level nicety; the current form is fine functionally.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
packages/clerk-js/package.json(2 hunks)packages/clerk-js/src/core/clerk.ts(8 hunks)packages/clerk-js/src/core/resources/SignIn.ts(10 hunks)packages/clerk-js/src/core/resources/SignUp.ts(4 hunks)packages/clerk-js/src/ui/components/SignIn/SignInFactorOneSolanaWalletsCard.tsx(1 hunks)packages/clerk-js/src/ui/components/SignIn/SignInSocialButtons.tsx(1 hunks)packages/clerk-js/src/ui/components/SignIn/index.tsx(2 hunks)packages/clerk-js/src/ui/components/SignUp/SignUpSocialButtons.tsx(1 hunks)packages/clerk-js/src/ui/components/SignUp/SignUpStartSolanaWalletsCard.tsx(1 hunks)packages/clerk-js/src/ui/components/SignUp/index.tsx(2 hunks)packages/clerk-js/src/ui/elements/Web3WalletButtons.tsx(1 hunks)packages/clerk-js/src/ui/elements/contexts/index.tsx(1 hunks)packages/clerk-js/src/utils/injectedWeb3EthProviders.ts(3 hunks)packages/clerk-js/src/utils/injectedWeb3SolanaProviders.ts(1 hunks)packages/clerk-js/src/utils/web3.ts(4 hunks)packages/elements/src/internals/machines/sign-in/start.machine.ts(1 hunks)packages/elements/src/internals/machines/sign-up/start.machine.ts(1 hunks)packages/elements/src/react/hooks/use-third-party-provider.hook.ts(1 hunks)packages/react/src/isomorphicClerk.ts(2 hunks)packages/shared/src/types/clerk.ts(3 hunks)packages/shared/src/types/factors.ts(1 hunks)packages/shared/src/types/signIn.ts(2 hunks)packages/shared/src/types/signInFuture.ts(2 hunks)packages/shared/src/types/signUp.ts(2 hunks)packages/shared/src/types/signUpCommon.ts(1 hunks)packages/shared/src/types/web3.ts(1 hunks)packages/shared/src/types/web3Wallet.ts(1 hunks)packages/shared/src/web3.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
packages/shared/src/types/signIn.ts (1)
packages/shared/src/types/clerk.ts (1)
AuthenticateWithSolanaParams(2302-2309)
packages/shared/src/types/signUp.ts (1)
packages/shared/src/types/signUpCommon.ts (1)
SignUpAuthenticateWithSolanaParams(120-122)
packages/shared/src/types/web3Wallet.ts (2)
packages/shared/src/types/strategies.ts (1)
Web3Strategy(20-20)packages/shared/src/types/web3.ts (1)
Web3Provider(15-20)
packages/clerk-js/src/ui/components/SignIn/SignInFactorOneSolanaWalletsCard.tsx (7)
packages/clerk-js/src/ui/elements/contexts/index.tsx (2)
useCardState(42-70)withCardStateProvider(72-81)packages/clerk-js/src/ui/contexts/components/SignIn.ts (1)
useSignInContext(38-169)packages/clerk-js/src/ui/utils/errorHandler.ts (1)
handleError(64-86)packages/clerk-js/src/ui/customizables/Flow.tsx (1)
Flow(41-44)packages/clerk-js/src/ui/elements/Header.tsx (1)
Header(103-108)packages/clerk-js/src/ui/elements/Web3WalletButtons.tsx (1)
Web3WalletButtons(106-123)packages/clerk-js/src/ui/common/withRedirect.tsx (2)
withRedirectToSignInTask(93-113)withRedirectToAfterSignIn(55-72)
packages/clerk-js/src/ui/components/SignUp/index.tsx (1)
packages/clerk-js/src/ui/components/SignUp/SignUpStartSolanaWalletsCard.tsx (1)
SignUpStartSolanaWalletsCard(75-77)
packages/clerk-js/src/core/clerk.ts (2)
packages/shared/src/types/clerk.ts (1)
AuthenticateWithSolanaParams(2302-2309)packages/clerk-js/src/utils/web3.ts (2)
getWeb3Identifier(19-39)generateSignatureWithSolana(130-132)
packages/clerk-js/src/ui/components/SignIn/index.tsx (1)
packages/clerk-js/src/ui/components/SignIn/SignInFactorOneSolanaWalletsCard.tsx (1)
SignInFactorOneSolanaWalletsCard(70-72)
packages/clerk-js/src/ui/components/SignUp/SignUpStartSolanaWalletsCard.tsx (3)
packages/clerk-js/src/ui/elements/contexts/index.tsx (2)
useCardState(42-70)withCardStateProvider(72-81)packages/clerk-js/src/ui/contexts/components/SignUp.ts (1)
useSignUpContext(37-158)packages/clerk-js/src/ui/common/withRedirect.tsx (2)
withRedirectToSignUpTask(115-135)withRedirectToAfterSignUp(74-91)
packages/shared/src/types/signInFuture.ts (1)
packages/shared/src/types/web3.ts (1)
Web3Provider(15-20)
packages/clerk-js/src/utils/injectedWeb3EthProviders.ts (1)
packages/shared/src/types/web3.ts (2)
MetamaskWeb3Provider(9-9)OKXWalletWeb3Provider(11-11)
packages/clerk-js/src/ui/elements/Web3WalletButtons.tsx (2)
packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
useCardState(42-70)packages/clerk-js/src/ui/customizables/index.ts (3)
Grid(18-18)Flex(16-16)Image(30-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Packages
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (29)
packages/clerk-js/src/utils/injectedWeb3EthProviders.ts (4)
30-38: LGTM: Consistent renaming to disambiguate Ethereum providers.The type alias and class renaming from
InjectedWeb3ProvidertoInjectedWeb3EthProviderandInjectedWeb3ProviderstoInjectedWeb3EthProvidersis clear and consistent, properly disambiguating Ethereum providers from the new Solana providers being added in this PR.
48-53: LGTM: getInstance method correctly updated.All references to the renamed class are consistent throughout the singleton getInstance implementation.
55-55: LGTM: Method signature updated consistently.The
getmethod parameter type is correctly updated to useInjectedWeb3EthProvider.
75-75: All call sites have been correctly updated to use the new export name.Verification confirms zero remaining references to the old naming patterns (
getInjectedWeb3Providers,InjectedWeb3Providers, or incompatibleInjectedWeb3Providerreferences). The sole import site inpackages/clerk-js/src/utils/web3.tscorrectly uses the new name and the export is properly defined.packages/shared/src/types/factors.ts (1)
42-47: LGTM! Clean type extension for wallet name tracking.The addition of the optional
walletNameproperty toWeb3SignatureFactoris a non-breaking change that enables tracking which specific wallet was used for Web3 authentication, aligning well with the Solana wallet selection feature.packages/elements/src/react/hooks/use-third-party-provider.hook.ts (1)
100-102: LGTM! Consistent Web3 provider integration.The Solana provider handling follows the established pattern for other Web3 providers (Metamask, Base, Coinbase, OKX), sending the appropriate
AUTHENTICATE.WEB3event with theweb3_solana_signaturestrategy.packages/shared/src/types/web3.ts (1)
13-26: LGTM! Correct type modeling for Solana as a non-Ethereum Web3 provider.The type definitions correctly distinguish between
Web3Provider(which includes Solana) andEthereumWeb3Provider(which excludes Solana), properly reflecting that Solana is a separate blockchain platform rather than an Ethereum-compatible network.packages/clerk-js/src/ui/components/SignUp/index.tsx (1)
90-92: LGTM! Route addition for Solana wallet selection.The new
choose-walletroute integrates cleanly into the SignUp flow. The generic route name allows for potential future expansion to support other blockchain wallet selections beyond Solana.packages/clerk-js/src/ui/components/SignIn/index.tsx (1)
81-83: LGTM! Symmetric route addition maintaining flow consistency.The
choose-walletroute for SignIn mirrors the identical route in SignUp, maintaining consistency across authentication flows. The route placement between account switching and verification steps is logical.packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
128-129: LGTM! FlowMetadata extension for wallet selection tracking.The addition of
'choose-wallet'to theFlowMetadata.partunion enables proper flow metadata tracking for the new Solana wallet selection UI, maintaining consistency with the route names defined in the SignIn and SignUp components.packages/elements/src/internals/machines/sign-up/start.machine.ts (1)
50-52: LGTM!The Solana authentication handling follows the established pattern for other Web3 strategies. Verification confirms that
signUp.authenticateWithSolana()is properly implemented on the SignUpResource and returnsPromise<SignUpResource>.packages/elements/src/internals/machines/sign-in/start.machine.ts (1)
45-47: LGTM! Verification complete.The
authenticateWithSolana()method is properly defined on SignInResource and returnsPromise<SignInResource>as expected. The implementation is consistent with other Web3 authentication strategies in the SignIn flow.packages/shared/src/types/clerk.ts (1)
2258-2258: LGTM: walletName addition to web3 params.Adding the optional
walletNamefield toClerkAuthenticateWithWeb3Paramsis appropriate since it's Solana-specific and other Web3 providers don't require it.packages/shared/src/web3.ts (1)
24-28: LGTM: Solana provider addition.The Solana provider entry follows the established pattern and is correctly structured with the appropriate strategy and display name.
packages/shared/src/types/signUpCommon.ts (1)
120-122: LGTM: Correct type definition for Solana sign-up params.The
SignUpAuthenticateWithSolanaParamstype correctly extends the base Web3 params and makeswalletNamerequired, which aligns with Solana authentication requirements.Note: This highlights the inconsistency with
AuthenticateWithSolanaParamsinpackages/shared/src/types/clerk.tswherewalletNameis optional (already flagged in a separate comment).packages/shared/src/types/signIn.ts (1)
1-1: LGTM: Solana authentication method added to SignIn resource.The
authenticateWithSolanamethod follows the established pattern of other Web3 authentication methods and maintains API consistency.Note: This method relies on
AuthenticateWithSolanaParamsfromclerk.ts, which has a potential type inconsistency withwalletNameoptionality (already flagged in a separate comment).Also applies to: 80-80
packages/shared/src/types/signUp.ts (1)
9-9: LGTM: Solana authentication method added to SignUp resource.The
authenticateWithSolanamethod correctly follows the established pattern of other Web3 authentication methods in SignUp (e.g.,authenticateWithMetamask,authenticateWithBase).The params being optional at the method level while
SignUpAuthenticateWithSolanaParamsrequireswalletNameis consistent with the existing pattern where params are optional, but required fields must be provided when calling the method.Also applies to: 111-111
packages/clerk-js/package.json (1)
72-73: Solana packages verified as latest stable with no security vulnerabilities.Verification confirms @solana/wallet-adapter-react (^0.15.39) and @solana/wallet-standard (^1.1.4) are the latest stable versions. GitHub's security advisory database shows no known vulnerabilities for these packages or their dependencies.
packages/react/src/isomorphicClerk.ts (1)
18-18: Solana auth wiring matches existing web3 patternsThe
AuthenticateWithSolanaParamsimport andauthenticateWithSolanamethod are consistent with the existingauthenticateWithMetamask/authenticateWithCoinbaseWallet/authenticateWithBase/authenticateWithOKXWalletimplementations, including premount queuing behavior. No issues from this file’s side, assumingBrowserClerkexposesauthenticateWithSolanaas well.Also applies to: 1412-1419
packages/clerk-js/src/ui/components/SignIn/SignInFactorOneSolanaWalletsCard.tsx (1)
21-36: Solana sign-in factor flow is consistent with existing web3 patternsThe factor-one Solana card correctly:
- Sets loading with the selected
walletName,- Calls
clerk.authenticateWithWeb3usingweb3_solana_signature,customNavigate,redirectUrl,signUpContinueUrl, andsecondFactorUrl,- Handles errors via the shared
handleErrorhelper and resets card state.Overall structure and redirect HOCs around the card match the existing sign-in flow conventions.
Also applies to: 42-67
packages/clerk-js/src/ui/elements/Web3WalletButtons.tsx (1)
18-34: ****The original review is incorrect. The codebase correctly uses Wallet-Standard auto-detection, which does not require explicit wallet adapters.
The file imports
MAINNET_ENDPOINTfrom@solana/wallet-standard, and the Wallet-Standard wallets are auto-detected by @solana/wallet-adapter-base, so you do not need to create adapters for them—passing an empty wallets array to WalletProvider is the correct pattern for Wallet-Standard-only support. TheuseWallet()hook will correctly expose installed wallets (Phantom, Backpack, etc.) via auto-detection, not from thewalletsprop. The code is working as designed and no changes are required.Likely an incorrect or invalid review comment.
packages/clerk-js/src/core/resources/SignIn.ts (3)
191-221: Solana web3 strategy correctly wired into first-factor prepThe addition of
case 'web3_solana_signature'intoprepareFirstFactorwith aWeb3SignatureConfigusingweb3WalletIdkeeps the Web3 factor treatment consistent across providers and ensures Solana factors are prepared through the same path.
385-424: Web3 Solana path andwalletNamevalidation inauthenticateWithWeb3look correctDeriving
providerfromstrategy, requiringwalletNamewhenprovider === 'solana', and passingwalletNameinto both the initial and Coinbase-retry signature calls aligns with the Solana utils’ expectations and avoids ambiguous failures when no wallet is selected.
983-1068: Futureweb3Solana branch is consistent with core flowThe new
case 'solana'inSignInFuture.web3correctly:
- Enforces
walletNameat runtime with a specificClerkRuntimeError.- Resolves the identifier via
getSolanaIdentifier({ walletName }).- Uses
generateSignatureWithSolanaand passeswalletNamefor signing and for the Coinbase retry branch.This keeps the experimental Future API in sync with the main SignIn Web3/Solana semantics.
packages/clerk-js/src/core/clerk.ts (1)
2260-2265: Solana helper on Clerk is wired correctly to the generic Web3 flow
authenticateWithSolanasimply forwards props intoauthenticateWithWeb3withstrategy: 'web3_solana_signature', which keeps the public API small and reuses the core Web3 handling. This is consistent with the existing Metamask/Coinbase/Base/OKX helpers.packages/clerk-js/src/utils/web3.ts (4)
19-39: Solana/EVM split ingetWeb3Identifieris clear and matches provider abstractionsUsing a shared
getWeb3WalletProviderand branching onprovider === 'solana'to callwalletProvider.features[standard:connect].connect()(vseth_requestAccountsfor EVM wallets) is a good fit for the wallet-standard model and keeps identifier resolution consistent across all providers.
46-79: Solana signing flow ingenerateWeb3Signatureis well-structuredThe Solana branch:
- Resolves the wallet via
getWeb3WalletProvider(provider, walletName).- Finds the account matching
identifier.- Calls the
solana:signMessagefeature.- Encodes the resulting signature with
bs58.Returning
''with a console warning when the provider, account, or signature is missing keeps behavior aligned with the existing EVM paths that fall back to an empty identifier/signature on missing providers.
97-132:getSolanaIdentifier/generateSignatureWithSolanahelpers correctly encapsulate Solana detailsThe new helpers nicely wrap the generic primitives:
getSolanaIdentifierdelegates togetWeb3Identifier({ provider: 'solana', walletName }).generateSignatureWithSolanaforwards togenerateWeb3Signaturewithprovider: 'solana'and propagateswalletName.This keeps call sites (SignIn, SignUp, Clerk) simple while centralizing Solana-specific behavior.
134-180: Provider discovery helper cleanly separates Solana vs EVM resolution
getWeb3WalletProvidernow:
- Uses SDKs for
coinbase_walletandbase(with environment checks).- For
solana, requireswalletNameand looks up the provider viagetInjectedWeb3SolanaProviders().get(walletName), warning and returningnullif none is provided.- For other EVM wallets, relies on
getInjectedWeb3EthProviders().get(provider).This nicely centralizes provider discovery and lets higher-level flows stay provider-agnostic.
| if (strategy === 'web3_solana_signature') { | ||
| // TODO: Add support to pass legalAccepted status | ||
| return navigate(`choose-wallet?strategy=${strategy}`); | ||
| } |
There was a problem hiding this comment.
Address the TODO: Pass legalAccepted status to wallet chooser.
The TODO comment indicates that legalAccepted status should be passed to the wallet chooser route, but it's currently not included. This could result in users needing to re-accept legal terms or the legal acceptance status being lost.
Consider passing legalAccepted via query parameters or route state to maintain consistency with the OAuth and other web3 flows (lines 58, 71, 89).
Would you like me to suggest an implementation that propagates legalAccepted through the navigation?
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/components/SignUp/SignUpSocialButtons.tsx around
lines 77-80, the wallet chooser navigation for strategy 'web3_solana_signature'
currently omits the legalAccepted state; update the navigate call to include
legalAccepted like the other flows (lines 58, 71, 89) by adding it as a query
parameter or route state (e.g., include &legalAccepted=${String(legalAccepted)}
or pass { state: { legalAccepted } }), ensuring the strategy param is preserved
and the boolean is stringified/encoded if using query params.
| const onSelect = async ({ walletName }: { walletName: string }) => { | ||
| card.setLoading(walletName); | ||
| try { | ||
| await clerk.authenticateWithWeb3({ | ||
| customNavigate: router.navigate, | ||
| redirectUrl: ctx.afterSignUpUrl || '/', | ||
| signUpContinueUrl: '../continue', | ||
| unsafeMetadata: ctx.unsafeMetadata, | ||
| strategy: 'web3_solana_signature', | ||
| // TODO: Add support to pass legalAccepted status | ||
| // legalAccepted: , | ||
| walletName, | ||
| }); | ||
| } catch (err) { | ||
| await sleep(1000); | ||
| handleError(err as Error, [], card.setError); | ||
| card.setIdle(); | ||
| } | ||
| await sleep(5000); | ||
| card.setIdle(); | ||
| }; |
There was a problem hiding this comment.
Remove redundant sleeps and idle reset in onSelect
onSelect currently:
- Sleeps 1s on error, then calls
setIdle, and - Always sleeps an additional 5s and calls
setIdleagain after thetry/catch.
This makes the handler stay pending for ~6s on failure and adds an extra setIdle, and it also delays success cases unnecessarily. It would be cleaner to mirror other flows (e.g. the SignIn Solana card) and move the idle reset into a finally (or just the error path) without the extra 5s delay.
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/components/SignUp/SignUpStartSolanaWalletsCard.tsx
around lines 22-42, remove the redundant await sleep(5000) and the duplicate
card.setIdle() after the try/catch, and restructure the handler so
card.setIdle() is invoked exactly once by moving it into a finally block; keep
the short await sleep(1000) in the catch before calling handleError and do not
call card.setIdle() inside the catch as well to avoid double resets and
unnecessary delays on success.
kduprey
changed the title
packages/clerk-js/package.json
Outdated
| "@solana/wallet-adapter-react": "^0.15.39", | ||
| "@solana/wallet-standard": "^1.1.4", |
There was a problem hiding this comment.
| "@solana/wallet-adapter-react": "^0.15.39", | |
| "@solana/wallet-standard": "^1.1.4", | |
| "@solana/wallet-adapter-react": "0.15.39", | |
| "@solana/wallet-standard": "1.1.4", |
There was a problem hiding this comment.
I see we're using these in the AIO like so:
import { WalletReadyState } from '@solana/wallet-adapter-base';
import { ConnectionProvider, useWallet, WalletProvider } from '@solana/wallet-adapter-react';
Are we sure that we need both packages? Is there any chance WalletReadyState is re-experted from wallet-adapter-react instead?
There was a problem hiding this comment.
Unfortunately, I checked in the midst of development before installing the additional lib, and WalletReadyState is not exported from @solana/wallet-adapter-react, so I would have just used the one lib here. I am all in favour of us declaring it with a link to the lib, but that means we need to maintain the value as up to date.
| }); | ||
| }; | ||
|
|
||
| public authenticateWithSolana = async (params?: AuthenticateWithSolanaParams): Promise<SignInResource> => { |
There was a problem hiding this comment.
i guess the params should be required here right? We need the walletName always
| public authenticateWithSolana = async (params?: AuthenticateWithSolanaParams): Promise<SignInResource> => { | |
| public authenticateWithSolana = async (params: AuthenticateWithSolanaParams): Promise<SignInResource> => { |
| signature = await generateSignature({ | ||
| identifier, | ||
| nonce: message, | ||
| walletName: params.walletName, |
There was a problem hiding this comment.
why do we need to pass the wallet name here, as we are in the case of the coinbase wallet
|
|
||
| public authenticateWithSolana = async ( | ||
| params?: SignUpAuthenticateWithWeb3Params & { | ||
| walletName?: string; |
There was a problem hiding this comment.
in case of Solana this should be a required parameter right?
| }} | ||
| web3Callback={strategy => { | ||
| if (strategy === 'web3_solana_signature') { | ||
| return navigate(`choose-wallet?strategy=${strategy}`); |
There was a problem hiding this comment.
is this the proper way to navigate to the next screen? In general, why we need to pass along the strategy? Doesn't than mean that the rest of the query parameters will be lost after the navigate. For example, if somebody navigates to example.com/sign-up?redirect_url=/checkout, we need to make sure that after clicking the solana wallet button, the extra query parameters will persist
There was a problem hiding this comment.
After speaking with @dstaley regarding this implementation strategy, we agreed that this would pave the way for the future of supporting Web3 chains, rather than specific wallets, which would result in the user needing to select their wallet of choice, and inturn require the persistance of the strategy to listen in the browser window for the proper wallet extension providers that support the chosen strategy.
@panteliselef - Please do let me know if the query param will cause issues in this flow and if it should be reworked.
| web3Callback={strategy => { | ||
| if (strategy === 'web3_solana_signature') { | ||
| // TODO: Add support to pass legalAccepted status | ||
| return navigate(`choose-wallet?strategy=${strategy}`); |
packages/clerk-js/src/utils/web3.ts
Outdated
|
|
||
| async function getEthereumProvider(provider: Web3Provider) { | ||
| export async function generateSignatureWithSolana(params: GenerateSolanaSignatureParams): Promise<string> { | ||
| return await generateWeb3Signature({ ...params, provider: 'solana', walletName: params.walletName }); |
There was a problem hiding this comment.
| return await generateWeb3Signature({ ...params, provider: 'solana', walletName: params.walletName }); | |
| return await generateWeb3Signature({ ...params, provider: 'solana' }); |
packages/clerk-js/src/utils/web3.ts
Outdated
| console.warn('Wallet name must be provided to get Solana wallet provider'); | ||
| return null; |
There was a problem hiding this comment.
shouldn't this throw an error instead? Or just not check for this at all and let it though, to not find any solana provider and throw an error later in the stack
packages/clerk-js/package.json
Outdated
| "@solana/wallet-adapter-react": "^0.15.39", | ||
| "@solana/wallet-standard": "^1.1.4", |
There was a problem hiding this comment.
| "@solana/wallet-adapter-react": "^0.15.39", | |
| "@solana/wallet-standard": "^1.1.4", | |
| "@solana/wallet-adapter-react": "0.15.39", | |
| "@solana/wallet-standard": "1.1.4", |
packages/clerk-js/package.json
Outdated
| "@wallet-standard/core": "^1.1.1", | ||
| "@wallet-standard/react": "^1.0.1", |
There was a problem hiding this comment.
| "@wallet-standard/core": "^1.1.1", | |
| "@wallet-standard/react": "^1.0.1", | |
| "@wallet-standard/core": "1.1.1", | |
| "@wallet-standard/react": "1.0.1", |
packages/shared/src/types/web3.ts
Outdated
| export type Web3Provider = | ||
| | MetamaskWeb3Provider | ||
| | BaseWeb3Provider | ||
| | CoinbaseWalletWeb3Provider | ||
| | OKXWalletWeb3Provider | ||
| | SolanaWeb3Provider; |
There was a problem hiding this comment.
| export type Web3Provider = | |
| | MetamaskWeb3Provider | |
| | BaseWeb3Provider | |
| | CoinbaseWalletWeb3Provider | |
| | OKXWalletWeb3Provider | |
| | SolanaWeb3Provider; | |
| export type Web3Provider = | |
| | EthereumWeb3Provider | |
| | SolanaWeb3Provider; |
nikosdouvlis
left a comment
nikosdouvlis
left a comment
There was a problem hiding this comment.
I see that bundlewatch checks fail, this is because we now use 5 new libs and bundle them in the core clerk-js bundle directly. We need to update rspack.config.ts so that the modules are bundled as a 3p vendor instead
packages/clerk-js/package.json
Outdated
| "@solana/wallet-adapter-react": "^0.15.39", | ||
| "@solana/wallet-standard": "^1.1.4", |
There was a problem hiding this comment.
I see we're using these in the AIO like so:
import { WalletReadyState } from '@solana/wallet-adapter-base';
import { ConnectionProvider, useWallet, WalletProvider } from '@solana/wallet-adapter-react';
Are we sure that we need both packages? Is there any chance WalletReadyState is re-experted from wallet-adapter-react instead?
packages/clerk-js/package.json
Outdated
| "@wallet-standard/core": "^1.1.1", | ||
| "@wallet-standard/react": "^1.0.1", |
There was a problem hiding this comment.
| "@wallet-standard/core": "^1.1.1", | |
| "@wallet-standard/react": "^1.0.1", | |
| "@wallet-standard/core": "1.1.1", | |
| "@wallet-standard/react": "1.0.1", |
Do we absolutely need both @wallet-standard/core and @wallet-standard/core and @solana/wallet-standard?
There was a problem hiding this comment.
We could type check against WalletWithSolanaFeatures but we risk it being too strict of a type and providers not matching all required features in this wallet standard (I realise that is the point of a standard, but this is still a relatively new standard at that.), resulting in omitting providers that provide standard:connect and solana:signMessage but not the rest of the methods required in this type.
packages/clerk-js/package.json
Outdated
| "@rspack/cli": "^1.4.11", | ||
| "@rspack/core": "^1.4.11", | ||
| "@rspack/plugin-react-refresh": "^1.5.0", | ||
| "@solana/wallet-adapter-base": "^0.9.27", |
There was a problem hiding this comment.
| "@solana/wallet-adapter-base": "^0.9.27", | |
| "@solana/wallet-adapter-base": "0.9.27", |
This should probably be a normal dependency instead
…ents Signed-off-by: Kenton Duprey <kenton@clerk.dev>
Signed-off-by: Kenton Duprey <kenton@clerk.dev>
… methods Signed-off-by: Kenton Duprey <kenton@clerk.dev>
…l error handling and strict types Signed-off-by: Kenton Duprey <kenton@clerk.dev>
… logic Signed-off-by: Kenton Duprey <kenton@clerk.dev>
Signed-off-by: Kenton Duprey <kenton@clerk.dev>
… authentication methods Signed-off-by: Kenton Duprey <kenton@clerk.dev>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/clerk-js/src/core/resources/SignUp.ts (1)
271-320: Consider adding walletName validation for Solana in authenticateWithWeb3.The
authenticateWithWeb3method acceptswalletNameas optional (line 283), but whenprovideris 'solana',walletNameis required. Consider adding validation similar to what's done in SignIn.ts (lines 394-396) to fail fast with a clear error message:public authenticateWithWeb3 = async ( params: AuthenticateWithWeb3Params & { unsafeMetadata?: SignUpUnsafeMetadata; legalAccepted?: boolean; }, ): Promise<SignUpResource> => { const { generateSignature, identifier, unsafeMetadata, strategy = 'web3_metamask_signature', legalAccepted, walletName, } = params || {}; const provider = strategy.replace('web3_', '').replace('_signature', '') as Web3Provider; if (!(typeof generateSignature === 'function')) { clerkMissingOptionError('generateSignature'); } + if (provider === 'solana' && !walletName) { + clerkMissingOptionError('walletName'); + }This provides consistency with SignIn and clearer error messaging.
packages/clerk-js/src/core/resources/SignIn.ts (1)
984-1018: Use ClerkRuntimeError for consistency and remove commented code.
Line 987: Remove the commented
// as Web3Provider;- it's unclear why it was commented out and adds noise.Lines 1010-1012: Use
ClerkRuntimeErrorinstead of genericErrorfor consistency with the rest of the codebase:case 'solana': if (!params.walletName) { - throw new Error('walletName is required for solana web3 authentication'); + throw new ClerkRuntimeError('walletName is required for solana web3 authentication', { + code: 'missing_wallet_name', + }); }This provides consistent error handling and structured error codes throughout the application.
♻️ Duplicate comments (2)
packages/clerk-js/src/utils/web3.ts (2)
1-2: CRITICAL: Remove non-existent type import.The type
SolanaWalletAdapterWalletdoes not exist in@solana/wallet-standard. According to the Wallet Standard specification, wallets implement theWalletinterface from@wallet-standard/base, which includes:
accounts: readonly WalletAccount[]features: Record<string, unknown>Remove this import and use the standard types instead.
Apply this diff:
-import type { GenerateSignature, GenerateSolanaSignatureParams, Web3Provider } from '@clerk/shared/types'; -import type { SolanaWalletAdapterWallet } from '@solana/wallet-standard'; +import type { GenerateSignature, GenerateSolanaSignatureParams, Web3Provider } from '@clerk/shared/types'; +import type { Wallet } from '@wallet-standard/base';
53-69: CRITICAL: Fix invalid type cast and improve error handling.Multiple issues in the Solana signing logic:
Invalid cast (lines 55, 60): The cast to
SolanaWalletAdapterWalletis invalid since this type doesn't exist. Use theWallettype from@wallet-standard/baseinstead.Silent failures (lines 57-59, 65-67): Using
console.warnand returning empty strings hides errors from users. These should throw proper errors.Type safety: The wallet accounts and features should be accessed with proper type guards.
Apply this diff:
if (provider === 'solana') { const bs58 = await import('bs58').then(mod => mod.default); - const walletAccount = (wallet as SolanaWalletAdapterWallet).accounts.find(a => a.address === identifier); + const walletAccount = wallet.accounts.find(a => a.address === identifier); if (!walletAccount) { - console.warn(`Wallet account with address ${identifier} not found`); - return ''; + throw new ClerkRuntimeError(`Wallet account with address ${identifier} not found`, { + code: 'wallet_account_not_found', + }); } - const signedMessages = await (wallet as SolanaWalletAdapterWallet).features[SolanaSignMessage]?.signMessage({ + const signMessageFeature = wallet.features[SolanaSignMessage]; + if (!signMessageFeature || typeof signMessageFeature.signMessage !== 'function') { + throw new ClerkRuntimeError('Wallet does not support message signing', { + code: 'wallet_feature_not_supported', + }); + } + const signedMessages = await signMessageFeature.signMessage({ account: walletAccount, message: new TextEncoder().encode(nonce), }); if (!signedMessages || signedMessages.length === 0) { - console.warn('No signed messages returned from wallet'); - return ''; + throw new ClerkRuntimeError('No signed messages returned from wallet', { + code: 'wallet_signing_failed', + }); } return bs58.encode(signedMessages[0].signature); }Additionally, import
ClerkRuntimeError:+import { ClerkRuntimeError } from '@clerk/shared/error';
🧹 Nitpick comments (3)
packages/clerk-js/src/core/resources/SignIn.ts (1)
1041-1061: Address inconsistent walletName handling in retry logic.The Coinbase wallet retry logic (lines 1053-1057) doesn't pass
walletNamein the retry attempt, while the initial attempt does (line 1041-1045). This inconsistency could cause issues if a Solana-specific parameter is needed during retry.However, looking at the logic more carefully: this retry is specifically for
coinbase_wallet(line 1053), not Solana. ThewalletNamewould beundefinedfor Coinbase wallet anyway, so this is actually safe.For clarity and to prevent confusion, consider adding a comment explaining why
walletNameis omitted in the retry:if (provider === 'coinbase_wallet' && err.code === 4001) { + // Retry without walletName as Coinbase wallet doesn't require it signature = await generateSignature({ identifier, nonce: message, });packages/clerk-js/src/utils/web3.ts (2)
20-40: Improve error handling - avoid silent failures with empty string returns.Lines 25-29: When a provider is not found, the function returns an empty string with just a TODO comment. This silently fails and makes debugging difficult. Consider throwing a proper error or returning a Result type.
Apply this diff:
const walletProvider = await getWeb3Wallet(provider, walletName); // TODO - core-3: Improve error handling for the case when the provider is not found if (!walletProvider) { - // If a plugin for the requested provider is not found, - // the flow will fail as it has been the expected behavior so far. - return ''; + throw new ClerkRuntimeError(`Web3 provider "${provider}" not found`, { + code: 'web3_provider_not_found', + }); }The same issue exists in
generateWeb3Signatureat lines 47-51.
93-95: Refactor: Make walletName parameter direct instead of object.For internal utility functions, passing
walletNamedirectly as a string parameter rather than wrapping it in an object improves clarity and reduces boilerplate:-export async function getSolanaIdentifier(walletName: string): Promise<string> { - return await getWeb3Identifier({ provider: 'solana', walletName }); -} +export async function getSolanaIdentifier(walletName: string): Promise<string> { + if (!walletName) { + throw new ClerkRuntimeError('Wallet name is required for Solana', { + code: 'wallet_name_required', + }); + } + return await getWeb3Identifier({ provider: 'solana', walletName }); +}Also consider adding the validation here to fail fast.
Based on coding guidelines: "As per coding guidelines, internal utilities can accept direct parameters instead of objects for simplicity."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
packages/clerk-js/package.json(1 hunks)packages/clerk-js/src/core/clerk.ts(9 hunks)packages/clerk-js/src/core/resources/SignIn.ts(12 hunks)packages/clerk-js/src/core/resources/SignUp.ts(4 hunks)packages/clerk-js/src/utils/injectedWeb3SolanaProviders.ts(1 hunks)packages/clerk-js/src/utils/web3.ts(4 hunks)packages/react/src/isomorphicClerk.ts(2 hunks)packages/shared/src/types/clerk.ts(3 hunks)packages/shared/src/types/signIn.ts(2 hunks)packages/shared/src/types/signUp.ts(2 hunks)packages/shared/src/types/web3.ts(1 hunks)packages/shared/src/types/web3Wallet.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/clerk-js/package.json
- packages/shared/src/types/signUp.ts
- packages/clerk-js/src/utils/injectedWeb3SolanaProviders.ts
- packages/react/src/isomorphicClerk.ts
- packages/shared/src/types/clerk.ts
- packages/shared/src/types/signIn.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/shared/src/types/web3.tspackages/clerk-js/src/core/clerk.tspackages/shared/src/types/web3Wallet.tspackages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/core/resources/SignUp.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/shared/src/types/web3.tspackages/clerk-js/src/core/clerk.tspackages/shared/src/types/web3Wallet.tspackages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/core/resources/SignUp.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/types/web3.tspackages/clerk-js/src/core/clerk.tspackages/shared/src/types/web3Wallet.tspackages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/core/resources/SignUp.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/shared/src/types/web3.tspackages/clerk-js/src/core/clerk.tspackages/shared/src/types/web3Wallet.tspackages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/core/resources/SignUp.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/shared/src/types/web3.tspackages/clerk-js/src/core/clerk.tspackages/shared/src/types/web3Wallet.tspackages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/core/resources/SignUp.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/shared/src/types/web3.tspackages/clerk-js/src/core/clerk.tspackages/shared/src/types/web3Wallet.tspackages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/core/resources/SignUp.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/shared/src/types/web3.tspackages/clerk-js/src/core/clerk.tspackages/shared/src/types/web3Wallet.tspackages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.tspackages/clerk-js/src/core/resources/SignUp.ts
🧬 Code graph analysis (3)
packages/clerk-js/src/core/clerk.ts (3)
packages/shared/src/types/clerk.ts (1)
AuthenticateWithSolanaParams(2302-2309)packages/clerk-js/src/utils/web3.ts (2)
getWeb3Identifier(20-40)generateSignatureWithSolana(118-120)packages/shared/src/types/web3Wallet.ts (1)
GenerateSignature(28-30)
packages/shared/src/types/web3Wallet.ts (2)
packages/shared/src/types/strategies.ts (1)
Web3Strategy(20-20)packages/shared/src/types/web3.ts (2)
EthereumWeb3Provider(17-21)SolanaWeb3Provider(13-13)
packages/clerk-js/src/core/resources/SignUp.ts (2)
packages/shared/src/types/signUp.ts (1)
SignUpResource(41-123)packages/clerk-js/src/utils/web3.ts (2)
getSolanaIdentifier(93-95)generateSignatureWithSolana(118-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
packages/shared/src/types/web3.ts (1)
13-21: LGTM! Clean type organization.The addition of
SolanaWeb3Providerand the refactoring to introduceEthereumWeb3Providerimproves type clarity and maintains backward compatibility while supporting the new Solana integration.packages/clerk-js/src/core/resources/SignUp.ts (1)
382-397: Verify AuthenticateWithSolanaParams type includes walletName as required.The method signature requires
walletName: stringin the params (line 384), which is correct for Solana. Ensure that theAuthenticateWithSolanaParamstype in the shared types package includeswalletNameas a required field, not optional.Based on the relevant code snippet from packages/shared/src/types/clerk.ts showing
AuthenticateWithSolanaParamsincludeswalletName: string(required), this is correctly aligned.packages/shared/src/types/web3Wallet.ts (1)
28-51: LGTM! Type definitions correctly distinguish Ethereum and Solana requirements.The separation into
GenerateEthereumSignatureParamsandGenerateSolanaSignatureParamswith differentwalletNamerequirements (optional vs required) correctly models the domain:
- Ethereum wallets can be auto-detected, so
walletNameis optional- Solana requires explicit wallet selection, so
walletNameis requiredThe union type for
GenerateSignatureenables proper type narrowing based on the provider.packages/clerk-js/src/core/clerk.ts (2)
2260-2265: LGTM! Solana authentication properly delegates to Web3 flow.The
authenticateWithSolanamethod correctly:
- Accepts
AuthenticateWithSolanaParamswith requiredwalletName- Delegates to
authenticateWithWeb3with the appropriate strategy- Follows the established pattern of other wallet-specific methods (Metamask, Coinbase, etc.)
2267-2302: Based on my verification, I have confirmed the review comment through examining:
Type definitions:
AuthenticateWithWeb3ParamsandClerkAuthenticateWithWeb3Paramsboth definewalletNameas optional (walletName?: string)Propagation flow:
- ✅
walletNameis destructured inClerk.authenticateWithWeb3(line 2275)- ✅ Passed to
getWeb3Identifier(line 2284)- ✅ Forwarded to
SignIn.authenticateWithWeb3(line 2317)- ✅ Forwarded to
SignUp.authenticateWithWeb3(line 2327)Optional handling:
- ✅
SignIn.authenticateWithWeb3validateswalletNameis required only for Solana (line 394-395)- ✅
SignUpprovides separateauthenticateWithSolana()method that enforceswalletNamerequirement- ✅ Non-Solana providers (Metamask, Coinbase, Base, OKX) do not pass
walletName- ✅
getWeb3Identifiercorrectly handles optionalwalletNameparameter✅ walletName is correctly propagated through the entire authentication flow with proper handling of its optional nature for non-Solana providers.
packages/clerk-js/src/core/resources/SignIn.ts (2)
394-396: LGTM! Proper validation for Solana-specific requirements.The validation ensures that
walletNameis provided when using Solana, which is essential since Solana wallets cannot be auto-detected like Ethereum wallets. This prevents runtime errors downstream in the signature generation flow.
472-480: LGTM! Solana authentication method follows established patterns.The
authenticateWithSolanamethod correctly:
- Requires
walletNamein the params- Fetches the Solana identifier using the provided wallet name
- Delegates to
authenticateWithWeb3with the appropriate strategy and parameters
…nal walletname Signed-off-by: Kenton Duprey <kenton@clerk.dev>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/clerk-js/src/utils/web3.ts (1)
1-3: RevisitSolanaWalletAdapterWalletimport and cast – likely unnecessary and brittleThe import and cast here are fragile:
import type { SolanaWalletAdapterWallet } from '@solana/wallet-standard';(wallet as SolanaWalletAdapterWallet).accountsand(wallet as SolanaWalletAdapterWallet).features[SolanaSignMessage]...Unless your version of
@solana/wallet-standardexplicitly exportsSolanaWalletAdapterWallet, this will fail type‑checking. Even if it does,getInjectedWeb3SolanaProviders().get(walletName)is conceptually returning a Wallet‑StandardWallet, so a narrower adapter‑specific type is not really needed here.A more robust approach would be:
- Use the canonical Wallet Standard types (e.g.
Wallet/WalletAccountfrom@wallet-standard/baseor your existing Solana wallet helper types), and- Drop the explicit
SolanaWalletAdapterWalletcast, accessingwallet.accountsandwallet.features[SolanaSignMessage]through those types or a local structural type.This avoids tight coupling to a specific adapter type and reduces the risk of type mismatch when upgrading Solana dependencies.
Does the `@solana/wallet-standard` package (at the version used in this repo) export a `SolanaWalletAdapterWallet` type, or should we instead type injected Solana wallets using the standard `Wallet` / `WalletAccount` interfaces plus the `solana:signMessage` feature types?Also applies to: 53-61
🧹 Nitpick comments (3)
packages/clerk-js/src/core/resources/SignIn.ts (2)
215-221: Solana Web3 auth wiring looks consistent; small type/style nitsThe Solana additions here line up well with existing Web3 providers:
- Adding
'web3_solana_signature'toprepareFirstFactorand reusingWeb3SignatureConfigis consistent.authenticateWithWeb3’sproviderderivation and thewalletNameguard forprovider === 'solana'match the requirement that Solana flows always know the wallet name.authenticateWithSolanacorrectly resolves the identifier viagetSolanaIdentifierand delegates toauthenticateWithWeb3usinggenerateSignatureWithSolana.Minor cleanups you may want to consider:
authenticateWithSolanadeclaresparams: AuthenticateWithSolanaParams(non‑optional), soparams?.walletNamecan just beparams.walletNamein both thegetSolanaIdentifiercall and the object passed toauthenticateWithWeb3.- The Solana‑specific
walletNamecheck inauthenticateWithWeb3overlaps with the guard ingetWeb3Wallet(viaerrorThrower) on the utils side; if you want a single source of truth, you could rely on the utils guard and keep only the friendlierclerkMissingOptionErrorhere (or vice‑versa). Not critical, but worth aligning to avoid future drift.Also applies to: 386-397, 413-425, 472-480
984-1014: SignInFuture.web3 Solana branch is correct; consider stronger typing & error consistencyThe
web3future API’s Solana branch is wired correctly:
providerderivation fromstrategymatches the main SignIn path.- The Solana case enforces
params.walletName, obtains the identifier viagetSolanaIdentifier, and usesgenerateSignatureWithSolana.- The shared
generateSignature: GenerateSignatureabstraction keeps Solana and EVM paths consistent.A couple of refinements you might want to make:
- Replace
throw new Error('walletName is required for solana web3 authentication');with a typed error (e.g.ClerkRuntimeErrorwith a specificcode) to match other error paths in this class and make downstream handling easier.params?.walletNameis unnecessary sinceparamsis non‑nullable;params.walletNameis sufficient and slightly clearer.These are cosmetic; behavior-wise this looks good.
Also applies to: 1038-1058
packages/clerk-js/src/utils/web3.ts (1)
20-37: getWeb3Identifier / getWeb3Wallet Solana path behavior looks good; minor polishBehavior-wise, the Solana additions are consistent:
getWeb3Identifiernow threadswalletNameintogetWeb3Walletand uses the Wallet Standardstandard:connectfeature to derive the address.getWeb3WalletenforceswalletNamefor Solana viaerrorThrower, then delegates togetInjectedWeb3SolanaProviders().get(walletName).- Non‑Solana providers still go through the existing Coinbase/Base SDK or injected Ethereum providers, preserving previous behavior.
Minor nits you might consider:
- After
errorThrower.throw(...)in the Solana branch, thereturn;is effectively unreachable (the throw isnever), so it can be dropped for clarity.- Since
getWeb3Walletalready enforceswalletNamefor Solana, you may not need additional guards at higher levels unless you specifically want different error shapes/messages there.Nothing blocking here; just possible cleanups.
Also applies to: 93-95, 122-168
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/clerk-js/src/core/resources/SignIn.ts(12 hunks)packages/clerk-js/src/utils/web3.ts(4 hunks)packages/shared/src/types/web3Wallet.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared/src/types/web3Wallet.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/clerk-js/src/utils/web3.tspackages/clerk-js/src/core/resources/SignIn.ts
🧬 Code graph analysis (1)
packages/clerk-js/src/utils/web3.ts (4)
packages/shared/src/types/web3.ts (1)
Web3Provider(15-15)packages/shared/src/types/web3Wallet.ts (2)
GenerateSignature(28-28)GenerateSignatureParams(37-42)packages/clerk-js/src/utils/injectedWeb3SolanaProviders.ts (3)
wallet(24-26)wallet(28-30)getInjectedWeb3SolanaProviders(66-66)packages/clerk-js/src/utils/injectedWeb3EthProviders.ts (1)
getInjectedWeb3EthProviders(75-75)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
| export const generateWeb3Signature: GenerateSignature = async (params): Promise<string> => { | ||
| const { identifier, nonce, provider, walletName = '' } = params; | ||
| const wallet = await getWeb3Wallet(provider, walletName); | ||
|
|
||
| // TODO - core-3: Improve error handling for the case when the provider is not found | ||
| if (!ethereum) { | ||
| if (!wallet) { | ||
| // If a plugin for the requested provider is not found, | ||
| // the flow will fail as it has been the expected behavior so far. | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
Tighten walletName typing for Solana helpers to catch errors at compile time
The runtime behavior is correct — Solana flows require walletName, and getWeb3Wallet enforces this:
generateWeb3SignaturedestructureswalletName = ''and callsgetWeb3Wallet(provider, walletName).- In the Solana branch of
getWeb3Wallet, missing/emptywalletNametriggerserrorThrower.throw('Wallet name must be provided to get Solana wallet provider'). getSolanaIdentifier(walletName: string)andgenerateSignatureWithSolanaare the public Solana entry points.
However, the exported Solana wrapper still exposes a signature that doesn’t reflect this requirement:
type GenerateSignatureParams = {
identifier: string;
nonce: string;
};
export async function generateSignatureWithSolana(params: GenerateSignatureParams): Promise<string> {
return await generateWeb3Signature({ ...params, provider: 'solana' });
}This means callers can (and will be type‑checked as allowed to) call generateSignatureWithSolana({ identifier, nonce }) without a walletName, only to hit a runtime error from errorThrower.
Given this is a new public helper, I’d strongly recommend tightening the types so Solana’s walletName requirement is enforced at compile time, e.g.:
-type GenerateSignatureParams = {
- identifier: string;
- nonce: string;
-};
+type GenerateSignatureParams = {
+ identifier: string;
+ nonce: string;
+};
+
+type GenerateSolanaSignatureParams = GenerateSignatureParams & {
+ walletName: string;
+};
-export async function generateSignatureWithSolana(params: GenerateSignatureParams): Promise<string> {
- return await generateWeb3Signature({ ...params, provider: 'solana' });
-}
+export async function generateSignatureWithSolana(
+ params: GenerateSolanaSignatureParams,
+): Promise<string> {
+ return await generateWeb3Signature({ ...params, provider: 'solana' });
+}Call sites that already have walletName (e.g. SignIn/SignInFuture) will keep working; any new usage will be forced to provide walletName instead of silently compiling and failing at runtime.
Also applies to: 93-95, 118-120, 122-168
| identifier, | ||
| generateSignature: generateSignatureWithSolana, | ||
| strategy: 'web3_solana_signature', | ||
| walletName: params?.walletName, |
There was a problem hiding this comment.
params should not be optional right?
| walletName: params?.walletName, | |
| walletName: params.walletName, |
| if (!params.walletName) { | ||
| throw new Error('walletName is required for solana web3 authentication'); | ||
| } |
There was a problem hiding this comment.
i would expect this error to be thrown by getSolanaIdentifier() instead
| console.warn('No signed messages returned from wallet'); | ||
| return ''; | ||
| } | ||
| return bs58.encode(signedMessages[0].signature); |
There was a problem hiding this comment.
yep this will be much simpler for the SDK and we will be able to drop the b58 dependency
| if (!signedMessages || signedMessages.length === 0) { | ||
| console.warn('No signed messages returned from wallet'); | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
great then we can drop this if block then
Description
<SignIn />,<SignUp />components and flowschoose-wallet) andSignInFactorOneSolanaWalletsCardcomponent to choose an installed Solana walletauthenticateWithSolanatoSignIn,SignUp,IsomorphicClerk, and the publicClerkinterfaceinjectedWeb3SolanaProviders.tssingleton mirroring Ethereum provider structure (EIP-6963 style pattern), using wallet-standard eventsweb3.tsto fetch identifier viaStandardConnectand sign messages viasolana:signMessagefeature (with base58 encoding)AuthenticateWithSolanaParams,SignUpAuthenticateWithSolanaParams) and optionalwalletNamethreading@solana/wallet-standard,@solana/wallet-adapter-react,@solana/wallet-adapter-base, andbs58libswalletNamethrough Clerk core (clerk.ts) during web3 authentication attemptsTesting
This can be tested with a Clerk application that has Sign in with Solana enabled on the instance, by installing this branch of Clerk's libs into a boilerplate application to render the
<SignIn />or<SignUp />components. This does require the browser to have a Web3 wallet extension with a valid Solana chain account to be able to successfully complete the flow.Fixes:
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.