Skip to content

Commit cb580f1

Browse files
committed
refactor(clerk-js): Consolidate resource events into single resource:state-change event
1 parent 254faac commit cb580f1

13 files changed

Lines changed: 276 additions & 129 deletions

File tree

.changeset/fiery-games-pick.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
---
4+
5+
Fix inconsistent `fetchStatus` during sign-in and sign-up flows where the resource would briefly show as `complete` while `fetchStatus` was still `fetching`

packages/clerk-js/src/core/__tests__/state.test.ts

Lines changed: 114 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,74 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
33
import { eventBus } from '../events';
44
import { SignIn } from '../resources/SignIn';
55
import { SignUp } from '../resources/SignUp';
6-
import { signInResourceSignal, signUpResourceSignal } from '../signals';
6+
import { signInFetchSignal, signInResourceSignal, signUpResourceSignal } from '../signals';
77
import { State } from '../state';
88

9+
describe('Signal batching', () => {
10+
let state: State;
11+
12+
beforeEach(() => {
13+
signInResourceSignal({ resource: null });
14+
signInFetchSignal({ status: 'idle' });
15+
state = new State();
16+
});
17+
18+
it('should produce at most 3 renders with clean fetchStatus transitions during an API call', async () => {
19+
const signIn = new SignIn(null);
20+
const snapshots: Array<{ fetchStatus: string; hasSignIn: boolean }> = [];
21+
22+
state.__internal_effect(() => {
23+
const s = state.signInSignal();
24+
snapshots.push({ fetchStatus: s.fetchStatus, hasSignIn: s.signIn !== null });
25+
});
26+
27+
await signIn.__internal_future.password({ password: 'test123', identifier: 'test@example.com' }).catch(() => {
28+
// Expected to fail since there's no real API
29+
});
30+
31+
expect(snapshots.length).toBeLessThanOrEqual(3);
32+
33+
// fetchStatus follows a clean idle → fetching → idle progression
34+
const transitions = snapshots.map(s => s.fetchStatus).filter((s, i, arr) => i === 0 || s !== arr[i - 1]);
35+
expect(transitions).toEqual(['idle', 'fetching', 'idle']);
36+
});
37+
38+
it('should skip resource-only updates while fetching and apply them on idle', () => {
39+
const signIn = new SignIn({ id: 'signin_123', status: 'needs_identifier' } as any);
40+
41+
// Simulate fetchStatus: fetching (as runAsyncResourceTask would)
42+
eventBus.emit('resource:state-change', { resource: signIn, error: null, fetchStatus: 'fetching' });
43+
expect(signInFetchSignal().status).toBe('fetching');
44+
45+
// Simulate fromJSON updating the resource mid-flight (resource-only event)
46+
eventBus.emit('resource:state-change', { resource: signIn });
47+
48+
// Resource signal should NOT have been updated — skipped while fetching
49+
expect(signInResourceSignal().resource?.id).toBe('signin_123');
50+
51+
// Simulate task completion — resource is carried again with fetchStatus: idle
52+
eventBus.emit('resource:state-change', { resource: signIn, error: null, fetchStatus: 'idle' });
53+
54+
// Now both resource and fetchStatus are consistent
55+
expect(signInFetchSignal().status).toBe('idle');
56+
expect(signInResourceSignal().resource).toBe(signIn);
57+
});
58+
59+
it('should reflect new resource data immediately when no operation is in flight', () => {
60+
let latestSignInId: string | undefined;
61+
62+
state.__internal_effect(() => {
63+
latestSignInId = state.signInSignal().signIn?.id;
64+
});
65+
66+
expect(latestSignInId).toBeUndefined();
67+
68+
new SignIn({ id: 'signin_123', status: 'needs_identifier' } as any);
69+
70+
expect(latestSignInId).toBe('signin_123');
71+
});
72+
});
73+
974
describe('State', () => {
1075
let _state: State;
1176

@@ -52,7 +117,7 @@ describe('State', () => {
52117
expect(existingSignUp.__internal_future.canBeDiscarded).toBe(false);
53118

54119
// Act: Emit a resource update with a null SignUp (simulating client refresh with null sign_up)
55-
const _nullSignUp = new SignUp(null);
120+
new SignUp(null);
56121

57122
// Assert: Signal should NOT be updated - should still have the existing SignUp
58123
expect(signUpResourceSignal().resource).toBe(existingSignUp);
@@ -96,11 +161,6 @@ describe('State', () => {
96161
resetSignUp: vi.fn().mockImplementation(function (this: typeof mockClient) {
97162
newSignUpFromReset = new SignUp(null);
98163
this.signUp = newSignUpFromReset;
99-
// reset() emits resource:error to clear errors, but the signal update
100-
// happens via resource:update when the new SignUp is created
101-
eventBus.emit('resource:error', { resource: newSignUpFromReset, error: null });
102-
// Emit resource:update to update the signal (simulating what happens in real flow)
103-
eventBus.emit('resource:update', { resource: newSignUpFromReset });
104164
}),
105165
};
106166
SignUp.clerk = { client: mockClient } as any;
@@ -127,10 +187,10 @@ describe('State', () => {
127187

128188
it('should allow resource update when new resource has an id (not a null update)', () => {
129189
// Arrange: Set up a SignUp with id
130-
const _existingSignUp = new SignUp({ id: 'signup_123', status: 'missing_requirements' } as any);
190+
new SignUp({ id: 'signup_123', status: 'missing_requirements' } as any);
131191
expect(signUpResourceSignal().resource?.id).toBe('signup_123');
132192

133-
// Act: Emit a resource update with a different SignUp that also has an id
193+
// Act: Create a different SignUp that also has an id
134194
const newSignUp = new SignUp({ id: 'signup_456', status: 'complete' } as any);
135195

136196
// Assert: Signal should be updated with the new SignUp
@@ -159,7 +219,7 @@ describe('State', () => {
159219
expect(existingSignIn.__internal_future.canBeDiscarded).toBe(false);
160220

161221
// Act: Emit a resource update with a null SignIn (simulating client refresh with null sign_in)
162-
const _nullSignIn = new SignIn(null);
222+
new SignIn(null);
163223

164224
// Assert: Signal should NOT be updated - should still have the existing SignIn
165225
expect(signInResourceSignal().resource).toBe(existingSignIn);
@@ -201,16 +261,13 @@ describe('State', () => {
201261
resetSignIn: vi.fn().mockImplementation(function (this: typeof mockClient) {
202262
newSignInFromReset = new SignIn(null);
203263
this.signIn = newSignInFromReset;
204-
eventBus.emit('resource:error', { resource: newSignInFromReset, error: null });
205-
eventBus.emit('resource:update', { resource: newSignInFromReset });
206264
}),
207265
};
208266
SignIn.clerk = { client: mockClient } as any;
209267

210268
// Create a SignIn with id
211269
const existingSignIn = new SignIn({ id: 'signin_123', status: 'needs_identifier' } as any);
212270
expect(signInResourceSignal().resource?.id).toBe('signin_123');
213-
expect(existingSignIn.__internal_future.canBeDiscarded).toBe(false);
214271

215272
// Act: Call reset()
216273
await existingSignIn.__internal_future.reset();
@@ -224,10 +281,10 @@ describe('State', () => {
224281

225282
it('should allow resource update when new resource has an id (not a null update)', () => {
226283
// Arrange: Set up a SignIn with id
227-
const _existingSignIn = new SignIn({ id: 'signin_123', status: 'needs_identifier' } as any);
284+
new SignIn({ id: 'signin_123', status: 'needs_identifier' } as any);
228285
expect(signInResourceSignal().resource?.id).toBe('signin_123');
229286

230-
// Act: Emit a resource update with a different SignIn that also has an id
287+
// Act: Create a different SignIn that also has an id
231288
const newSignIn = new SignIn({ id: 'signin_456', status: 'complete' } as any);
232289

233290
// Assert: Signal should be updated with the new SignIn
@@ -240,19 +297,19 @@ describe('State', () => {
240297
describe('Edge cases', () => {
241298
it('should handle rapid successive updates correctly', () => {
242299
// First update with valid SignUp
243-
const _signUp1 = new SignUp({ id: 'signup_1', status: 'missing_requirements' } as any);
300+
new SignUp({ id: 'signup_1', status: 'missing_requirements' } as any);
244301
expect(signUpResourceSignal().resource?.id).toBe('signup_1');
245302

246303
// Second update with another valid SignUp
247-
const _signUp2 = new SignUp({ id: 'signup_2', status: 'missing_requirements' } as any);
304+
new SignUp({ id: 'signup_2', status: 'missing_requirements' } as any);
248305
expect(signUpResourceSignal().resource?.id).toBe('signup_2');
249306

250307
// Null update should be ignored
251-
const _nullSignUp = new SignUp(null);
308+
new SignUp(null);
252309
expect(signUpResourceSignal().resource?.id).toBe('signup_2');
253310

254311
// Another valid update should work
255-
const _signUp3 = new SignUp({ id: 'signup_3', status: 'complete' } as any);
312+
new SignUp({ id: 'signup_3', status: 'complete' } as any);
256313
expect(signUpResourceSignal().resource?.id).toBe('signup_3');
257314
});
258315

@@ -262,10 +319,47 @@ describe('State', () => {
262319
expect(signUpResourceSignal().resource?.id).toBe('signup_123');
263320

264321
// Manually emit update with the same instance (simulating fromJSON on same instance)
265-
eventBus.emit('resource:update', { resource: signUp });
322+
eventBus.emit('resource:state-change', { resource: signUp });
266323

267324
// Signal should still have the same instance
268325
expect(signUpResourceSignal().resource).toBe(signUp);
269326
});
270327
});
328+
329+
describe('Client.destroy()', () => {
330+
it('should update signals when resources are replaced with null instances', async () => {
331+
const mockSetActive = vi.fn().mockResolvedValue({});
332+
SignIn.clerk = { setActive: mockSetActive, client: { sessions: [{ id: 'session_123' }] } } as any;
333+
334+
const existingSignIn = new SignIn({
335+
id: 'signin_123',
336+
status: 'complete',
337+
created_session_id: 'session_123',
338+
} as any);
339+
expect(signInResourceSignal().resource).toBe(existingSignIn);
340+
341+
await existingSignIn.__internal_future.finalize();
342+
expect(existingSignIn.__internal_future.canBeDiscarded).toBe(true);
343+
344+
// Simulates what Client.destroy() does — creating a null resource replaces the existing one
345+
const nullSignIn = new SignIn(null);
346+
347+
expect(signInResourceSignal().resource).toBe(nullSignIn);
348+
expect(signInResourceSignal().resource?.id).toBeUndefined();
349+
});
350+
});
351+
352+
describe('fetchStatus clearing on reset', () => {
353+
it('should clear fetchStatus to idle when resource is reset during an in-flight fetch', () => {
354+
const signIn = new SignIn({ id: 'signin_123', status: 'needs_identifier' } as any);
355+
eventBus.emit('resource:state-change', { resource: signIn, error: null, fetchStatus: 'fetching' });
356+
expect(signInFetchSignal().status).toBe('fetching');
357+
358+
// Reset replaces the resource and clears fetchStatus in one event
359+
const nullSignIn = new SignIn(null);
360+
eventBus.emit('resource:state-change', { resource: nullSignIn, error: null, fetchStatus: 'idle' });
361+
362+
expect(signInFetchSignal().status).toBe('idle');
363+
});
364+
});
271365
});

packages/clerk-js/src/core/events.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,22 @@ export const events = {
99
UserSignOut: 'user:signOut',
1010
EnvironmentUpdate: 'environment:update',
1111
SessionTokenResolved: 'session:tokenResolved',
12-
ResourceUpdate: 'resource:update',
13-
ResourceError: 'resource:error',
14-
ResourceFetch: 'resource:fetch',
12+
ResourceStateChange: 'resource:state-change',
1513
} as const;
1614

1715
type TokenUpdatePayload = { token: TokenResource | null };
18-
export type ResourceUpdatePayload = { resource: BaseResource };
19-
export type ResourceErrorPayload = { resource: BaseResource; error: ClerkError | null };
20-
export type ResourceFetchPayload = { resource: BaseResource; status: 'idle' | 'fetching' };
16+
export type ResourceStateChangePayload = {
17+
resource: BaseResource;
18+
error?: ClerkError | null;
19+
fetchStatus?: 'idle' | 'fetching';
20+
};
2121

2222
type InternalEvents = {
2323
[events.TokenUpdate]: TokenUpdatePayload;
2424
[events.UserSignOut]: null;
2525
[events.EnvironmentUpdate]: null;
2626
[events.SessionTokenResolved]: null;
27-
[events.ResourceUpdate]: ResourceUpdatePayload;
28-
[events.ResourceError]: ResourceErrorPayload;
29-
[events.ResourceFetch]: ResourceFetchPayload;
27+
[events.ResourceStateChange]: ResourceStateChangePayload;
3028
};
3129

3230
export const eventBus = createEventBus<InternalEvents>();

packages/clerk-js/src/core/resources/Client.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,13 @@ export class Client extends BaseResource implements ClientResource {
106106
resetSignIn(): void {
107107
this.signIn = new SignIn(null);
108108
// Cast needed because this.signIn is typed as SignInResource (interface), not SignIn (class extending BaseResource)
109-
eventBus.emit('resource:error', { resource: this.signIn as SignIn, error: null });
109+
eventBus.emit('resource:state-change', { resource: this.signIn as SignIn, error: null, fetchStatus: 'idle' });
110110
}
111111

112112
resetSignUp(): void {
113113
this.signUp = new SignUp(null);
114114
// Cast needed because this.signUp is typed as SignUpResource (interface), not SignUp (class extending BaseResource)
115-
eventBus.emit('resource:error', { resource: this.signUp as SignUp, error: null });
115+
eventBus.emit('resource:state-change', { resource: this.signUp as SignUp, error: null, fetchStatus: 'idle' });
116116
}
117117

118118
clearCache(): void {

packages/clerk-js/src/core/resources/SignIn.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ export class SignIn extends BaseResource implements SignInResource {
596596
this.clientTrustState = data.client_trust_state ?? undefined;
597597
}
598598

599-
eventBus.emit('resource:update', { resource: this });
599+
eventBus.emit('resource:state-change', { resource: this });
600600
return this;
601601
}
602602

@@ -1415,14 +1415,19 @@ class SignInFuture implements SignInFutureResource {
14151415
return this.create({ ticket: ticket ?? undefined });
14161416
}
14171417

1418+
/**
1419+
* Finalizes a completed sign-in by setting the active session.
1420+
* Like reset(), this does NOT set fetchStatus to 'fetching' — the sign-in
1421+
* is already complete, and setActive is a session-level operation.
1422+
*/
14181423
async finalize(params?: SignInFutureFinalizeParams): Promise<{ error: ClerkError | null }> {
14191424
const { navigate } = params || {};
14201425

14211426
if (!this.#resource.createdSessionId) {
14221427
throw new Error('Cannot finalize sign-in without a created session.');
14231428
}
14241429

1425-
return runAsyncResourceTask(this.#resource, async () => {
1430+
try {
14261431
// Reload the client if the created session is not in the client's sessions. This can happen during modal SSO
14271432
// flows where the in-memory client does not have the created session.
14281433
if (SignIn.clerk.client && !SignIn.clerk.client.sessions.some(s => s.id === this.#resource.createdSessionId)) {
@@ -1431,12 +1436,15 @@ class SignInFuture implements SignInFutureResource {
14311436

14321437
this.#canBeDiscarded = true;
14331438
await SignIn.clerk.setActive({ session: this.#resource.createdSessionId, navigate });
1434-
});
1439+
return { error: null };
1440+
} catch (err) {
1441+
return { error: err as ClerkError };
1442+
}
14351443
}
14361444

14371445
/**
14381446
* Resets the current sign-in attempt by clearing all local state back to null.
1439-
* Unlike other methods, this does NOT emit resource:fetch with 'fetching' status,
1447+
* Unlike other methods, this does NOT set fetchStatus to 'fetching',
14401448
* allowing for smooth UI transitions without loading states.
14411449
*/
14421450
reset(): Promise<{ error: ClerkError | null }> {

packages/clerk-js/src/core/resources/SignUp.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,7 @@ export class SignUp extends BaseResource implements SignUpResource {
510510
this.locale = data.locale;
511511
}
512512

513-
eventBus.emit('resource:update', { resource: this });
513+
eventBus.emit('resource:state-change', { resource: this });
514514
return this;
515515
}
516516

@@ -1137,21 +1137,29 @@ class SignUpFuture implements SignUpFutureResource {
11371137
return this.create({ ...params, ticket: ticket ?? undefined });
11381138
}
11391139

1140+
/**
1141+
* Finalizes a completed sign-up by setting the active session.
1142+
* Like reset(), this does NOT set fetchStatus to 'fetching' — the sign-up
1143+
* is already complete, and setActive is a session-level operation.
1144+
*/
11401145
async finalize(params?: SignUpFutureFinalizeParams): Promise<{ error: ClerkError | null }> {
11411146
const { navigate } = params || {};
1142-
return runAsyncResourceTask(this.#resource, async () => {
1147+
1148+
try {
11431149
if (!this.#resource.createdSessionId) {
11441150
throw new Error('Cannot finalize sign-up without a created session.');
11451151
}
1146-
11471152
this.#canBeDiscarded = true;
11481153
await SignUp.clerk.setActive({ session: this.#resource.createdSessionId, navigate });
1149-
});
1154+
return { error: null };
1155+
} catch (err) {
1156+
return { error: err as ClerkError };
1157+
}
11501158
}
11511159

11521160
/**
11531161
* Resets the current sign-up attempt by clearing all local state back to null.
1154-
* Unlike other methods, this does NOT emit resource:fetch with 'fetching' status,
1162+
* Unlike other methods, this does NOT set fetchStatus to 'fetching',
11551163
* allowing for smooth UI transitions without loading states.
11561164
*/
11571165
reset(): Promise<{ error: ClerkError | null }> {

packages/clerk-js/src/core/resources/Waitlist.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ export class Waitlist extends BaseResource implements WaitlistResource {
2727
this.updatedAt = unixEpochToDate(data.updated_at);
2828
this.createdAt = unixEpochToDate(data.created_at);
2929

30-
eventBus.emit('resource:update', { resource: this });
30+
eventBus.emit('resource:state-change', { resource: this });
3131
return this;
3232
}
3333

3434
async join(params: JoinWaitlistParams): Promise<{ error: ClerkError | null }> {
3535
return runAsyncResourceTask(this, async () => {
36-
await Waitlist.join(params);
36+
await this._basePost({ path: this.pathRoot, body: params });
3737
});
3838
}
3939

0 commit comments

Comments
 (0)