Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 470
feat(clerk-js): Improve redirects on OAuth callback#1563
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
c596cf947f7c4fa8d779664ffc867841cbd0e3acf8f0d2c6cfae3436File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@clerk/clerk-js': minor | ||
| '@clerk/types': minor | ||
| --- | ||
| Improve redirects on OAuth callback. Now, if you try to sign up with a provider that allows unverified accounts, it will | ||
| navigate to the appropriate change when needed, fixing the broken flow. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -44,6 +44,7 @@ import type { | ||
| SignOut, | ||
| SignOutCallback, | ||
| SignOutOptions, | ||
| SignUpField, | ||
| SignUpProps, | ||
| SignUpResource, | ||
| UnsubscribeCallback, | ||
| @@ -53,6 +54,7 @@ import type { | ||
| } from '@clerk/types'; | ||
| import type { MountComponentRenderer } from '../ui/Components'; | ||
| import { completeSignUpFlow } from '../ui/components/SignUp/util'; | ||
| import { | ||
| appendAsQueryParams, | ||
| buildURL, | ||
| @@ -835,6 +837,7 @@ export default class Clerk implements ClerkInterface { | ||
| const { externalAccount } = signUp.verifications; | ||
| const su = { | ||
| status: signUp.status, | ||
| missingFields: signUp.missingFields, | ||
| externalAccountStatus: externalAccount.status, | ||
| externalAccountErrorCode: externalAccount.error?.code, | ||
| externalAccountSessionId: externalAccount.error?.meta?.sessionId, | ||
| @@ -874,6 +877,23 @@ export default class Clerk implements ClerkInterface { | ||
| buildURL({ base: displayConfig.signUpUrl, hashPath: '/continue' }, { stringify: true }), | ||
| ); | ||
| const navigateToNextStepSignUp = ({ missingFields }: { missingFields: SignUpField[] }) => { | ||
| if (missingFields.length) { | ||
| return navigateToContinueSignUp(); | ||
gkats marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| return completeSignUpFlow({ | ||
| signUp, | ||
| verifyEmailPath: | ||
| params.verifyEmailAddressUrl || | ||
| buildURL({ base: displayConfig.signUpUrl, hashPath: '/verify-email-address' }, { stringify: true }), | ||
| verifyPhonePath: | ||
| params.verifyPhoneNumberUrl || | ||
| buildURL({ base: displayConfig.signUpUrl, hashPath: '/verify-phone-number' }, { stringify: true }), | ||
| navigate, | ||
| }); | ||
| }; | ||
| const userExistsButNeedsToSignIn = | ||
| su.externalAccountStatus === 'transferable' && su.externalAccountErrorCode === 'external_account_exists'; | ||
| @@ -903,7 +923,7 @@ export default class Clerk implements ClerkInterface { | ||
| beforeEmit: navigateAfterSignUp, | ||
| }); | ||
| case 'missing_requirements': | ||
| return navigateToContinueSignUp(); | ||
| return navigateToNextStepSignUp({ missingFields: res.missingFields }); | ||
| default: | ||
| clerkOAuthCallbackDidNotCompleteSignInSignUp('sign in'); | ||
| } | ||
| @@ -939,7 +959,7 @@ export default class Clerk implements ClerkInterface { | ||
| } | ||
| if (su.externalAccountStatus === 'verified' && su.status === 'missing_requirements') { | ||
| return navigateToContinueSignUp(); | ||
| return navigateToNextStepSignUp({ missingFields: signUp.missingFields }); | ||
| } | ||
| return navigateToSignIn(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -451,6 +451,16 @@ export type HandleOAuthCallbackParams = { | ||
| * Full URL or path to navigate after an incomplete sign up. | ||
| */ | ||
| continueSignUpUrl?: string | null; | ||
| /** | ||
| * Full URL or path to navigate after requesting email verification. | ||
| */ | ||
| verifyEmailAddressUrl?: string | null; | ||
| /** | ||
| * Full URL or path to navigate after requesting phone verification. | ||
| */ | ||
| verifyPhoneNumberUrl?: string | null; | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ Are these required for this fix? Seems like this is a broader API change for our components. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It shouldn't need these, but for sign-in, using a relative URL won't work. I want to redirect in these two cases:
For the first one to work, a relative URL like Or I might be possibly missing something. Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, thanks for explaining. I guess the problem is the the transfer flow from sign in? We are on the Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello everyone, is there a design document explaining the changes proposed here? MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @nikosdouvlis, there is no design document for this, but there is more information about the goal of this task in the ticket: https://linear.app/clerk/issue/USR-248 Do you have any concerns about these changes? | ||
| }; | ||
| /** | ||
Uh oh!
There was an error while loading. Please reload this page.