Skip to content

Commit e436882

Browse files
jacekradkojacekradko
authored
fix(clerk-js): Remove duplicate prepare first factor calls (#6134)
1 parent 10f3dda commit e436882

File tree

4 files changed

+238
-6
lines changed

4 files changed

+238
-6
lines changed

‎.changeset/real-bees-post.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+
Reworked the cache key creation logic in SignInFactorOneCodeForm.tsx not to rely on sign_in.id, which can change after host app re-renders

‎packages/clerk-js/bundlewatch.config.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"files": [
3-
{ "path": "./dist/clerk.js", "maxSize": "608.92kB" },
3+
{ "path": "./dist/clerk.js", "maxSize": "610kB" },
44
{ "path": "./dist/clerk.browser.js", "maxSize": "70.16KB" },
55
{ "path": "./dist/clerk.legacy.browser.js", "maxSize": "113KB" },
66
{ "path": "./dist/clerk.headless*.js", "maxSize": "53.06KB" },

‎packages/clerk-js/src/ui/components/SignIn/SignInFactorOneCodeForm.tsx

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { isUserLockedError } from '@clerk/shared/error';
22
import { useClerk } from '@clerk/shared/react';
33
import type { EmailCodeFactor, PhoneCodeFactor, ResetPasswordCodeFactor } from '@clerk/types';
4+
import { useMemo } from 'react';
45

56
import { useCardState } from '@/ui/elements/contexts';
67
import type { VerificationCodeCardProps } from '@/ui/elements/VerificationCodeCard';
@@ -41,6 +42,31 @@ export const SignInFactorOneCodeForm = (props: SignInFactorOneCodeFormProps) =>
4142

4243
const shouldAvoidPrepare = signIn.firstFactorVerification.status === 'verified' && props.factorAlreadyPrepared;
4344

45+
const cacheKey = useMemo(() => {
46+
const factor = props.factor;
47+
let factorKey = factor.strategy;
48+
49+
if ('emailAddressId' in factor) {
50+
factorKey += `_${factor.emailAddressId}`;
51+
}
52+
if ('phoneNumberId' in factor) {
53+
factorKey += `_${factor.phoneNumberId}`;
54+
}
55+
if ('channel' in factor && factor.channel) {
56+
factorKey += `_${factor.channel}`;
57+
}
58+
59+
return {
60+
name: 'signIn.prepareFirstFactor',
61+
factorKey,
62+
};
63+
}, [
64+
props.factor.strategy,
65+
'emailAddressId' in props.factor ? props.factor.emailAddressId : undefined,
66+
'phoneNumberId' in props.factor ? props.factor.phoneNumberId : undefined,
67+
'channel' in props.factor ? props.factor.channel : undefined,
68+
]);
69+
4470
const goBack = () => {
4571
return navigate('../');
4672
};
@@ -64,11 +90,7 @@ export const SignInFactorOneCodeForm = (props: SignInFactorOneCodeFormProps) =>
6490
?.prepareFirstFactor(props.factor)
6591
.then(() => props.onFactorPrepare())
6692
.catch(err => handleError(err, [], card.setError)),
67-
{
68-
name: 'signIn.prepareFirstFactor',
69-
factor: props.factor,
70-
id: signIn.id,
71-
},
93+
cacheKey,
7294
{
7395
staleTime: 100,
7496
},

‎packages/clerk-js/src/ui/components/SignIn/__tests__/SignInFactorOneCodeForm.spec.tsx

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
3+
import { render } from '../../../../vitestUtils';
4+
import { CardStateProvider } from '../../../elements/contexts';
5+
import { clearFetchCache, useFetch } from '../../../hooks';
6+
import { localizationKeys } from '../../../localization';
7+
import { bindCreateFixtures } from '../../../utils/vitest/createFixtures';
8+
import { SignInFactorOneCodeForm } from '../SignInFactorOneCodeForm';
9+
10+
const { createFixtures } = bindCreateFixtures('SignIn');
11+
12+
vi.mock('../../../hooks', async () => {
13+
const actual = await vi.importActual('../../../hooks');
14+
return {
15+
...actual,
16+
useFetch: vi.fn(),
17+
};
18+
});
19+
20+
describe('SignInFactorOneCodeForm', () => {
21+
beforeEach(() => {
22+
clearFetchCache();
23+
vi.mocked(useFetch).mockClear();
24+
});
25+
26+
const renderWithProviders = (component: React.ReactElement, options?: any) => {
27+
return render(<CardStateProvider>{component}</CardStateProvider>, options);
28+
};
29+
30+
const defaultProps = {
31+
factor: {
32+
strategy: 'phone_code' as const,
33+
phoneNumberId: 'idn_123',
34+
safeIdentifier: '+1234567890',
35+
},
36+
factorAlreadyPrepared: false,
37+
onFactorPrepare: vi.fn(),
38+
cardTitle: localizationKeys('signIn.phoneCode.title'),
39+
cardSubtitle: localizationKeys('signIn.phoneCode.subtitle'),
40+
inputLabel: localizationKeys('signIn.phoneCode.formTitle'),
41+
resendButton: localizationKeys('signIn.phoneCode.resendButton'),
42+
};
43+
44+
describe('Cache Key Generation', () => {
45+
it('generates cache key without signIn.id to prevent extra API calls', async () => {
46+
const { wrapper } = await createFixtures(f => {
47+
f.withPhoneNumber();
48+
f.startSignInWithPhoneNumber({ supportPhoneCode: true });
49+
});
50+
51+
renderWithProviders(<SignInFactorOneCodeForm {...defaultProps} />, { wrapper });
52+
53+
expect(vi.mocked(useFetch)).toHaveBeenCalledWith(
54+
expect.any(Function),
55+
{
56+
name: 'signIn.prepareFirstFactor',
57+
factorKey: 'phone_code_idn_123',
58+
},
59+
{
60+
staleTime: 100,
61+
},
62+
);
63+
});
64+
65+
it('includes channel in cache key for phone code with WhatsApp', async () => {
66+
const { wrapper } = await createFixtures(f => {
67+
f.withPhoneNumber();
68+
f.startSignInWithPhoneNumber({ supportPhoneCode: true });
69+
});
70+
71+
const phonePropsWithChannel = {
72+
factor: {
73+
strategy: 'phone_code' as const,
74+
phoneNumberId: 'idn_123',
75+
safeIdentifier: '+1234567890',
76+
channel: 'whatsapp' as const,
77+
},
78+
factorAlreadyPrepared: false,
79+
onFactorPrepare: vi.fn(),
80+
cardTitle: localizationKeys('signIn.phoneCode.title'),
81+
cardSubtitle: localizationKeys('signIn.phoneCode.subtitle'),
82+
inputLabel: localizationKeys('signIn.phoneCode.formTitle'),
83+
resendButton: localizationKeys('signIn.phoneCode.resendButton'),
84+
};
85+
86+
renderWithProviders(<SignInFactorOneCodeForm {...phonePropsWithChannel} />, { wrapper });
87+
88+
expect(vi.mocked(useFetch)).toHaveBeenCalledWith(
89+
expect.any(Function),
90+
{
91+
name: 'signIn.prepareFirstFactor',
92+
factorKey: 'phone_code_idn_123_whatsapp',
93+
},
94+
{
95+
staleTime: 100,
96+
},
97+
);
98+
});
99+
100+
it('still calls prepare when factor is already prepared but verification not verified', async () => {
101+
const { wrapper } = await createFixtures(f => {
102+
f.withPhoneNumber();
103+
f.startSignInWithPhoneNumber({ supportPhoneCode: true });
104+
});
105+
106+
const props = {
107+
factor: {
108+
strategy: 'phone_code' as const,
109+
phoneNumberId: 'idn_123',
110+
safeIdentifier: '+1234567890',
111+
},
112+
factorAlreadyPrepared: true,
113+
onFactorPrepare: vi.fn(),
114+
cardTitle: localizationKeys('signIn.phoneCode.title'),
115+
cardSubtitle: localizationKeys('signIn.phoneCode.subtitle'),
116+
inputLabel: localizationKeys('signIn.phoneCode.formTitle'),
117+
resendButton: localizationKeys('signIn.phoneCode.resendButton'),
118+
};
119+
120+
renderWithProviders(<SignInFactorOneCodeForm {...props} />, { wrapper });
121+
122+
expect(vi.mocked(useFetch)).toHaveBeenCalledWith(expect.any(Function), expect.any(Object), expect.any(Object));
123+
});
124+
});
125+
126+
describe('shouldAvoidPrepare Logic', () => {
127+
it('still calls prepare when factor is already prepared but verification not verified', async () => {
128+
const { wrapper } = await createFixtures(f => {
129+
f.withPhoneNumber();
130+
f.startSignInWithPhoneNumber({ supportPhoneCode: true });
131+
});
132+
133+
const propsWithFactorPrepared = {
134+
...defaultProps,
135+
factorAlreadyPrepared: true,
136+
};
137+
138+
renderWithProviders(<SignInFactorOneCodeForm {...propsWithFactorPrepared} />, { wrapper });
139+
140+
expect(vi.mocked(useFetch)).toHaveBeenCalledWith(
141+
expect.any(Function), // fetcher should still be a function because shouldAvoidPrepare requires BOTH conditions
142+
expect.any(Object),
143+
expect.any(Object),
144+
);
145+
});
146+
147+
it('allows prepare when factor is not already prepared', async () => {
148+
const { wrapper } = await createFixtures(f => {
149+
f.withPhoneNumber();
150+
f.startSignInWithPhoneNumber({ supportPhoneCode: true });
151+
});
152+
153+
const propsWithFactorNotPrepared = {
154+
...defaultProps,
155+
factorAlreadyPrepared: false,
156+
};
157+
158+
renderWithProviders(<SignInFactorOneCodeForm {...propsWithFactorNotPrepared} />, { wrapper });
159+
160+
expect(vi.mocked(useFetch)).toHaveBeenCalledWith(
161+
expect.any(Function), // fetcher should be a function when prepare is allowed
162+
expect.any(Object),
163+
expect.any(Object),
164+
);
165+
});
166+
});
167+
168+
describe('Component Rendering', () => {
169+
it('renders phone code verification form', async () => {
170+
const { wrapper } = await createFixtures(f => {
171+
f.withPhoneNumber();
172+
f.startSignInWithPhoneNumber({ supportPhoneCode: true });
173+
});
174+
175+
renderWithProviders(<SignInFactorOneCodeForm {...defaultProps} />, { wrapper });
176+
177+
expect(document.querySelector('input[type="text"]')).toBeInTheDocument();
178+
});
179+
180+
it('renders email code verification form', async () => {
181+
const { wrapper } = await createFixtures(f => {
182+
f.withEmailAddress();
183+
f.startSignInWithEmailAddress({ supportEmailCode: true });
184+
});
185+
186+
const emailProps = {
187+
factor: {
188+
strategy: 'email_code' as const,
189+
emailAddressId: 'idn_456',
190+
safeIdentifier: 'test@example.com',
191+
},
192+
factorAlreadyPrepared: false,
193+
onFactorPrepare: vi.fn(),
194+
cardTitle: localizationKeys('signIn.emailCode.title'),
195+
cardSubtitle: localizationKeys('signIn.emailCode.subtitle'),
196+
inputLabel: localizationKeys('signIn.emailCode.formTitle'),
197+
resendButton: localizationKeys('signIn.emailCode.resendButton'),
198+
};
199+
200+
renderWithProviders(<SignInFactorOneCodeForm {...emailProps} />, { wrapper });
201+
202+
expect(document.querySelector('input[type="text"]')).toBeInTheDocument();
203+
});
204+
});
205+
});

0 commit comments

Comments
 (0)