diff --git a/.changeset/modern-cars-fall.md b/.changeset/modern-cars-fall.md new file mode 100644 index 00000000000..f54b7a6bc1d --- /dev/null +++ b/.changeset/modern-cars-fall.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': patch +--- + +Optimize Session.#hydrateCache to only cache token if it's new/different diff --git a/packages/clerk-js/src/core/__tests__/tokenCache.test.ts b/packages/clerk-js/src/core/__tests__/tokenCache.test.ts index accf272dc66..a5d7f892bb8 100644 --- a/packages/clerk-js/src/core/__tests__/tokenCache.test.ts +++ b/packages/clerk-js/src/core/__tests__/tokenCache.test.ts @@ -277,6 +277,41 @@ describe('SessionTokenCache', () => { // Critical: postMessage should NOT be called when handling a broadcast expect(mockBroadcastChannel.postMessage).not.toHaveBeenCalled(); }); + + it('always broadcasts regardless of cache state', async () => { + mockBroadcastChannel.postMessage.mockClear(); + + const tokenId = 'sess_2GbDB4enNdCa5vS1zpC3Xzg9tK9'; + const tokenResolver = Promise.resolve( + new Token({ + id: tokenId, + jwt: mockJwt, + object: 'token', + }) as TokenResource, + ); + + SessionTokenCache.set({ tokenId, tokenResolver }); + await tokenResolver; + + expect(mockBroadcastChannel.postMessage).toHaveBeenCalledTimes(1); + const firstCall = mockBroadcastChannel.postMessage.mock.calls[0][0]; + expect(firstCall.tokenId).toBe(tokenId); + + mockBroadcastChannel.postMessage.mockClear(); + + const tokenResolver2 = Promise.resolve( + new Token({ + id: tokenId, + jwt: mockJwt, + object: 'token', + }) as TokenResource, + ); + + SessionTokenCache.set({ tokenId, tokenResolver: tokenResolver2 }); + await tokenResolver2; + + expect(mockBroadcastChannel.postMessage).toHaveBeenCalledTimes(1); + }); }); describe('token expiration with absolute time', () => { diff --git a/packages/clerk-js/src/core/resources/Session.ts b/packages/clerk-js/src/core/resources/Session.ts index c2b5760fd58..b774b12730f 100644 --- a/packages/clerk-js/src/core/resources/Session.ts +++ b/packages/clerk-js/src/core/resources/Session.ts @@ -131,9 +131,15 @@ export class Session extends BaseResource implements SessionResource { }; #hydrateCache = (token: TokenResource | null) => { - if (token) { + if (!token) { + return; + } + + const tokenId = this.#getCacheId(); + const existing = SessionTokenCache.get({ tokenId }); + if (!existing) { SessionTokenCache.set({ - tokenId: this.#getCacheId(), + tokenId, tokenResolver: Promise.resolve(token), }); } diff --git a/packages/clerk-js/src/core/resources/__tests__/Session.test.ts b/packages/clerk-js/src/core/resources/__tests__/Session.test.ts index 59569d0b207..4de20208046 100644 --- a/packages/clerk-js/src/core/resources/__tests__/Session.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/Session.test.ts @@ -35,7 +35,7 @@ describe('Session', () => { beforeEach(() => { dispatchSpy = vi.spyOn(eventBus, 'emit'); - BaseResource.clerk = clerkMock() as any; + BaseResource.clerk = clerkMock(); }); afterEach(() => { @@ -76,7 +76,7 @@ describe('Session', () => { it('hydrates token cache from lastActiveToken', async () => { BaseResource.clerk = clerkMock({ organization: new Organization({ id: 'activeOrganization' } as OrganizationJSON), - }) as any; + }); const session = new Session({ status: 'active', @@ -100,10 +100,81 @@ describe('Session', () => { expect(dispatchSpy).toHaveBeenCalledTimes(2); }); + it('does not re-cache token when Session is reconstructed with same token', async () => { + BaseResource.clerk = clerkMock({ + organization: new Organization({ id: 'activeOrganization' } as OrganizationJSON), + }); + + SessionTokenCache.clear(); + + const session1 = new Session({ + status: 'active', + id: 'session_1', + object: 'session', + user: createUser({}), + last_active_organization_id: 'activeOrganization', + last_active_token: { object: 'token', jwt: mockJwt }, + actor: null, + created_at: new Date().getTime(), + updated_at: new Date().getTime(), + } as SessionJSON); + + expect(SessionTokenCache.size()).toBe(1); + const cachedEntry1 = SessionTokenCache.get({ tokenId: 'session_1-activeOrganization' }); + expect(cachedEntry1).toBeDefined(); + + const session2 = new Session({ + status: 'active', + id: 'session_1', + object: 'session', + user: createUser({}), + last_active_organization_id: 'activeOrganization', + last_active_token: { object: 'token', jwt: mockJwt }, + actor: null, + created_at: new Date().getTime(), + updated_at: new Date().getTime(), + } as SessionJSON); + + expect(SessionTokenCache.size()).toBe(1); + + const token1 = await session1.getToken(); + const token2 = await session2.getToken(); + + expect(token1).toBe(token2); + expect(token1).toEqual(mockJwt); + expect(BaseResource.clerk.getFapiClient().request).not.toHaveBeenCalled(); + }); + + it('caches token from cookie during degraded mode recovery', async () => { + BaseResource.clerk = clerkMock(); + + SessionTokenCache.clear(); + + const sessionFromCookie = new Session({ + status: 'active', + id: 'session_1', + object: 'session', + user: createUser({}), + last_active_organization_id: null, + last_active_token: { object: 'token', jwt: mockJwt }, + actor: null, + created_at: new Date().getTime(), + updated_at: new Date().getTime(), + } as SessionJSON); + + expect(SessionTokenCache.size()).toBe(1); + const cachedEntry = SessionTokenCache.get({ tokenId: 'session_1' }); + expect(cachedEntry).toBeDefined(); + + const token = await sessionFromCookie.getToken(); + expect(token).toEqual(mockJwt); + expect(BaseResource.clerk.getFapiClient().request).not.toHaveBeenCalled(); + }); + it('dispatches token:update event on getToken with active organization', async () => { BaseResource.clerk = clerkMock({ organization: new Organization({ id: 'activeOrganization' } as OrganizationJSON), - }) as any; + }); const session = new Session({ status: 'active', @@ -138,7 +209,7 @@ describe('Session', () => { it('does not dispatch token:update if template is provided', async () => { BaseResource.clerk = clerkMock({ organization: new Organization({ id: 'activeOrganization' } as OrganizationJSON), - }) as any; + }); const session = new Session({ status: 'active', @@ -159,7 +230,7 @@ describe('Session', () => { it('dispatches token:update when provided organization ID matches current active organization', async () => { BaseResource.clerk = clerkMock({ organization: new Organization({ id: 'activeOrganization' } as OrganizationJSON), - }) as any; + }); const session = new Session({ status: 'active', @@ -178,7 +249,7 @@ describe('Session', () => { }); it('does not dispatch token:update when provided organization ID does not match current active organization', async () => { - BaseResource.clerk = clerkMock() as any; + BaseResource.clerk = clerkMock(); const session = new Session({ status: 'active', @@ -240,7 +311,7 @@ describe('Session', () => { it(`uses the current session's lastActiveOrganizationId by default, not clerk.organization.id`, async () => { BaseResource.clerk = clerkMock({ organization: new Organization({ id: 'oldActiveOrganization' } as OrganizationJSON), - }) as any; + }); const session = new Session({ status: 'active', @@ -261,7 +332,7 @@ describe('Session', () => { }); it('deduplicates concurrent getToken calls to prevent multiple API requests', async () => { - BaseResource.clerk = clerkMock() as any; + BaseResource.clerk = clerkMock(); const session = new Session({ status: 'active', @@ -286,7 +357,7 @@ describe('Session', () => { }); it('deduplicates concurrent getToken calls with same template', async () => { - BaseResource.clerk = clerkMock() as any; + BaseResource.clerk = clerkMock(); const session = new Session({ status: 'active', @@ -313,7 +384,7 @@ describe('Session', () => { }); it('does not deduplicate getToken calls with different templates', async () => { - BaseResource.clerk = clerkMock() as any; + BaseResource.clerk = clerkMock(); const session = new Session({ status: 'active', @@ -335,7 +406,7 @@ describe('Session', () => { }); it('does not deduplicate getToken calls with different organization IDs', async () => { - BaseResource.clerk = clerkMock() as any; + BaseResource.clerk = clerkMock(); const session = new Session({ status: 'active', @@ -362,7 +433,7 @@ describe('Session', () => { beforeEach(() => { dispatchSpy = vi.spyOn(eventBus, 'emit'); - BaseResource.clerk = clerkMock() as any; + BaseResource.clerk = clerkMock(); }); afterEach(() => {