Skip to content

Commit 82da272

Browse files
nicknisiCopilot
andauthored
Fix: Show loading state during initial token fetch to prevent flash (#297)
* fix: show loading state during initial token fetch to prevent flash Previously, useAccessToken would report loading:false during the initial token fetch, creating a race condition where useAuth had finished loading but the token hadn't arrived yet. This caused downstream consumers to see a brief "unauthenticated" state. This led to workarounds like adding !accessToken to loading calculations (see workos/template-convex-nextjs-authkit#5), which then broke for logged-out users. This fix ensures useAccessToken correctly reports loading:true during initial fetch, eliminating the race condition at its source. Downstream consumers can now safely check loading states without needing the !accessToken workaround. Changes: - Track initial token fetch with isInitialTokenLoading state - Report loading:true until initial fetch completes - Update tests to expect loading state during mount * Update src/components/useAccessToken.ts Co-authored-by: Copilot <[email protected]> * fix: prevent unnecessary loading states for cached tokens --------- Co-authored-by: Copilot <[email protected]>
1 parent 4954a3e commit 82da272

File tree

2 files changed

+83
-16
lines changed

2 files changed

+83
-16
lines changed

__tests__/useAccessToken.spec.tsx

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,15 @@ describe('useAccessToken', () => {
5757
);
5858
};
5959

60-
it('should fetch an access token on mount without showing loading state', async () => {
60+
it('should fetch an access token on mount and show loading state initially', async () => {
6161
const mockToken =
6262
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwic2lkIjoic2Vzc2lvbl8xMjMiLCJleHAiOjk5OTk5OTk5OTl9.mock-signature';
6363
(getAccessTokenAction as jest.Mock).mockResolvedValueOnce(mockToken);
6464

6565
const { getByTestId } = render(<TestComponent />);
6666

67-
// Loading should remain false for background fetches
68-
expect(getByTestId('loading')).toHaveTextContent('false');
67+
// Loading should be true during initial fetch
68+
expect(getByTestId('loading')).toHaveTextContent('true');
6969

7070
await waitFor(() => {
7171
expect(getAccessTokenAction).toHaveBeenCalledTimes(1);
@@ -77,7 +77,7 @@ describe('useAccessToken', () => {
7777
});
7878
});
7979

80-
it('should handle token refresh when an expiring token is received without showing loading', async () => {
80+
it('should handle token refresh when an expiring token is received', async () => {
8181
// Create a token that's about to expire (exp is very close to current time)
8282
const currentTimeInSeconds = Math.floor(Date.now() / 1000);
8383
// Use 25 seconds to ensure it's within the 30-second buffer for short-lived tokens
@@ -97,8 +97,8 @@ describe('useAccessToken', () => {
9797

9898
const { getByTestId } = render(<TestComponent />);
9999

100-
// Loading should remain false throughout
101-
expect(getByTestId('loading')).toHaveTextContent('false');
100+
// Loading should be true initially during token fetch
101+
expect(getByTestId('loading')).toHaveTextContent('true');
102102

103103
await waitFor(() => {
104104
expect(getByTestId('loading')).toHaveTextContent('false');
@@ -155,14 +155,14 @@ describe('useAccessToken', () => {
155155
});
156156
});
157157

158-
it('should handle errors during token fetch without showing loading', async () => {
158+
it('should handle errors during token fetch', async () => {
159159
const error = new Error('Failed to fetch token');
160160
(getAccessTokenAction as jest.Mock).mockRejectedValueOnce(error);
161161

162162
const { getByTestId } = render(<TestComponent />);
163163

164-
// Loading should remain false even when there's an error
165-
expect(getByTestId('loading')).toHaveTextContent('false');
164+
// Loading should be true initially
165+
expect(getByTestId('loading')).toHaveTextContent('true');
166166

167167
await waitFor(() => {
168168
expect(getByTestId('loading')).toHaveTextContent('false');
@@ -381,8 +381,8 @@ describe('useAccessToken', () => {
381381

382382
const { getByTestId } = render(<TestComponent />);
383383

384-
// Loading should remain false for background operations
385-
expect(getByTestId('loading')).toHaveTextContent('false');
384+
// Loading should be true initially during fetch
385+
expect(getByTestId('loading')).toHaveTextContent('true');
386386

387387
await waitFor(() => {
388388
expect(fetchCalls).toBe(1);
@@ -459,6 +459,44 @@ describe('useAccessToken', () => {
459459
});
460460
});
461461

462+
it('should show loading state immediately on first render when user exists but no token', () => {
463+
// Mock user with no token initially
464+
(useAuth as jest.Mock).mockImplementation(() => ({
465+
user: { id: 'user_123' },
466+
sessionId: 'session_123',
467+
refreshAuth: jest.fn().mockResolvedValue({}),
468+
}));
469+
470+
(getAccessTokenAction as jest.Mock).mockImplementation(
471+
() => new Promise((resolve) => setTimeout(() => resolve('token'), 100)),
472+
);
473+
474+
const { getByTestId } = render(<TestComponent />);
475+
476+
expect(getByTestId('loading')).toHaveTextContent('true');
477+
expect(getByTestId('token')).toHaveTextContent('no-token');
478+
});
479+
480+
it('should not show loading when a valid token already exists', async () => {
481+
const existingToken =
482+
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJleGlzdGluZyIsInNpZCI6InNlc3Npb24xMjMiLCJleHAiOjk5OTk5OTk5OTl9.existing';
483+
484+
await act(async () => {
485+
(getAccessTokenAction as jest.Mock).mockResolvedValueOnce(existingToken);
486+
await tokenStore.getAccessTokenSilently();
487+
});
488+
489+
// Reset the mock to track new calls
490+
(getAccessTokenAction as jest.Mock).mockClear();
491+
492+
const { getByTestId } = render(<TestComponent />);
493+
494+
expect(getByTestId('loading')).toHaveTextContent('false');
495+
expect(getByTestId('token')).toHaveTextContent(existingToken);
496+
497+
expect(getAccessTokenAction).not.toHaveBeenCalled();
498+
});
499+
462500
// Additional test cases to increase coverage
463501
it('should handle concurrent manual refresh attempts', async () => {
464502
const initialToken =

src/components/useAccessToken.ts

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useEffect, useRef, useSyncExternalStore } from 'react';
1+
import { useCallback, useEffect, useRef, useState, useSyncExternalStore } from 'react';
22
import { useAuth } from './authkit-provider.js';
33
import { tokenStore } from './tokenStore.js';
44

@@ -42,9 +42,17 @@ export function useAccessToken(): UseAccessTokenReturn {
4242

4343
const tokenState = useSyncExternalStore(tokenStore.subscribe, tokenStore.getSnapshot, tokenStore.getServerSnapshot);
4444

45+
// Track if we're waiting for the initial token fetch for the current user
46+
// Initialize synchronously to prevent first-paint flash
47+
const [isInitialTokenLoading, setIsInitialTokenLoading] = useState(() => {
48+
// Only show loading if we have a user but no token yet
49+
return Boolean(user && !tokenState.token && !tokenState.error);
50+
});
51+
4552
useEffect(() => {
4653
if (!user) {
4754
tokenStore.clearToken();
55+
setIsInitialTokenLoading(false);
4856
return;
4957
}
5058

@@ -59,10 +67,28 @@ export function useAccessToken(): UseAccessTokenReturn {
5967
prevSessionRef.current = sessionId;
6068
prevUserIdRef.current = userId;
6169

70+
// Check if getAccessTokenSilently will actually fetch (not just return cached)
71+
const currentToken = tokenStore.getSnapshot().token;
72+
const tokenData = currentToken ? tokenStore.parseToken(currentToken) : null;
73+
const willActuallyFetch = !currentToken || (tokenData && tokenData.isExpiring);
74+
75+
// Only show loading if we're actually going to fetch
76+
if (willActuallyFetch) {
77+
setIsInitialTokenLoading(true);
78+
}
79+
6280
/* istanbul ignore next */
63-
tokenStore.getAccessTokenSilently().catch(() => {
64-
// Error is handled in the store
65-
});
81+
tokenStore
82+
.getAccessTokenSilently()
83+
.catch(() => {
84+
// Error is handled in the store
85+
})
86+
.finally(() => {
87+
// Only clear loading if we were actually loading
88+
if (willActuallyFetch) {
89+
setIsInitialTokenLoading(false);
90+
}
91+
});
6692
}, [userId, sessionId]);
6793

6894
useEffect(() => {
@@ -112,9 +138,12 @@ export function useAccessToken(): UseAccessTokenReturn {
112138
return tokenStore.refreshToken();
113139
}, []);
114140

141+
// Combine loading states: initial token fetch OR token store is loading
142+
const isLoading = isInitialTokenLoading || tokenState.loading;
143+
115144
return {
116145
accessToken: tokenState.token,
117-
loading: tokenState.loading,
146+
loading: isLoading,
118147
error: tokenState.error,
119148
refresh,
120149
getAccessToken,

0 commit comments

Comments
 (0)