Skip to content

Commit dd2cbfe

Browse files
panteliselefpanteliselef
authored
fix(clerk-js,types): Close modals when setActive({redirectUrl}) is called (#5092)
1 parent 7076310 commit dd2cbfe

22 files changed

+357
-30
lines changed

‎.changeset/beige-colts-fold.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/clerk-react': patch
3+
---
4+
5+
Exclude `__internal_addNavigationListener` from `IsomorphicClerk`.

‎.changeset/eighty-pigs-sniff.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/types': minor
3+
---
4+
5+
Introduce `__internal_addNavigationListener` method the `Clerk` singleton.

‎.changeset/violet-fishes-provide.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
---
4+
5+
Bug fix: Close modals when calling `Clerk.navigate()` or `Clerk.setActive({redirectUrl})`.

‎eslint.config.mjs

+83
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,75 @@ const ECMA_VERSION = 2021,
1919
TEST_FILES = ['**/*.test.js', '**/*.test.jsx', '**/*.test.ts', '**/*.test.tsx', '**/test/**', '**/__tests__/**'],
2020
TYPESCRIPT_FILES = ['**/*.cts', '**/*.mts', '**/*.ts', '**/*.tsx'];
2121

22+
const noNavigateUseClerk = {
23+
meta: {
24+
type: 'problem',
25+
docs: {
26+
description: 'Disallow any usage of `navigate` from `useClerk()`',
27+
recommended: false,
28+
},
29+
messages: {
30+
noNavigate:
31+
'Usage of `navigate` from `useClerk()` is not allowed.\nUse `useRouter().navigate` to navigate in-between flows or `setActive({ redirectUrl })`.',
32+
},
33+
schema: [],
34+
},
35+
create(context) {
36+
const sourceCode = context.getSourceCode();
37+
38+
return {
39+
// Case 1: Destructuring `navigate` from `useClerk()`
40+
VariableDeclarator(node) {
41+
if (
42+
node.id.type === 'ObjectPattern' && // Checks if it's an object destructuring
43+
node.init?.type === 'CallExpression' &&
44+
node.init.callee.name === 'useClerk'
45+
) {
46+
for (const property of node.id.properties) {
47+
if (property.type === 'Property' && property.key.name === 'navigate') {
48+
context.report({
49+
node: property,
50+
messageId: 'noNavigate',
51+
});
52+
}
53+
}
54+
}
55+
},
56+
57+
// Case 2 & 3: Accessing `navigate` on a variable or directly calling `useClerk().navigate`
58+
MemberExpression(node) {
59+
if (
60+
node.property.name === 'navigate' &&
61+
node.object.type === 'CallExpression' &&
62+
node.object.callee.name === 'useClerk'
63+
) {
64+
// Case 3: Direct `useClerk().navigate`
65+
context.report({
66+
node,
67+
messageId: 'noNavigate',
68+
});
69+
} else if (node.property.name === 'navigate' && node.object.type === 'Identifier') {
70+
// Case 2: `clerk.navigate` where `clerk` is assigned `useClerk()`
71+
const scope = sourceCode.scopeManager.acquire(node);
72+
if (!scope) return;
73+
74+
const variable = scope.variables.find(v => v.name === node.object.name);
75+
76+
if (
77+
variable?.defs?.[0]?.node?.init?.type === 'CallExpression' &&
78+
variable.defs[0].node.init.callee.name === 'useClerk'
79+
) {
80+
context.report({
81+
node,
82+
messageId: 'noNavigate',
83+
});
84+
}
85+
}
86+
},
87+
};
88+
},
89+
};
90+
2291
export default tseslint.config([
2392
{
2493
name: 'repo/ignores',
@@ -285,6 +354,20 @@ export default tseslint.config([
285354
'react-hooks/rules-of-hooks': 'warn',
286355
},
287356
},
357+
{
358+
name: 'packages/clerk-js',
359+
files: ['packages/clerk-js/src/ui/**/*'],
360+
plugins: {
361+
'custom-rules': {
362+
rules: {
363+
'no-navigate-useClerk': noNavigateUseClerk,
364+
},
365+
},
366+
},
367+
rules: {
368+
'custom-rules/no-navigate-useClerk': 'error',
369+
},
370+
},
288371
{
289372
name: 'packages/expo-passkeys',
290373
files: ['packages/expo-passkeys/src/**/*'],

‎integration/templates/react-vite/src/buttons/index.tsx

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { SignInButton, SignUpButton } from '@clerk/clerk-react';
2+
3+
export default function Home() {
4+
return (
5+
<main>
6+
<SignInButton
7+
mode='modal'
8+
forceRedirectUrl='/protected'
9+
signUpForceRedirectUrl='/protected'
10+
>
11+
Sign in button (force)
12+
</SignInButton>
13+
14+
<SignInButton
15+
mode='modal'
16+
fallbackRedirectUrl='/protected'
17+
>
18+
Sign in button (fallback)
19+
</SignInButton>
20+
21+
<SignUpButton
22+
mode='modal'
23+
forceRedirectUrl='/protected'
24+
signInForceRedirectUrl='/protected'
25+
>
26+
Sign up button (force)
27+
</SignUpButton>
28+
29+
<SignUpButton
30+
mode='modal'
31+
fallbackRedirectUrl='/protected'
32+
>
33+
Sign up button (fallback)
34+
</SignUpButton>
35+
</main>
36+
);
37+
}

‎integration/templates/react-vite/src/main.tsx

+5
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import OrganizationProfile from './organization-profile';
1818
import OrganizationList from './organization-list';
1919
import CreateOrganization from './create-organization';
2020
import OrganizationSwitcher from './organization-switcher';
21+
import Buttons from './buttons';
2122

2223
const Root = () => {
2324
const navigate = useNavigate();
@@ -68,6 +69,10 @@ const router = createBrowserRouter([
6869
path: '/protected',
6970
element: <Protected />,
7071
},
72+
{
73+
path: '/buttons',
74+
element: <Buttons />,
75+
},
7176
{
7277
path: '/custom-user-profile/*',
7378
element: <UserProfileCustom />,

‎integration/testUtils/signInPageObject.ts

+5
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ export const createSignInComponentPageObject = (testArgs: TestArgs) => {
2424
waitForMounted: (selector = '.cl-signIn-root') => {
2525
return page.waitForSelector(selector, { state: 'attached' });
2626
},
27+
waitForModal: (state?: 'open' | 'closed') => {
28+
return page.waitForSelector('.cl-modalContent:has(.cl-signIn-root)', {
29+
state: state === 'closed' ? 'detached' : 'attached',
30+
});
31+
},
2732
setIdentifier: (val: string) => {
2833
return self.getIdentifierInput().fill(val);
2934
},

‎integration/testUtils/signUpPageObject.ts

+5
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ export const createSignUpComponentPageObject = (testArgs: TestArgs) => {
2727
waitForMounted: (selector = '.cl-signUp-root') => {
2828
return page.waitForSelector(selector, { state: 'attached' });
2929
},
30+
waitForModal: (state?: 'open' | 'closed') => {
31+
return page.waitForSelector('.cl-modalContent:has(.cl-signUp-root)', {
32+
state: state === 'closed' ? 'detached' : 'attached',
33+
});
34+
},
3035
signUpWithOauth: (provider: string) => {
3136
return page.getByRole('button', { name: new RegExp(`continue with ${provider}`, 'gi') });
3237
},

‎integration/testUtils/userProfilePageObject.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,10 @@ export const createUserProfileComponentPageObject = (testArgs: TestArgs) => {
6666
typeEmailAddress: (value: string) => {
6767
return page.getByLabel(/Email address/i).fill(value);
6868
},
69-
waitForUserProfileModal: () => {
70-
return page.waitForSelector('.cl-modalContent > .cl-userProfile-root', { state: 'visible' });
69+
waitForUserProfileModal: (state?: 'open' | 'closed') => {
70+
return page.waitForSelector('.cl-modalContent:has(.cl-userProfile-root)', {
71+
state: state === 'closed' ? 'detached' : 'attached',
72+
});
7173
},
7274
};
7375
return self;

‎integration/tests/oauth-flows.test.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import { test } from '@playwright/test';
21
import { createClerkClient } from '@clerk/backend';
2+
import { test } from '@playwright/test';
33

44
import { appConfigs } from '../presets';
5+
import { instanceKeys } from '../presets/envs';
56
import type { FakeUser } from '../testUtils';
67
import { createTestUtils, testAgainstRunningApps } from '../testUtils';
7-
import { instanceKeys } from '../presets/envs';
88
import { createUserService } from '../testUtils/usersService';
99

1010
testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('oauth flows @nextjs', ({ app }) => {
@@ -78,7 +78,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('oauth flo
7878

7979
await u.page.getByText('Sign in button (force)').click();
8080

81-
await u.po.signIn.waitForMounted();
81+
await u.po.signIn.waitForModal();
8282
await u.page.getByRole('button', { name: 'E2E OAuth Provider' }).click();
8383
await u.page.getByText('Sign in to oauth-provider').waitFor();
8484

@@ -103,7 +103,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('oauth flo
103103

104104
await u.page.getByText('Sign up button (force)').click();
105105

106-
await u.po.signUp.waitForMounted();
106+
await u.po.signUp.waitForModal();
107107
await u.page.getByRole('button', { name: 'E2E OAuth Provider' }).click();
108108
await u.page.getByText('Sign in to oauth-provider').waitFor();
109109

‎integration/tests/sign-in-flow.test.ts

+10
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,16 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('sign in f
4040
await u.po.expect.toBeSignedIn();
4141
});
4242

43+
test('(modal) sign in with email and instant password', async ({ page, context }) => {
44+
const u = createTestUtils({ app, page, context });
45+
await u.page.goToRelative('/buttons');
46+
await u.page.getByText('Sign in button (force)').click();
47+
await u.po.signIn.waitForModal();
48+
await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password });
49+
await u.po.expect.toBeSignedIn();
50+
await u.po.signIn.waitForModal('closed');
51+
});
52+
4353
test('sign in with email code', async ({ page, context }) => {
4454
const u = createTestUtils({ app, page, context });
4555
await u.po.signIn.goTo();

‎integration/tests/sign-in-or-up-flow.test.ts

+43
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,20 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withSignInOrUpFlow] })('sign-
6161
await u.po.expect.toBeSignedIn();
6262
});
6363

64+
test('(modal) sign in with email code', async ({ page, context }) => {
65+
const u = createTestUtils({ app, page, context });
66+
await u.page.goToRelative('/buttons');
67+
await u.page.getByText('Sign in button (fallback)').click();
68+
await u.po.signIn.waitForModal();
69+
await u.po.signIn.getIdentifierInput().fill(fakeUser.email);
70+
await u.po.signIn.continue();
71+
await u.po.signIn.getUseAnotherMethodLink().click();
72+
await u.po.signIn.getAltMethodsEmailCodeButton().click();
73+
await u.po.signIn.enterTestOtpCode();
74+
await u.po.expect.toBeSignedIn();
75+
await u.po.signIn.waitForModal('closed');
76+
});
77+
6478
test('sign in with phone number and password', async ({ page, context }) => {
6579
const u = createTestUtils({ app, page, context });
6680
await u.po.signIn.goTo();
@@ -221,6 +235,35 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withSignInOrUpFlow] })('sign-
221235
await fakeUser.deleteIfExists();
222236
});
223237

238+
test('(modal) sign up with username, email, and password', async ({ page, context }) => {
239+
const u = createTestUtils({ app, page, context });
240+
const fakeUser = u.services.users.createFakeUser({
241+
fictionalEmail: true,
242+
withPassword: true,
243+
withUsername: true,
244+
});
245+
246+
await u.page.goToRelative('/buttons');
247+
await u.page.getByText('Sign in button (fallback)').click();
248+
await u.po.signIn.waitForModal();
249+
await u.po.signIn.setIdentifier(fakeUser.username);
250+
await u.po.signIn.continue();
251+
252+
const prefilledUsername = u.po.signUp.getUsernameInput();
253+
await expect(prefilledUsername).toHaveValue(fakeUser.username);
254+
255+
await u.po.signUp.setEmailAddress(fakeUser.email);
256+
await u.po.signUp.setPassword(fakeUser.password);
257+
await u.po.signUp.continue();
258+
259+
await u.po.signUp.enterTestOtpCode();
260+
261+
await u.po.expect.toBeSignedIn();
262+
await u.po.signIn.waitForModal('closed');
263+
264+
await fakeUser.deleteIfExists();
265+
});
266+
224267
test('sign up, sign out and sign in again', async ({ page, context }) => {
225268
const u = createTestUtils({ app, page, context });
226269
const fakeUser = u.services.users.createFakeUser({

‎integration/tests/sign-up-flow.test.ts

+32-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { appConfigs } from '../presets';
44
import { createTestUtils, testAgainstRunningApps } from '../testUtils';
55

66
testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('sign up flow @generic @nextjs', ({ app }) => {
7-
test.describe.configure({ mode: 'serial' });
7+
test.describe.configure({ mode: 'parallel' });
88

99
test.afterAll(async () => {
1010
await app.teardown();
@@ -90,6 +90,37 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('sign up f
9090
await fakeUser.deleteIfExists();
9191
});
9292

93+
test('(modal) can sign up with phone number', async ({ page, context }) => {
94+
const u = createTestUtils({ app, page, context });
95+
const fakeUser = u.services.users.createFakeUser({
96+
fictionalEmail: true,
97+
withPhoneNumber: true,
98+
withUsername: true,
99+
});
100+
101+
// Open modal
102+
await u.page.goToRelative('/buttons');
103+
await u.page.getByText('Sign up button (fallback)').click();
104+
await u.po.signUp.waitForModal();
105+
106+
// Fill in sign up form
107+
await u.po.signUp.signUp({
108+
email: fakeUser.email,
109+
phoneNumber: fakeUser.phoneNumber,
110+
password: fakeUser.password,
111+
});
112+
113+
// Verify email
114+
await u.po.signUp.enterTestOtpCode();
115+
// Verify phone number
116+
await u.po.signUp.enterTestOtpCode();
117+
118+
// Check if user is signed in
119+
await u.po.expect.toBeSignedIn();
120+
await u.po.signUp.waitForModal('closed');
121+
await fakeUser.deleteIfExists();
122+
});
123+
93124
test('sign up with first name, last name, email, phone and password', async ({ page, context }) => {
94125
const u = createTestUtils({ app, page, context });
95126
const fakeUser = u.services.users.createFakeUser({

0 commit comments

Comments
 (0)