Skip to content

fix(clerk-js): Use afterSwitchSessionUrl switching sessions within UserButton#4726

Merged
panteliselef merged 10 commits intomainfrom
elef/sdki-784-userbutton-not-using-afterswitchsessionurl-on-switching
Dec 12, 2024
Merged

fix(clerk-js): Use afterSwitchSessionUrl switching sessions within UserButton#4726
panteliselef merged 10 commits intomainfrom
elef/sdki-784-userbutton-not-using-afterswitchsessionurl-on-switching

Conversation

@panteliselef
Copy link
Copy Markdown
Contributor

@panteliselef panteliselef commented Dec 6, 2024

Description

When switching session within <UserButton/>, we should respect the value of afterSwitchSessionUrl. Only inside the SignInAccountSwitcher internal component we need to use afterSignInUrl.

Checklist

  • pnpm test runs as expected.
  • pnpm 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:

@panteliselef panteliselef requested review from a team and fadymak December 6, 2024 09:35
@panteliselef panteliselef self-assigned this Dec 6, 2024
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Dec 6, 2024

🦋 Changeset detected

Latest commit: 3e3e53d

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

@vercel
Copy link
Copy Markdown

vercel Bot commented Dec 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
clerk-js-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 10, 2024 2:34pm

}

if (redirectUrl) {
if (redirectUrl && !beforeEmit) {
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.

Clerk.signOut() uses both beforeEmit and redirectUrl which is an incorrect state as the redirection should be trigger by only one of them.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we fix signOut? Then throw in this if both beforeEmit and redirectUrl come through.

Comment thread packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx Outdated
Comment thread packages/clerk-js/src/ui/components/UserButton/useMultisessionActions.tsx Outdated
@panteliselef
Copy link
Copy Markdown
Contributor Author

!snapshot

@clerk-cookie
Copy link
Copy Markdown
Collaborator

Hey @panteliselef - the snapshot version command generated the following package versions:

Package Version
@clerk/astro 2.0.1-snapshot.v20241209201459
@clerk/backend 1.20.3-snapshot.v20241209201459
@clerk/chrome-extension 2.0.9-snapshot.v20241209201459
@clerk/clerk-js 5.40.3-snapshot.v20241209201459
@clerk/elements 0.21.4-snapshot.v20241209201459
@clerk/clerk-expo 2.4.5-snapshot.v20241209201459
@clerk/expo-passkeys 0.0.17-snapshot.v20241209201459
@clerk/express 1.3.26-snapshot.v20241209201459
@clerk/fastify 2.0.28-snapshot.v20241209201459
@clerk/localizations 3.8.2-snapshot.v20241209201459
@clerk/nextjs 6.8.3-snapshot.v20241209201459
@clerk/nuxt 0.0.13-snapshot.v20241209201459
@clerk/clerk-react 5.19.3-snapshot.v20241209201459
@clerk/react-router 0.0.2-snapshot.v20241209201459
@clerk/remix 4.3.6-snapshot.v20241209201459
@clerk/clerk-sdk-node 5.0.77-snapshot.v20241209201459
@clerk/shared 2.19.4-snapshot.v20241209201459
@clerk/tanstack-start 0.6.6-snapshot.v20241209201459
@clerk/testing 1.3.38-snapshot.v20241209201459
@clerk/themes 2.1.55-snapshot.v20241209201459
@clerk/types 4.39.2-snapshot.v20241209201459
@clerk/ui 0.2.4-snapshot.v20241209201459
@clerk/vue 0.0.17-snapshot.v20241209201459

Tip: Use the snippet copy button below to quickly install the required packages.
@clerk/astro

npm i @clerk/astro@2.0.1-snapshot.v20241209201459 --save-exact

@clerk/backend

npm i @clerk/backend@1.20.3-snapshot.v20241209201459 --save-exact

@clerk/chrome-extension

npm i @clerk/chrome-extension@2.0.9-snapshot.v20241209201459 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@5.40.3-snapshot.v20241209201459 --save-exact

@clerk/elements

npm i @clerk/elements@0.21.4-snapshot.v20241209201459 --save-exact

@clerk/clerk-expo

npm i @clerk/clerk-expo@2.4.5-snapshot.v20241209201459 --save-exact

@clerk/expo-passkeys

npm i @clerk/expo-passkeys@0.0.17-snapshot.v20241209201459 --save-exact

@clerk/express

npm i @clerk/express@1.3.26-snapshot.v20241209201459 --save-exact

@clerk/fastify

npm i @clerk/fastify@2.0.28-snapshot.v20241209201459 --save-exact

@clerk/localizations

npm i @clerk/localizations@3.8.2-snapshot.v20241209201459 --save-exact

@clerk/nextjs

npm i @clerk/nextjs@6.8.3-snapshot.v20241209201459 --save-exact

@clerk/nuxt

npm i @clerk/nuxt@0.0.13-snapshot.v20241209201459 --save-exact

@clerk/clerk-react

npm i @clerk/clerk-react@5.19.3-snapshot.v20241209201459 --save-exact

@clerk/react-router

npm i @clerk/react-router@0.0.2-snapshot.v20241209201459 --save-exact

@clerk/remix

npm i @clerk/remix@4.3.6-snapshot.v20241209201459 --save-exact

@clerk/clerk-sdk-node

npm i @clerk/clerk-sdk-node@5.0.77-snapshot.v20241209201459 --save-exact

@clerk/shared

npm i @clerk/shared@2.19.4-snapshot.v20241209201459 --save-exact

@clerk/tanstack-start

npm i @clerk/tanstack-start@0.6.6-snapshot.v20241209201459 --save-exact

@clerk/testing

npm i @clerk/testing@1.3.38-snapshot.v20241209201459 --save-exact

@clerk/themes

npm i @clerk/themes@2.1.55-snapshot.v20241209201459 --save-exact

@clerk/types

npm i @clerk/types@4.39.2-snapshot.v20241209201459 --save-exact

@clerk/ui

npm i @clerk/ui@0.2.4-snapshot.v20241209201459 --save-exact

@clerk/vue

npm i @clerk/vue@0.0.17-snapshot.v20241209201459 --save-exact

Copy link
Copy Markdown
Member

@Ephem Ephem left a comment

Choose a reason for hiding this comment

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

I've tested the snapshot focused on the afterSwitchSessionUrl and it works like a charm! Verified the bug was there before, upgraded and it went away. Found no way to break it. Not quite qualified to review the other changes.

This change will mean that if you relied on afterSignInUrl working before, things will break right (in my testing it did)? I think that's fine and expected considering it's undocumented and does not exist on the types.

beforeEmit: expect.any(Function),
redirectUrl: '/after-sign-out',
});
expect(sut.navigate).toHaveBeenCalledWith('/after-sign-out');
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.

this was only passing because we were always doing this, inside signOut

const defaultCb = () => this.navigate(redirectUrl);

@panteliselef panteliselef merged commit 85a36a8 into main Dec 12, 2024
@panteliselef panteliselef deleted the elef/sdki-784-userbutton-not-using-afterswitchsessionurl-on-switching branch December 12, 2024 14:01
wobsoriano pushed a commit that referenced this pull request Feb 8, 2025
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.

6 participants