{orgId}
+sign-out
+sign-in
; +} diff --git a/integration/templates/next-app-router/src/app/transitive-state/user-button/layout.tsx b/integration/templates/next-app-router/src/app/transitive-state/user-button/layout.tsx new file mode 100644 index 00000000000..838e7ec86c2 --- /dev/null +++ b/integration/templates/next-app-router/src/app/transitive-state/user-button/layout.tsx @@ -0,0 +1,47 @@ +'use client'; + +import { UserButton, useAuth } from '@clerk/nextjs'; +import { useState } from 'react'; +import { usePathname } from 'next/navigation'; + +function EmissionLog() { + const { userId } = useAuth(); + const pathname = usePathname(); + const [log, setLog] = useStateinitial
; +} diff --git a/integration/templates/next-app-router/src/app/transitive-state/user-button/switched/page.tsx b/integration/templates/next-app-router/src/app/transitive-state/user-button/switched/page.tsx new file mode 100644 index 00000000000..9eb3ec71004 --- /dev/null +++ b/integration/templates/next-app-router/src/app/transitive-state/user-button/switched/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + returnswitched
; +} diff --git a/integration/tests/sign-out-smoke.test.ts b/integration/tests/sign-out-smoke.test.ts index 950269bd35f..6b040080bd5 100644 --- a/integration/tests/sign-out-smoke.test.ts +++ b/integration/tests/sign-out-smoke.test.ts @@ -87,6 +87,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('sign out await u.page.getByRole('button', { name: 'Open user menu' }).click(); await u.page.getByRole('menuitem', { name: 'Sign out' }).click(); + await u.po.expect.toBeSignedOut(); await u.page.getByRole('link', { name: 'Protected', exact: true }).click(); await u.page.waitForURL(url => url.href.includes('/sign-in?redirect_url')); }); diff --git a/integration/tests/transitive-state.test.ts b/integration/tests/transitive-state.test.ts new file mode 100644 index 00000000000..374743cbc4a --- /dev/null +++ b/integration/tests/transitive-state.test.ts @@ -0,0 +1,262 @@ +import { parsePublishableKey } from '@clerk/shared/keys'; +import { clerkSetup } from '@clerk/testing/playwright'; +import { expect, test } from '@playwright/test'; + +import { appConfigs } from '../presets'; +import type { FakeOrganization, FakeUser } from '../testUtils'; +import { createTestUtils, testAgainstRunningApps } from '../testUtils'; + +/* + These tests verify that useAuth emits the correct transitive state sequence when switching + auth context (org or user) with navigation. The expected pattern is: + Path A - Value A, Path A - undefined, Path B - undefined, Path B - Value B +*/ + +testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('transitive state @nextjs', ({ app }) => { + //test.describe.configure({ mode: 'serial' }); + + let fakeUser: FakeUser; + let orgA: FakeOrganization; + let orgB: FakeOrganization; + let userA: FakeUser; + let userB: FakeUser; + let userAId: string; + let userBId: string; + + test.beforeAll(async () => { + const u = createTestUtils({ app }); + + const publishableKey = appConfigs.envs.withEmailCodes.publicVariables.get('CLERK_PUBLISHABLE_KEY'); + const secretKey = appConfigs.envs.withEmailCodes.privateVariables.get('CLERK_SECRET_KEY'); + const apiUrl = appConfigs.envs.withEmailCodes.privateVariables.get('CLERK_API_URL'); + const { frontendApi: frontendApiUrl } = parsePublishableKey(publishableKey); + + await clerkSetup({ + publishableKey, + frontendApiUrl, + secretKey, + // @ts-expect-error Not typed + apiUrl, + dotenv: false, + }); + + // Org switching test: 1 user with 2 orgs + fakeUser = u.services.users.createFakeUser(); + const user = await u.services.users.createBapiUser(fakeUser); + orgB = await u.services.users.createFakeOrganization(user.id); + orgA = await u.services.users.createFakeOrganization(user.id); + + // User switching test: 2 users for multi-session + userA = u.services.users.createFakeUser(); + userB = u.services.users.createFakeUser(); + const createdUserA = await u.services.users.createBapiUser(userA); + const createdUserB = await u.services.users.createBapiUser(userB); + userAId = createdUserA.id; + userBId = createdUserB.id; + }); + + test.afterAll(async () => { + await orgA.delete(); + await orgB.delete(); + await fakeUser.deleteIfExists(); + await userA.deleteIfExists(); + await userB.deleteIfExists(); + await app.teardown(); + }); + + test('should emit correct transitive auth state when switching orgs with navigation', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + await u.po.signIn.goTo(); + await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password }); + await u.po.expect.toBeSignedIn(); + + const pathA = `/transitive-state/organization-switcher/${orgA.organization.id}`; + const pathB = `/transitive-state/organization-switcher/${orgB.organization.id}`; + + await u.po.page.goToRelative(pathA); + + // Wait for initial state to settle - emission log should contain pathA with orgA + await test + .expect(u.po.page.getByTestId('emission-log').locator(`li:has-text("${pathA} - ${orgA.organization.id}")`)) + .toBeVisible(); + + // Switch to orgB via OrganizationSwitcher + await u.po.organizationSwitcher.waitForMounted(); + await u.po.organizationSwitcher.waitForAnOrganizationToSelected(); + await u.po.organizationSwitcher.toggleTrigger(); + await test.expect(u.page.locator('.cl-organizationSwitcherPopoverCard')).toBeVisible(); + await u.page.getByText(orgB.name, { exact: true }).click(); + + // Wait for transition to complete - current-org-id shows orgB + await test.expect(u.po.page.getByTestId('current-org-id')).toHaveText(orgB.organization.id); + + // Assert the emission sequence: last 4 entries are Path A - Org A, Path A - undefined, Path B - undefined, Path B - Org B + const emissionItems = u.po.page.getByTestId('emission-log').locator('li'); + const count = await emissionItems.count(); + const texts: string[] = []; + for (let i = 0; i < count; i++) { + texts.push((await emissionItems.nth(i).textContent()) ?? ''); + } + + expect(texts.slice(-4)).toEqual([ + `${pathA} - ${orgA.organization.id}`, + `${pathA} - undefined`, + `${pathB} - undefined`, + `${pathB} - ${orgB.organization.id}`, + ]); + }); + + test('should emit correct transitive auth state when switching users with navigation', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + + const pathInitial = '/transitive-state/user-button'; + const pathSwitched = '/transitive-state/user-button/switched'; + + // Clear session from previous test + await context.clearCookies(); + + // Sign in as userA + await u.po.signIn.goTo(); + await u.po.signIn.signInWithEmailAndInstantPassword({ email: userA.email, password: userA.password }); + await u.po.expect.toBeSignedIn(); + + // Sign in as userB to create second session (multi-session) + await u.po.signIn.goTo(); + await u.po.signIn.setIdentifier(userB.email); + await u.po.signIn.continue(); + await u.po.signIn.setPassword(userB.password); + await u.po.signIn.continue(); + + // Avoid backend rate-limiting on session touch + await new Promise(resolve => setTimeout(resolve, 3000)); + + // Navigate to user-button page (userB is active) + await u.po.page.goToRelative(pathInitial); + + // Wait for initial state to settle - emission log should contain pathInitial with userB + await test + .expect(u.po.page.getByTestId('emission-log').locator(`li:has-text("${pathInitial} - ${userBId}")`)) + .toBeVisible(); + + // Switch to userA via UserButton + await u.po.userButton.waitForMounted(); + await u.po.userButton.toggleTrigger(); + await u.po.userButton.waitForPopover(); + await u.po.userButton.switchAccount(userA.email); + await u.po.userButton.waitForPopoverClosed(); + + // Wait for navigation to switched page + await test.expect(u.po.page.getByTestId('page-name')).toHaveText('switched'); + + // Assert the emission sequence + const emissionItems = u.po.page.getByTestId('emission-log').locator('li'); + const count = await emissionItems.count(); + const texts: string[] = []; + for (let i = 0; i < count; i++) { + texts.push((await emissionItems.nth(i).textContent()) ?? ''); + } + + expect(texts.slice(-4)).toEqual([ + `${pathInitial} - ${userBId}`, + `${pathInitial} - undefined`, + `${pathSwitched} - undefined`, + `${pathSwitched} - ${userAId}`, + ]); + }); + + test('should emit correct transitive auth state when signing out with navigation', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + + await context.clearCookies(); + + // Sign in as userA + await u.po.signIn.goTo(); + await u.po.signIn.signInWithEmailAndInstantPassword({ email: userA.email, password: userA.password }); + await u.po.expect.toBeSignedIn(); + + const pathA = '/transitive-state/sign-out'; + const pathB = '/transitive-state/sign-out/sign-in'; + + // Navigate to sign-out page + await u.po.page.goToRelative(pathA); + + // Wait for initial state to settle + await test + .expect(u.po.page.getByTestId('emission-log').locator(`li:has-text("${pathA} - ${userAId}")`)) + .toBeVisible(); + + // Click SignOutButton + await u.page.getByRole('button', { name: 'Sign out' }).click(); + + // Wait for navigation to sign-in page + await test.expect(u.po.page.getByTestId('page-name')).toHaveText('sign-in'); + + // Assert emission sequence + const emissionItems = u.po.page.getByTestId('emission-log').locator('li'); + const count = await emissionItems.count(); + const texts: string[] = []; + for (let i = 0; i < count; i++) { + texts.push((await emissionItems.nth(i).textContent()) ?? ''); + } + + expect(texts.slice(-4)).toEqual([ + `${pathA} - ${userAId}`, + `${pathA} - undefined`, + `${pathB} - undefined`, + `${pathB} - null`, + ]); + }); + + test('should emit correct transitive auth state when signing out with navigation (multi-session)', async ({ + page, + context, + }) => { + const u = createTestUtils({ app, page, context }); + + await context.clearCookies(); + + // Sign in as userA + await u.po.signIn.goTo(); + await u.po.signIn.signInWithEmailAndInstantPassword({ email: userA.email, password: userA.password }); + await u.po.expect.toBeSignedIn(); + + // Sign in as userB to create second session (multi-session) + await u.po.signIn.goTo(); + await u.po.signIn.setIdentifier(userB.email); + await u.po.signIn.continue(); + await u.po.signIn.setPassword(userB.password); + await u.po.signIn.continue(); + + const pathA = '/transitive-state/sign-out'; + const pathB = '/transitive-state/sign-out/sign-in'; + + // Navigate to sign-out page + await u.po.page.goToRelative(pathA); + + // Wait for initial state to settle + await test + .expect(u.po.page.getByTestId('emission-log').locator(`li:has-text("${pathA} - ${userBId}")`)) + .toBeVisible(); + + // Click SignOutButton + await u.page.getByRole('button', { name: 'Sign out' }).click(); + + // Wait for navigation to sign-in page + await test.expect(u.po.page.getByTestId('page-name')).toHaveText('sign-in'); + + // Assert emission sequence + const emissionItems = u.po.page.getByTestId('emission-log').locator('li'); + const count = await emissionItems.count(); + const texts: string[] = []; + for (let i = 0; i < count; i++) { + texts.push((await emissionItems.nth(i).textContent()) ?? ''); + } + + expect(texts.slice(-4)).toEqual([ + `${pathA} - ${userBId}`, + `${pathA} - undefined`, + `${pathB} - undefined`, + `${pathB} - null`, + ]); + }); +}); diff --git a/packages/clerk-js/bundlewatch.config.json b/packages/clerk-js/bundlewatch.config.json index c694488eb56..3114f47435d 100644 --- a/packages/clerk-js/bundlewatch.config.json +++ b/packages/clerk-js/bundlewatch.config.json @@ -3,7 +3,7 @@ { "path": "./dist/clerk.js", "maxSize": "539KB" }, { "path": "./dist/clerk.browser.js", "maxSize": "66KB" }, { "path": "./dist/clerk.chips.browser.js", "maxSize": "66KB" }, - { "path": "./dist/clerk.legacy.browser.js", "maxSize": "106KB" }, + { "path": "./dist/clerk.legacy.browser.js", "maxSize": "108KB" }, { "path": "./dist/clerk.no-rhc.js", "maxSize": "307KB" }, { "path": "./dist/clerk.native.js", "maxSize": "65KB" }, { "path": "./dist/vendors*.js", "maxSize": "7KB" }, diff --git a/packages/clerk-js/src/core/__tests__/clerk.test.ts b/packages/clerk-js/src/core/__tests__/clerk.test.ts index b467a476cad..1221882a40e 100644 --- a/packages/clerk-js/src/core/__tests__/clerk.test.ts +++ b/packages/clerk-js/src/core/__tests__/clerk.test.ts @@ -2607,6 +2607,35 @@ describe('Clerk singleton', () => { expect(mockOnAfterSetActive).toHaveBeenCalledTimes(1); }); }); + + it('does not emit to listeners when __internal_dangerouslySkipEmit is true', () => { + const mockSession = { + id: 'session_1', + status: 'active', + user: { id: 'user_1' }, + lastActiveToken: { getRawString: () => 'token_1' }, + }; + + const mockClient = { + sessions: [mockSession], + signedInSessions: [mockSession], + lastActiveSessionId: 'session_1', + }; + + const sut = new Clerk(productionPublishableKey); + sut.updateClient(mockClient as any); + + const listener = vi.fn(); + const unsubscribe = sut.addListener(listener, { skipInitialEmit: true }); + + listener.mockClear(); + + sut.updateClient(mockClient as any, { __internal_dangerouslySkipEmit: true }); + + unsubscribe(); + + expect(listener).not.toHaveBeenCalled(); + }); }); describe('__internal_attemptToEnableEnvironmentSetting', () => { diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 0ddb68921bf..f86092c2d0d 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -176,6 +176,7 @@ import { APIKeys } from './modules/apiKeys'; import { Billing } from './modules/billing'; import { createCheckoutInstance } from './modules/checkout/instance'; import { Protect } from './protect'; +import type { Session } from './resources/internal'; import { BaseResource, Client, Environment, Organization, Waitlist } from './resources/internal'; import { State } from './state'; @@ -615,8 +616,6 @@ export class Clerk implements ClerkInterface { // Notify other tabs that user is signing out and clean up cookies. eventBus.emit(events.UserSignOut, null); - this.#setTransitiveState(); - await tracker.track(async () => { if (signOutCallback) { await signOutCallback(); @@ -640,7 +639,10 @@ export class Clerk implements ClerkInterface { * Since we are calling `onBeforeSetActive` before signing out, we should NOT pass `"sign-out"`. */ await onBeforeSetActive(); + if (!opts.sessionId || this.client.signedInSessions.length === 1) { + this.#setTransitiveState(); + if (this.#options.experimental?.persistClient ?? true) { await this.client.removeSessions(); } else { @@ -657,11 +659,13 @@ export class Clerk implements ClerkInterface { 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) { + this.#setTransitiveState(); + await session?.remove(); await executeSignOut(); debugLogger.info('signOut() complete', { redirectUrl: stripOrigin(redirectUrl) }, 'clerk'); + } else { + await session?.remove(); } }; @@ -1550,13 +1554,53 @@ export class Clerk implements ClerkInterface { await onBeforeSetActive(newSession === null ? 'sign-out' : undefined); } + const taskUrl = + newSession?.status === 'pending' && + newSession?.currentTask && + this.#options.taskUrls?.[newSession?.currentTask.key]; + const shouldNavigate = !!(redirectUrl || taskUrl || setActiveNavigate); + //1. setLastActiveSession to passed user session (add a param). // Note that this will also update the session's active organization // id. + /* + The introduction of useSyncExternalStore introduced a bug here, where the + updateClient that happens as a result of this touch emitted and immediately + caused components to re-render. Previously, that emit was batched with the + setTransitiveState call. + + This whole block should likely happen later, after the transitive state is set. + + Because we want to minimize behavioral changes until we can tackle this properly, + this was refactored so that updateClient does not emit under these circumstances. + */ if (inActiveBrowserTab() || !this.#options.standardBrowser) { - await this.#touchCurrentSession(newSession); - // reload session from updated client - newSession = this.#getSessionFromClient(newSession?.id); + let updatedClient: ClientResource | undefined; + if (shouldNavigate && newSession && '__internal_touch' in newSession) { + try { + // __internal_touch does not call updateClient automatically + updatedClient = await (newSession as Session).__internal_touch(); + if (updatedClient) { + // We call updateClient manually, but without letting it emit + // It's important that the setTransitiveState call happens somewhat + // quickly after this, as during this period, the internal Clerk + // state does not match the emitted state. + this.updateClient(updatedClient, { __internal_dangerouslySkipEmit: true }); + } + } catch (e) { + if (is4xxError(e)) { + void this.handleUnauthenticated(); + } else { + throw e; + } + } + } else { + await this.#touchCurrentSession(newSession); + } + // If we do have the updatedClient, read from that, otherwise getSessionFromClient + // will fallback to this.client. This makes no difference now, but will if we + // decide to move the `updateClient` call out of this block later. + newSession = this.#getSessionFromClient(newSession?.id, updatedClient); } // getToken syncs __session and __client_uat to cookies using events.TokenUpdate dispatched event. @@ -1583,12 +1627,7 @@ export class Clerk implements ClerkInterface { // automatic reloading when reloading shouldn't be happening. const tracker = createBeforeUnloadTracker(this.#options.standardBrowser); - const taskUrl = - newSession?.status === 'pending' && - newSession?.currentTask && - this.#options.taskUrls?.[newSession?.currentTask.key]; - - if (redirectUrl || taskUrl || setActiveNavigate) { + if (shouldNavigate) { 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. @@ -2657,7 +2696,10 @@ export class Clerk implements ClerkInterface { this.internal_last_error = value; } - updateClient = (newClient: ClientResource): void => { + // Skipping emit is dangerous because if there is a gap between setting these + // and emitting, library consumers that both read state directly and set up listeners + // could end up in a inconsistent state. + updateClient = (newClient: ClientResource, options?: { __internal_dangerouslySkipEmit?: boolean }): void => { if (!this.client) { // This is the first time client is being // set, so we also need to set session @@ -2702,7 +2744,9 @@ export class Clerk implements ClerkInterface { eventBus.emit(events.TokenUpdate, { token: this.session?.lastActiveToken }); } - this.#emit(); + if (!options?.__internal_dangerouslySkipEmit) { + this.#emit(); + } }; get __internal_environment(): EnvironmentResource | null | undefined { diff --git a/packages/clerk-js/src/core/resources/Base.ts b/packages/clerk-js/src/core/resources/Base.ts index cc1b5db9819..499f8f51f96 100644 --- a/packages/clerk-js/src/core/resources/Base.ts +++ b/packages/clerk-js/src/core/resources/Base.ts @@ -14,10 +14,11 @@ import { clerkMissingFapiClientInResources } from '../errors'; import type { FapiClient, FapiRequestInit, FapiResponse, FapiResponseJSON, HTTPMethod } from '../fapiClient'; import { FraudProtection } from '../fraudProtection'; import type { Clerk } from './internal'; -import { Client } from './internal'; +import { getClientResourceFromPayload } from './internal'; export type BaseFetchOptions = ClerkResourceReloadParams & { forceUpdateClient?: boolean; + skipUpdateClient?: boolean; fetchMaxTries?: number; }; @@ -123,7 +124,7 @@ export abstract class BaseResource { } // TODO: Link to Client payload piggybacking design document - if (requestInit.method !== 'GET' || opts.forceUpdateClient) { + if ((requestInit.method !== 'GET' || opts.forceUpdateClient) && !opts.skipUpdateClient) { this._updateClient