Skip to content

Commit baab30c

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

12 files changed

Lines changed: 215 additions & 124 deletions

File tree

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

Lines changed: 91 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,51 @@ 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+
state = new State();
14+
});
15+
16+
it('should produce at most 3 renders with clean fetchStatus transitions during an API call', async () => {
17+
const signIn = new SignIn(null);
18+
const snapshots: Array<{ fetchStatus: string; hasSignIn: boolean }> = [];
19+
20+
state.__internal_effect(() => {
21+
const s = state.signInSignal();
22+
snapshots.push({ fetchStatus: s.fetchStatus, hasSignIn: s.signIn !== null });
23+
});
24+
25+
await signIn.__internal_future.password({ password: 'test123', identifier: 'test@example.com' }).catch(() => {
26+
// Expected to fail since there's no real API
27+
});
28+
29+
expect(snapshots.length).toBeLessThanOrEqual(3);
30+
31+
// fetchStatus follows a clean idle → fetching → idle progression
32+
const transitions = snapshots.map(s => s.fetchStatus).filter((s, i, arr) => i === 0 || s !== arr[i - 1]);
33+
expect(transitions).toEqual(['idle', 'fetching', 'idle']);
34+
});
35+
36+
it('should reflect new resource data immediately when no operation is in flight', () => {
37+
let latestSignInId: string | undefined;
38+
39+
state.__internal_effect(() => {
40+
latestSignInId = state.signInSignal().signIn?.id;
41+
});
42+
43+
expect(latestSignInId).toBeUndefined();
44+
45+
new SignIn({ id: 'signin_123', status: 'needs_identifier' } as any);
46+
47+
expect(latestSignInId).toBe('signin_123');
48+
});
49+
});
50+
951
describe('State', () => {
1052
let _state: State;
1153

@@ -52,7 +94,7 @@ describe('State', () => {
5294
expect(existingSignUp.__internal_future.canBeDiscarded).toBe(false);
5395

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

5799
// Assert: Signal should NOT be updated - should still have the existing SignUp
58100
expect(signUpResourceSignal().resource).toBe(existingSignUp);
@@ -96,11 +138,6 @@ describe('State', () => {
96138
resetSignUp: vi.fn().mockImplementation(function (this: typeof mockClient) {
97139
newSignUpFromReset = new SignUp(null);
98140
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 });
104141
}),
105142
};
106143
SignUp.clerk = { client: mockClient } as any;
@@ -127,10 +164,10 @@ describe('State', () => {
127164

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

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

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

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

164201
// Assert: Signal should NOT be updated - should still have the existing SignIn
165202
expect(signInResourceSignal().resource).toBe(existingSignIn);
@@ -201,16 +238,13 @@ describe('State', () => {
201238
resetSignIn: vi.fn().mockImplementation(function (this: typeof mockClient) {
202239
newSignInFromReset = new SignIn(null);
203240
this.signIn = newSignInFromReset;
204-
eventBus.emit('resource:error', { resource: newSignInFromReset, error: null });
205-
eventBus.emit('resource:update', { resource: newSignInFromReset });
206241
}),
207242
};
208243
SignIn.clerk = { client: mockClient } as any;
209244

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

215249
// Act: Call reset()
216250
await existingSignIn.__internal_future.reset();
@@ -224,10 +258,10 @@ describe('State', () => {
224258

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

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

233267
// Assert: Signal should be updated with the new SignIn
@@ -240,19 +274,19 @@ describe('State', () => {
240274
describe('Edge cases', () => {
241275
it('should handle rapid successive updates correctly', () => {
242276
// First update with valid SignUp
243-
const _signUp1 = new SignUp({ id: 'signup_1', status: 'missing_requirements' } as any);
277+
new SignUp({ id: 'signup_1', status: 'missing_requirements' } as any);
244278
expect(signUpResourceSignal().resource?.id).toBe('signup_1');
245279

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

250284
// Null update should be ignored
251-
const _nullSignUp = new SignUp(null);
285+
new SignUp(null);
252286
expect(signUpResourceSignal().resource?.id).toBe('signup_2');
253287

254288
// Another valid update should work
255-
const _signUp3 = new SignUp({ id: 'signup_3', status: 'complete' } as any);
289+
new SignUp({ id: 'signup_3', status: 'complete' } as any);
256290
expect(signUpResourceSignal().resource?.id).toBe('signup_3');
257291
});
258292

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

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

267301
// Signal should still have the same instance
268302
expect(signUpResourceSignal().resource).toBe(signUp);
269303
});
270304
});
305+
306+
describe('Client.destroy()', () => {
307+
it('should update signals when resources are replaced with null instances', async () => {
308+
const mockSetActive = vi.fn().mockResolvedValue({});
309+
SignIn.clerk = { setActive: mockSetActive, client: { sessions: [{ id: 'session_123' }] } } as any;
310+
311+
const existingSignIn = new SignIn({
312+
id: 'signin_123',
313+
status: 'complete',
314+
created_session_id: 'session_123',
315+
} as any);
316+
expect(signInResourceSignal().resource).toBe(existingSignIn);
317+
318+
await existingSignIn.__internal_future.finalize();
319+
expect(existingSignIn.__internal_future.canBeDiscarded).toBe(true);
320+
321+
// Simulates what Client.destroy() does — creating a null resource replaces the existing one
322+
const nullSignIn = new SignIn(null);
323+
324+
expect(signInResourceSignal().resource).toBe(nullSignIn);
325+
expect(signInResourceSignal().resource?.id).toBeUndefined();
326+
});
327+
});
328+
329+
describe('fetchStatus clearing on reset', () => {
330+
it('should clear fetchStatus to idle when resource is reset during an in-flight fetch', () => {
331+
const signIn = new SignIn({ id: 'signin_123', status: 'needs_identifier' } as any);
332+
eventBus.emit('resource:state-change', { resource: signIn, error: null, fetchStatus: 'fetching' });
333+
expect(signInFetchSignal().status).toBe('fetching');
334+
335+
// Reset replaces the resource and clears fetchStatus in one event
336+
const nullSignIn = new SignIn(null);
337+
eventBus.emit('resource:state-change', { resource: nullSignIn, error: null, fetchStatus: 'idle' });
338+
339+
expect(signInFetchSignal().status).toBe('idle');
340+
});
341+
});
271342
});

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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ export class SignIn extends BaseResource implements SignInResource {
572572
this.clientTrustState = data.client_trust_state ?? undefined;
573573
}
574574

575-
eventBus.emit('resource:update', { resource: this });
575+
eventBus.emit('resource:state-change', { resource: this });
576576
return this;
577577
}
578578

@@ -1306,7 +1306,7 @@ class SignInFuture implements SignInFutureResource {
13061306

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

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ export class SignUp extends BaseResource implements SignUpResource {
517517
this.locale = data.locale;
518518
}
519519

520-
eventBus.emit('resource:update', { resource: this });
520+
eventBus.emit('resource:state-change', { resource: this });
521521
return this;
522522
}
523523

@@ -1126,7 +1126,7 @@ class SignUpFuture implements SignUpFutureResource {
11261126

11271127
/**
11281128
* Resets the current sign-up attempt by clearing all local state back to null.
1129-
* Unlike other methods, this does NOT emit resource:fetch with 'fetching' status,
1129+
* Unlike other methods, this does NOT set fetchStatus to 'fetching',
11301130
* allowing for smooth UI transitions without loading states.
11311131
*/
11321132
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

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,9 +2032,8 @@ describe('SignIn', () => {
20322032
resetSignIn: vi.fn().mockImplementation(function (this: typeof mockClient) {
20332033
const newSignIn = new SignIn(null);
20342034
this.signIn = newSignIn;
2035-
// Emit events like the real implementation
2036-
eventBus.emit('resource:error', { resource: newSignIn, error: null });
2037-
// Also update signals directly since State isn't set up in tests
2035+
eventBus.emit('resource:state-change', { resource: newSignIn, error: null, fetchStatus: 'idle' });
2036+
// Also update signals directly since State isn't set up in these tests
20382037
signInResourceSignal({ resource: newSignIn });
20392038
signInErrorSignal({ error: null });
20402039
}),
@@ -2052,17 +2051,17 @@ describe('SignIn', () => {
20522051
signInErrorSignal({ error: null });
20532052
});
20542053

2055-
it('does NOT emit resource:fetch with status fetching', async () => {
2054+
it('does NOT set fetchStatus to fetching', async () => {
20562055
const emitSpy = vi.spyOn(eventBus, 'emit');
20572056
const mockFetch = vi.fn();
20582057
BaseResource._fetch = mockFetch;
20592058

20602059
const signIn = new SignIn({ id: 'signin_123', status: 'needs_first_factor' } as any);
20612060
await signIn.__internal_future.reset();
20622061

2063-
// Verify that resource:fetch was NOT called with status: 'fetching'
2062+
// Verify that no event was emitted with fetchStatus: 'fetching'
20642063
const fetchingCalls = emitSpy.mock.calls.filter(
2065-
call => call[0] === 'resource:fetch' && call[1]?.status === 'fetching',
2064+
call => call[0] === 'resource:state-change' && call[1]?.fetchStatus === 'fetching',
20662065
);
20672066
expect(fetchingCalls).toHaveLength(0);
20682067
// Verify no API calls were made

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -989,9 +989,8 @@ describe('SignUp', () => {
989989
resetSignUp: vi.fn().mockImplementation(function (this: typeof mockClient) {
990990
const newSignUp = new SignUp(null);
991991
this.signUp = newSignUp;
992-
// Emit events like the real implementation
993-
eventBus.emit('resource:error', { resource: newSignUp, error: null });
994-
// Also update signals directly since State isn't set up in tests
992+
eventBus.emit('resource:state-change', { resource: newSignUp, error: null, fetchStatus: 'idle' });
993+
// Also update signals directly since State isn't set up in these tests
995994
signUpResourceSignal({ resource: newSignUp });
996995
signUpErrorSignal({ error: null });
997996
}),
@@ -1009,17 +1008,17 @@ describe('SignUp', () => {
10091008
signUpErrorSignal({ error: null });
10101009
});
10111010

1012-
it('does NOT emit resource:fetch with status fetching', async () => {
1011+
it('does NOT set fetchStatus to fetching', async () => {
10131012
const emitSpy = vi.spyOn(eventBus, 'emit');
10141013
const mockFetch = vi.fn();
10151014
BaseResource._fetch = mockFetch;
10161015

10171016
const signUp = new SignUp({ id: 'signup_123', status: 'missing_requirements' } as any);
10181017
await signUp.__internal_future.reset();
10191018

1020-
// Verify that resource:fetch was NOT called with status: 'fetching'
1019+
// Verify that no event was emitted with fetchStatus: 'fetching'
10211020
const fetchingCalls = emitSpy.mock.calls.filter(
1022-
call => call[0] === 'resource:fetch' && call[1]?.status === 'fetching',
1021+
call => call[0] === 'resource:state-change' && call[1]?.fetchStatus === 'fetching',
10231022
);
10241023
expect(fetchingCalls).toHaveLength(0);
10251024
// Verify no API calls were made

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
import { describe, expect, it } from 'vitest';
1+
import { describe, expect, it, vi } from 'vitest';
22

3+
import { waitlistResourceSignal } from '../../signals';
4+
import { State } from '../../state';
35
import { Waitlist } from '../internal';
46

57
describe('Waitlist', () => {
@@ -17,4 +19,25 @@ describe('Waitlist', () => {
1719
updatedAt: new Date(5678),
1820
});
1921
});
22+
23+
describe('join()', () => {
24+
it('should update the waitlist instance and signal with data from the server response', async () => {
25+
const state = new State();
26+
const waitlist = state.__internal_waitlist;
27+
expect(waitlist.id).toBe('');
28+
29+
// Mock the network boundary — _basePost calls fromJSON on `this` with the response
30+
vi.spyOn(waitlist as any, '_basePost').mockImplementation(async function (this: Waitlist) {
31+
(this as any).fromJSON({ id: 'wl_abc123', created_at: 12345, updated_at: 5678 });
32+
return this;
33+
});
34+
35+
await waitlist.join({ emailAddress: 'test@example.com' });
36+
37+
// The instance should be updated in-place, and the signal should reflect it
38+
expect(waitlist.id).toBe('wl_abc123');
39+
expect(waitlistResourceSignal().resource).toBe(waitlist);
40+
expect(waitlistResourceSignal().resource?.id).toBe('wl_abc123');
41+
});
42+
});
2043
});

0 commit comments

Comments
 (0)