Skip to content

fix(clerk-js): Clear session after deleting an account - #3628

Merged
octoper merged 7 commits into
mainfrom
vaggelis/sdk-1825-deleting-a-users-account-does-not-clear-their-state-and
Jul 10, 2024
Merged

fix(clerk-js): Clear session after deleting an account#3628
octoper merged 7 commits into
mainfrom
vaggelis/sdk-1825-deleting-a-users-account-does-not-clear-their-state-and

Conversation

@octoper

@octoper octoper commented Jun 27, 2024

Copy link
Copy Markdown
Contributor

Description

This PR fixes an issue where the session is not cleared correctly after deleting an account

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:

@changeset-bot

changeset-bot Bot commented Jun 27, 2024

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 08f0d42

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

This PR includes changesets to release 4 packages
Name Type
@clerk/clerk-js Patch
@clerk/astro 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

@octoper

octoper commented Jun 27, 2024

Copy link
Copy Markdown
Contributor Author

!preview

@clerk-cookie

clerk-cookie commented Jun 27, 2024

Copy link
Copy Markdown
Collaborator

Hey @octoper, your preview is available.

Status Preview Updated (UTC)
🍪 Deployed Visit preview Jul 01, 2024 11:20 AM

@octoper

octoper commented Jun 27, 2024

Copy link
Copy Markdown
Contributor Author

!preview

@octoper
octoper marked this pull request as ready for review June 27, 2024 20:44
@octoper octoper changed the title fix(clerk-js): Clear session after deleting account fix(clerk-js): Clear session after deleting an account Jun 27, 2024

@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.

💯 Seems like we were working on the same thing, sorry for not communicating it. I asked some questions mainly because i went with a different direction

Comment thread packages/clerk-js/src/core/clerk.ts
Comment thread packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx Outdated
@octoper
octoper force-pushed the vaggelis/sdk-1825-deleting-a-users-account-does-not-clear-their-state-and branch from 02f53c5 to 3d05d89 Compare June 28, 2024 15:40
@octoper

octoper commented Jun 28, 2024

Copy link
Copy Markdown
Contributor Author

!preview

1 similar comment
@octoper

octoper commented Jun 28, 2024

Copy link
Copy Markdown
Contributor Author

!preview

@octoper
octoper force-pushed the vaggelis/sdk-1825-deleting-a-users-account-does-not-clear-their-state-and branch from 35446eb to fe407fd Compare July 1, 2024 11:13
@octoper

octoper commented Jul 1, 2024

Copy link
Copy Markdown
Contributor Author

!preview

@octoper
octoper force-pushed the vaggelis/sdk-1825-deleting-a-users-account-does-not-clear-their-state-and branch 2 times, most recently from d7f0e0a to 7c30128 Compare July 1, 2024 15:26
const navigationCallback =
otherSessions.length === 0 ? navigateAfterSignOut : navigateAfterMultiSessionSingleSignOutUrl;
return await setActive({
session,

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 should not use the previous session since it's (probably) already deleted. We should pass { session: null }

@dimkl dimkl 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.

I have left some comments. Also we need to add a test to verify the expected behaviour!

@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.

LGTM, added a comment for an updated changeset

Comment thread .changeset/chatty-feet-battle.md Outdated
@octoper

octoper commented Jul 1, 2024

Copy link
Copy Markdown
Contributor Author

@dimkl Added the change and E2E tests as you suggested

@octoper
octoper requested a review from dimkl July 1, 2024 18:47
@octoper
octoper force-pushed the vaggelis/sdk-1825-deleting-a-users-account-does-not-clear-their-state-and branch 4 times, most recently from 805a6c3 to a837390 Compare July 5, 2024 06:49
@octoper
octoper force-pushed the vaggelis/sdk-1825-deleting-a-users-account-does-not-clear-their-state-and branch 4 times, most recently from 082b1c4 to 961ecdb Compare July 9, 2024 12:52
@octoper
octoper force-pushed the vaggelis/sdk-1825-deleting-a-users-account-does-not-clear-their-state-and branch from 961ecdb to 08f0d42 Compare July 10, 2024 13:05
@octoper
octoper merged commit 09f905a into main Jul 10, 2024
@octoper
octoper deleted the vaggelis/sdk-1825-deleting-a-users-account-does-not-clear-their-state-and branch July 10, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants