-
Notifications
You must be signed in to change notification settings - Fork 320
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
fix(clerk-js): Remove setActive() call from signOut() #5171
base: main
Are you sure you want to change the base?
Changes from all commits
70a7429
6fff5bc
4032148
5260348
b9d496e
ea3bf5e
5172ee6
e3af753
a6b4789
bb63e4e
0bc89cc
ccd7847
e2d9d6f
8805538
5e690cc
d35bf14
77339d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@clerk/clerk-js': patch | ||
--- | ||
|
||
Fixes an issue where calling `signOut()` would not always redirect. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,7 +82,6 @@ import { | |
getClerkQueryParam, | ||
getWeb3Identifier, | ||
hasExternalAccountSignUpError, | ||
ignoreEventValue, | ||
inActiveBrowserTab, | ||
inBrowser, | ||
isDevAccountPortalOrigin, | ||
|
@@ -128,6 +127,8 @@ import { | |
} from './resources/internal'; | ||
import { warnings } from './warnings'; | ||
|
||
type SetActiveHook = (intent?: 'sign-out') => void | Promise<void>; | ||
|
||
export type ClerkCoreBroadcastChannelEvent = { type: 'signout' }; | ||
|
||
declare global { | ||
|
@@ -372,43 +373,79 @@ export class Clerk implements ClerkInterface { | |
if (!this.client || this.client.sessions.length === 0) { | ||
return; | ||
} | ||
|
||
const onBeforeSetActive: SetActiveHook = | ||
typeof window !== 'undefined' && typeof window.__unstable__onBeforeSetActive === 'function' | ||
? window.__unstable__onBeforeSetActive | ||
: noop; | ||
|
||
const onAfterSetActive: SetActiveHook = | ||
typeof window !== 'undefined' && typeof window.__unstable__onAfterSetActive === 'function' | ||
? window.__unstable__onAfterSetActive | ||
: noop; | ||
|
||
const opts = callbackOrOptions && typeof callbackOrOptions === 'object' ? callbackOrOptions : options || {}; | ||
|
||
const redirectUrl = opts?.redirectUrl || this.buildAfterSignOutUrl(); | ||
const signOutCallback = typeof callbackOrOptions === 'function' ? callbackOrOptions : undefined; | ||
|
||
const handleSetActive = () => { | ||
const signOutCallback = typeof callbackOrOptions === 'function' ? callbackOrOptions : undefined; | ||
const executeSignOut = async () => { | ||
const tracker = createBeforeUnloadTracker(this.#options.standardBrowser); | ||
|
||
/** | ||
* Hint to each framework, that the user will be signed out. | ||
*/ | ||
await onBeforeSetActive('sign-out'); | ||
// Notify other tabs that user is signing out. | ||
eventBus.dispatch(events.UserSignOut, null); | ||
if (signOutCallback) { | ||
return this.setActive({ | ||
session: null, | ||
beforeEmit: ignoreEventValue(signOutCallback), | ||
}); | ||
} | ||
// Clean up cookies | ||
eventBus.dispatch(events.TokenUpdate, { token: null }); | ||
|
||
return this.setActive({ | ||
session: null, | ||
redirectUrl, | ||
this.#setTransitiveState(); | ||
|
||
await tracker.track(async () => { | ||
if (signOutCallback) { | ||
await signOutCallback(); | ||
} else { | ||
await this.navigate(redirectUrl); | ||
} | ||
}); | ||
|
||
if (tracker.isUnloading()) { | ||
return; | ||
} | ||
|
||
this.#setAccessors(); | ||
this.#emit(); | ||
|
||
await onAfterSetActive(); | ||
}; | ||
|
||
/** | ||
* Clears the router cache for `@clerk/nextjs` on all routes except the current one. | ||
* Note: Calling `onBeforeSetActive` before signing out, allows for new RSC prefetch requests to render as signed in. | ||
*/ | ||
await onBeforeSetActive(); | ||
if (!opts.sessionId || this.client.signedInSessions.length === 1) { | ||
if (this.#options.experimental?.persistClient ?? true) { | ||
await this.client.removeSessions(); | ||
} else { | ||
await this.client.destroy(); | ||
} | ||
|
||
return handleSetActive(); | ||
await executeSignOut(); | ||
|
||
return; | ||
} | ||
|
||
// Multi-session handling | ||
const session = this.client.signedInSessions.find(s => s.id === opts.sessionId); | ||
const shouldSignOutCurrent = session?.id && this.session?.id === session.id; | ||
|
||
await session?.remove(); | ||
|
||
if (shouldSignOutCurrent) { | ||
return handleSetActive(); | ||
await executeSignOut(); | ||
} | ||
}; | ||
|
||
|
@@ -872,7 +909,6 @@ export class Clerk implements ClerkInterface { | |
); | ||
} | ||
|
||
type SetActiveHook = () => void | Promise<void>; | ||
const onBeforeSetActive: SetActiveHook = | ||
typeof window !== 'undefined' && typeof window.__unstable__onBeforeSetActive === 'function' | ||
? window.__unstable__onBeforeSetActive | ||
|
@@ -913,7 +949,10 @@ export class Clerk implements ClerkInterface { | |
eventBus.dispatch(events.TokenUpdate, { token: session.lastActiveToken }); | ||
} | ||
|
||
await onBeforeSetActive(); | ||
/** | ||
* Hint to each framework, that the user will be signed out when `{session: null}` is provided. | ||
*/ | ||
await onBeforeSetActive(newSession === null ? 'sign-out' : undefined); | ||
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. We need to support the case of |
||
|
||
//1. setLastActiveSession to passed user session (add a param). | ||
// Note that this will also update the session's active organization | ||
|
@@ -934,40 +973,41 @@ export class Clerk implements ClerkInterface { | |
// undefined, then wait for beforeEmit to complete before emitting the new session. | ||
// When undefined, neither SignedIn nor SignedOut renders, which avoids flickers or | ||
// automatic reloading when reloading shouldn't be happening. | ||
const beforeUnloadTracker = this.#options.standardBrowser ? createBeforeUnloadTracker() : undefined; | ||
const tracker = createBeforeUnloadTracker(this.#options.standardBrowser); | ||
|
||
if (beforeEmit) { | ||
deprecated( | ||
'Clerk.setActive({beforeEmit})', | ||
'Use the `redirectUrl` property instead. Example `Clerk.setActive({redirectUrl:"/"})`', | ||
); | ||
beforeUnloadTracker?.startTracking(); | ||
this.#setTransitiveState(); | ||
await beforeEmit(newSession); | ||
beforeUnloadTracker?.stopTracking(); | ||
await tracker.track(async () => { | ||
this.#setTransitiveState(); | ||
await beforeEmit(newSession); | ||
}); | ||
} | ||
|
||
if (redirectUrl && !beforeEmit) { | ||
beforeUnloadTracker?.startTracking(); | ||
this.#setTransitiveState(); | ||
|
||
if (this.client.isEligibleForTouch()) { | ||
const absoluteRedirectUrl = new URL(redirectUrl, window.location.href); | ||
|
||
await this.navigate(this.buildUrlWithAuth(this.client.buildTouchUrl({ redirectUrl: absoluteRedirectUrl }))); | ||
} else { | ||
await this.navigate(redirectUrl); | ||
} | ||
|
||
beforeUnloadTracker?.stopTracking(); | ||
await tracker.track(async () => { | ||
if (!this.client) { | ||
// Typescript is not happy because since thinks this.client might have changed to undefined because the function is asynchronous. | ||
return; | ||
} | ||
this.#setTransitiveState(); | ||
if (this.client.isEligibleForTouch()) { | ||
const absoluteRedirectUrl = new URL(redirectUrl, window.location.href); | ||
await this.navigate(this.buildUrlWithAuth(this.client.buildTouchUrl({ redirectUrl: absoluteRedirectUrl }))); | ||
} else { | ||
await this.navigate(redirectUrl); | ||
} | ||
}); | ||
} | ||
|
||
//3. Check if hard reloading (onbeforeunload). If not, set the user/session and emit | ||
if (beforeUnloadTracker?.isUnloading()) { | ||
//3. Check if hard reloading (onbeforeunload). If not, set the user/session and emit | ||
if (tracker.isUnloading()) { | ||
return; | ||
} | ||
|
||
this.#setAccessors(newSession); | ||
|
||
this.#emit(); | ||
await onAfterSetActive(); | ||
}; | ||
|
@@ -2127,17 +2167,13 @@ export class Clerk implements ClerkInterface { | |
#setAccessors = (session?: SignedInSessionResource | null) => { | ||
this.session = session || null; | ||
this.organization = this.#getLastActiveOrganizationFromSession(); | ||
this.#aliasUser(); | ||
this.user = this.session ? this.session.user : null; | ||
}; | ||
|
||
#getSessionFromClient = (sessionId: string | undefined): SignedInSessionResource | null => { | ||
return this.client?.signedInSessions.find(x => x.id === sessionId) || null; | ||
}; | ||
|
||
#aliasUser = () => { | ||
this.user = this.session ? this.session.user : null; | ||
}; | ||
|
||
#handleImpersonationFab = () => { | ||
this.addListener(({ session }) => { | ||
const isImpersonating = !!session?.actor; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,7 +55,7 @@ const NextClientClerkProvider = (props: NextClerkProviderProps) => { | |
}, [isPending]); | ||
|
||
useSafeLayoutEffect(() => { | ||
window.__unstable__onBeforeSetActive = () => { | ||
window.__unstable__onBeforeSetActive = intent => { | ||
/** | ||
* We need to invalidate the cache in case the user is navigating to a page that | ||
* was previously cached using the auth state that was active at the time. | ||
|
@@ -78,11 +78,15 @@ const NextClientClerkProvider = (props: NextClerkProviderProps) => { | |
return new Promise(res => { | ||
window.__clerk_internal_invalidateCachePromise = res; | ||
|
||
const nextVersion = window?.next?.version || ''; | ||
|
||
// NOTE: the following code will allow `useReverification()` to work properly when `handlerReverification` is called inside `startTransition` | ||
if (window.next?.version && typeof window.next.version === 'string' && window.next.version.startsWith('13')) { | ||
if (nextVersion.startsWith('13')) { | ||
startTransition(() => { | ||
router.refresh(); | ||
}); | ||
} else if (nextVersion.startsWith('15') && intent === 'sign-out') { | ||
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. On next 15 we still want to fire the server action when setting a session or when selecting an organization. |
||
res(); // noop | ||
} else { | ||
void invalidateCacheAction().then(() => res()); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notice that we call
onBeforeSetActive
twice