Uh oh!
There was an error while loading. Please reload this page.
feat(clerk-js): Handle new session pending status as authenticated state - #5136
Conversation
🦋 Changeset detectedLatest commit: e69c175 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 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 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
pending session status as authenticated userpending status4b907c1 to
cdee2c9Comparecdee2c9 to
a9143d3Comparea9143d3 to
f21d6e7Comparef21d6e7 to
b3044d2Compareb3044d2 to
96a7629Compare96a7629 to
f249b85Comparef249b85 to
37f3c34Compare37f3c34 to
f51ee21Compare| return this.#options[key]; | ||
| } | ||
| get hasAuthenticatedClient(): boolean { |
There was a problem hiding this comment.
I propose Clerk.isAuthenticated, i don't like having "client" in the name, since as a developer using Clerk that term is unfamiliar to me (compared to user and session).
There was a problem hiding this comment.
I agree! I tried that out first but was on the fence about whether to mention "client"
There was a problem hiding this comment.
We use isSignedIn elsewhere, I would say we stick with that unless we feel strongly that isAuthenticated() is a better representation.
There was a problem hiding this comment.
I quite like that! Here's a comparison of how it'd play out with the current SignedIn control component vs a custom flow
<SignedIn><p> I have successfully signed up but I still have pending tasks </p></SignedIn>if(!Clerk.isSignedIn){mountSignIn()}else{if(Clerk.session.tasks){// ...}}I think the same nomenclature should be extended to the session arrays, so would be something like: signedInSessions
| return this.sessions.filter(s => s.status === 'active') as ActiveSessionResource[]; | ||
| } | ||
| get authenticatedSessions(): AuthenticatedSessionResource[] { |
There was a problem hiding this comment.
Curious, would authenticatedSessions be in conflict with sessions from an anonymous login ? Are you authenticated when you use anonymous login ? (thinking out loud here, you can ignore)
There was a problem hiding this comment.
Are you authenticated when you use anonymous login ?
I'd expect not, cause the user haven't gone through "identity verification" in a way
| ```diff | ||
| - if (Clerk.user) { | ||
| + if (clerk.hasAuthenticatedClient) { |
There was a problem hiding this comment.
| + if (clerk.hasAuthenticatedClient) { | |
| + if (Clerk.hasAuthenticatedClient) { |
| return this.#options[key]; | ||
| } | ||
| get hasAuthenticatedClient(): boolean { |
There was a problem hiding this comment.
The goal of this property is to deprecate/do not recommend the use of Clerk.user for custom flows to determine the authenticate state
Is this custom flows only or in their codebase overall ?
| return this._basePostBypass({ body: params, path: this.path() + '/verify' }); | ||
| } | ||
| get hasAuthenticated() { |
There was a problem hiding this comment.
Would recommend present tense instead of past-tense:
| gethasAuthenticated(){ | |
| getisAuthenticated(){ |
There was a problem hiding this comment.
I agree with Bryce here, present tense seems a bit better
| const currentSessionToken = client.authenticatedSessions.find(s => s.id === client.lastActiveSessionId); | ||
| const token = currentSessionToken?.lastActiveToken?.getRawString(); |
There was a problem hiding this comment.
| constcurrentSessionToken=client.authenticatedSessions.find(s=>s.id===client.lastActiveSessionId); | |
| consttoken=currentSessionToken?.lastActiveToken?.getRawString(); | |
| constcurrentSession=client.authenticatedSessions.find(s=>s.id===client.lastActiveSessionId); | |
| consttoken=currentSession?.lastActiveToken?.getRawString(); |
| const [UserContext, useUserContext] = createContextAndHook<UserResource | null | undefined>('UserContext'); | ||
| const [ClientContext, useClientContext] = createContextAndHook<ClientResource | null | undefined>('ClientContext'); | ||
| const [SessionContext, useSessionContext] = createContextAndHook<ActiveSessionResource | null | undefined>( | ||
| const [SessionContext, useSessionContext] = createContextAndHook<AuthenticatedSessionResource | null | undefined>( |
There was a problem hiding this comment.
I think this might be a breaking change, if people are importing ActiveSessionResource from @clerk/types directly and depending on the return type of useSession().
EDIT: nevermind! It looks like AuthenticatedSessionResource is inclusive of ActiveSessionResource, so we should be safe 👀
| const { navigateAfterSignOut, navigateAfterMultiSessionSingleSignOutUrl } = useSignOutContext(); | ||
| const handleSignOutSessionClicked = (session: ActiveSessionResource) => () => { | ||
| const handleSignOutSessionClicked = (session: SignedInSessionResource) => () => { |
There was a problem hiding this comment.
It's much more idiomatic now! Thanks for the feedback @brkalow 🫡
| - if (Clerk.user) { | ||
| + if (Clerk.isSignedIn) { |
There was a problem hiding this comment.
The isSignedIn property makes sense to me!
In the PR description, you mention that:
Clerk.user shouldn't be used to determine if the user has fully authenticated or not
But the logic in the Client resource tells me that isSignedIn is going to be true if the current session is active or pending, meaning that currently, isSignedIn cannot be used to check if the user is fully authenticated or not.
Is this intentional? If yes, could you please provide an example where Clerk.isSignedIn !== !!Clerk.user?
There was a problem hiding this comment.
meaning that currently, isSignedIn cannot be used to check if the user is fully authenticated or not.
The user has fully authenticated even with a pending status. Still, they have pending tasks to complete, eg: After sign-in, complete all factors but FAPI returns a pending session for tasks such as having to select an org.
Is this intentional? If yes, could you please provide an example where Clerk.isSignedIn !== !!Clerk.user
Currently, they are the same. This PR treats pending exactly like active to maintain current functionality and to incrementally add protections for it since the feature is gated on FAPI and toggled via Dashboard.
That property was added to avoid relying only on the user object, or Clerk.activeSessions.length > 0 for our internal "signed-in" state checks.
Once we introduce tasks (#5170) -> I'd add another property that specifically checks if the user has a session that resolved all pending tasks, something like:
// Would resolve to `false` for session.status === 'pending' // Would resolve to `true` for session.status === 'active'if(Clerk.hasValidSession){clerk.mountUserButton(userbuttonDiv)}else{clerk.mountSignIn(signInDiv)}As a syntax sugar so that developers don't have to manually check for the session statuses on custom flows as well.
There was a problem hiding this comment.
I actually acknowledged that I misplaced "Clerk.user shouldn't be used to determine if the user has fully authenticated or not" statement and updated the changeset here
isSignedIn doesn't necessarily replace Clerk.user, but it does act like a syntax sugar to deprecate any manual references to Clerk.client.activeSessions.length > 0, !!Clerk.user or !!Clerk.session
144mdgross
commented
Mar 25, 2025
Hello! Sorry if this is the wrong place to ask but I wasn't able to get my discord account working. I begin getting type errors (error TS2322) when updating to version 4.47.0 of types. Is this expected and is it resolved by an update to the corresponding clerk packages? We are using |
Description
Context
Introducing a new FAPI session status:
pending. It builds a fundamental layer for the after-auth project and the future concept of tasks, eg: Forcing to select an organization after sign-in.Previously, only
activesessions were considered to be in an authenticated state. Now, we're introducing a new status assigned to the user session after a successful authentication process but when there are pending tasks.Next steps
These changes do not introduce new behavior on helpers / AIO components, neither breaking changes, as it handles
pendingas an authenticated state, pairing the same functionality asactiveNext PRs will start introducing the concept of pending tasks and enforcing resolution upon after-auth.
Developer-facing changes
Once these clerk-js get served, developers shouldn't have to worry about changing their app logic or breaking changes. However, some interface changes are preparing the DX for the next steps mentioned above:
Unifying an signed-in state check based on the session status with
Clerk.isSignedInorClerk.client.isSignedInFor custom flows,
Clerk.usershouldn't be used to determine if the user has fully authenticated or not, and also, it shouldn't be necessary for developers to explicitly check against the session as it's prone for breaking changes, therefore we're abstracting this behind a new property.Deprecating
activeSessionsin favor ofsignedInSessionsDeprecating explicit checks against "active" sessions in favor of a generic property that expresses the "signed-in" state instead, since in the future, we might add other types of session statuses for different levels of user verification as we're doing now with after-auth.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change