fix(clerk-js): Deprecate afterSignOutUrl from UserButton#3544
fix(clerk-js): Deprecate afterSignOutUrl from UserButton#3544panteliselef merged 7 commits intomainfrom
afterSignOutUrl from UserButton#3544Conversation
Also handles in introduction of `afterMultiSessionSingleSignOutUrl` in ClerkProvider
🦋 Changeset detectedLatest commit: 29a930c The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 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 |
panteliselef
changed the title
afterSignOutUrl from UserButton.afterSignOutUrl from UserButton
| // TODO: Investigate if we need to call `setActive` with {session: null} | ||
| if (otherSessions.length === 0) { | ||
| return navigateAfterSignOut(); | ||
| } | ||
| await navigateAfterMultiSessionSingleSignOutUrl(); |
There was a problem hiding this comment.
💭 I wouldn't necessarily expect that this method would need to be aware of multi-session. I understand why it was done this way as part of this PR, but it feels like a better internal abstraction might be to consolidate the multi-session logic into navigateAfterSignOut().
There was a problem hiding this comment.
I think creating an abstraction here would make our lifer harder, we have useMultisessionActions which needs to know the difference between single session sign out and multisession sign out. We can refactor but i don't see a clear benefit here.
There was a problem hiding this comment.
Benefit would be not having to spread some of this logic across many different methods. That's fine to leave it for now, let's jut keep an eye on these code paths and we can abstract if it feels like multi-session logic is too difficult to manage.
| await user.delete(); | ||
| await navigateAfterSignOut(); | ||
|
|
||
| // TODO: Investigate if we need to call `setActive` with {session: null} |
There was a problem hiding this comment.
Do we need to do this investigation before merge?
There was a problem hiding this comment.
I was waiting for #3518 to be merged, and i will check again, but simply calling setActive would not refresh the router.
There was a problem hiding this comment.
So i investigated further, even with #3518 the issue persisted. I've identified this as a FAPI issue, because after user.delete() client_uat comes back with a valid timestamp instead of being 0. This will need a separate ticket to be address and should not be part of this PR
brkalow
left a comment
brkalow
left a comment
There was a problem hiding this comment.
Left a comment on the changelog, but otherwise looks good.
panteliselef
deleted the
elef/sdk-1759-deprecate-afterSignOutUrl-userbutton
branch
5 participants
Description
Fixes: DASH-183
This PR address the following issues:
afterSignOutUrlbut when user deletes their account, the providedafterSignOutUrlis not respected.afterMultiSessionSingleSignOutUrlin user button but the user when they delete one of their accounts is not redirected to the appropriate url defined as prop.The main issue is that we don't propagate
afterSignOutUrlandafterMultiSessionSingleSignOutUrlfrom the UserButton to UserProfile.This PR addresses that by positioning those props at the root lever (ClerkProvider or Clerk.load). We considered simply introducing those props to UserProfile, but the naming would not be ideal. We also considered renaming them, but at that point you want the components to always be in sync, so we positioned them at the root.
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change