-
Notifications
You must be signed in to change notification settings - Fork 644
Support linking accounts with redirect mode #8643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support linking accounts with redirect mode #8643
Conversation
🦋 Changeset detectedLatest commit: 17d976a The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds redirect-mode account linking: new public Changes
sequenceDiagram
participant User as User
participant UI as Playground UI
participant Auth as In-App Auth
participant Connector as Web Connector
participant OAuth as OAuth Provider
participant Wallet as In-App Wallet
User->>UI: Click "Link Github"
UI->>Auth: call linkProfile(strategy: "github", mode: "redirect")
Auth->>Connector: linkProfile(...)
Connector->>Connector: detect redirect-mode social auth
Connector->>OAuth: loginWithOauthRedirect(authFlow: "link")
OAuth->>User: redirect to provider
User->>OAuth: authenticate
OAuth->>Auth: redirect back with authFlow=link in URL
Auth->>Auth: getUrlToken() extracts authFlow
Auth->>Auth: handleLinkingFlow() (auto-connect stored wallet)
Auth->>Wallet: linkAccount(token) using ClientScopedStorage
Wallet->>UI: trigger onConnect / update state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~27 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/thirdweb/src/exports/wallets/in-app.ts (1)
79-88: Add required public TSDoc forlinkProfileWithRedirect.
Please ensure the exported symbol’s source definition includes comprehensive TSDoc with an@exampleand a custom annotation tag. As per coding guidelines, ...
🤖 Fix all issues with AI agents
In `@apps/playground-web/src/components/in-app-wallet/profiles.tsx`:
- Around line 94-101: Update the button label text to use the correct “GitHub”
capitalization: in the component rendering the Button (the Button instance with
props className="rounded-full p-6", disabled={isPending}, onClick={linkGithub},
variant="default"), change the display text from "Link Github" to "Link GitHub"
so the UI copy uses the proper brand capitalization.
In `@packages/thirdweb/src/wallets/connection/autoConnectCore.ts`:
- Around line 282-305: The active wallet selection currently always uses
storedActiveWalletId; update the logic in autoConnectCore so it prefers the
redirect-provided urlToken.walletId (if present) and only falls back to
storedActiveWalletId, i.e. derive a selectedWalletId = urlToken.walletId ||
storedActiveWalletId and then find or create the active wallet using
wallets.find((w) => w.id === selectedWalletId) ||
createWalletFn(selectedWalletId); keep existing behavior for authProvider and
storage checks unchanged.
In `@packages/thirdweb/src/wallets/in-app/web/lib/web-connector.ts`:
- Around line 470-483: The method linkProfileWithRedirect currently leaves mode
undefined which causes loginWithOauthRedirect to default to opening a window;
change linkProfileWithRedirect so mode defaults to "redirect" (either via its
parameter default or by passing mode ?? "redirect" into the call) so that
loginWithOauthRedirect is invoked with mode set to "redirect"; update the
function signature or the call site where linkProfileWithRedirect calls
loginWithOauthRedirect (referencing linkProfileWithRedirect and
loginWithOauthRedirect) to ensure redirect is the default behavior.
🧹 Nitpick comments (2)
packages/thirdweb/src/wallets/in-app/core/authentication/getLoginPath.ts (1)
18-57: Add an explicit return type forgetLoginUrl.This keeps the API surface consistent with TS style requirements.
♻️ Suggested fix
-export const getLoginUrl = ({ +export const getLoginUrl = ({ authOption, client, ecosystem, mode = "popup", redirectUrl, authFlow, }: { @@ -}) => { +}): string => {As per coding guidelines, please keep explicit return types in TS.
packages/thirdweb/src/wallets/in-app/web/lib/auth/index.ts (1)
219-237: Add an explicit return type forlinkProfileWithRedirect.Keeps the exported API consistent and avoids implicit inference for public SDK functions.
♻️ Suggested fix
-export async function linkProfileWithRedirect( +export async function linkProfileWithRedirect( args: Omit<SocialAuthArgsType, "mode"> & { client: ThirdwebClient; ecosystem?: Ecosystem; mode?: "redirect" | "window"; }, -) { +): Promise<void> {As per coding guidelines, please keep explicit return types in TS.
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (4.34%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #8643 +/- ##
==========================================
- Coverage 53.01% 52.88% -0.13%
==========================================
Files 932 932
Lines 62355 62515 +160
Branches 4103 4105 +2
==========================================
+ Hits 33057 33063 +6
- Misses 29199 29352 +153
- Partials 99 100 +1
🚀 New features to boost your workflow:
|

PR-Codex overview
This PR introduces enhancements for account linking in the
thirdweblibrary, particularly focusing on a new redirect mode for linking profiles. It modifies existing functions and adds new functionalities to support this flow.Detailed summary
linkProfileWithRedirectfunction to handle OAuth linking with redirect mode.useLinkProfile.tsto change the query invalidation timeout from 500ms to 1000ms.authFlowparameter to various functions and interfaces to specify the type of authentication flow.getLoginUrlto includeauthFlowin the redirect URL.handleLinkingFlowto manage the linking process after redirection.LinkAccountcomponent to include a button for linking GitHub accounts using the new redirect method.authFlowparameter ingetUrlToken.Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.