Skip to content

fix(clerk-js): Update existing SignUp object on SignUpContinue OAuth - #3401

Merged
LauraBeatris merged 1 commit into
mainfrom
fix-sign-up-continue
May 21, 2024
Merged

fix(clerk-js): Update existing SignUp object on SignUpContinue OAuth#3401
LauraBeatris merged 1 commit into
mainfrom
fix-sign-up-continue

Conversation

@LauraBeatris

@LauraBeatris LauraBeatris commented May 17, 2024

Copy link
Copy Markdown
Contributor

Description

Resolves SDK-1689 - fixes Web3 sign-up flow with social connections.

The SDK was creating a new SignUp object on OAuth callback when continuing a signup flow, overriding properties such as web3_wallet.

Instead of creating a new SignUp object, it should update the existing one.

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

@LauraBeatris LauraBeatris self-assigned this May 17, 2024
@changeset-bot

changeset-bot Bot commented May 17, 2024

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c1a59fb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@clerk/clerk-js Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch

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

@LauraBeatris
LauraBeatris force-pushed the fix-sign-up-continue branch from 6eaa364 to 6e5369c Compare May 17, 2024 17:44
@LauraBeatris
LauraBeatris marked this pull request as ready for review May 17, 2024 17:47
@LauraBeatris
LauraBeatris requested a review from a team May 17, 2024 17:47
<SignUpSocialButtons
enableOAuthProviders={showOauthProviders}
enableWeb3Providers={showWeb3Providers}
continueSignUp

@LauraBeatris LauraBeatris May 17, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const authenticateFn = (args: SignUpCreateParams | SignUpUpdateParams) =>
continueSignUp && this.id ? this.update(args) : this.create(args);

Since continueSignUp wasn't being provided, then a new SignUp object was created on the redirect.

@LauraBeatris LauraBeatris May 17, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be missing other flows here and this could end up introducing side effects, so please let me know 🙇🏻

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓Shouldn't the absence of continueSignUp create issues for the rest of oath providers ? Did we verify that those were failing as well, because if they were successful that would seem weird.

@LauraBeatris LauraBeatris May 20, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create issues for the rest of oath providers

Good point, I haven't verified yet - what are other flows that someone could fall on ContinueSignUp?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have verified (offline) that flows work as expected

@panteliselef panteliselef left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants