Skip to content

Commit dd2cbfe

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

22 files changed

Lines changed: 357 additions & 30 deletions

‎.changeset/beige-colts-fold.md‎

Lines changed: 5 additions & 0 deletions
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‎

Lines changed: 5 additions & 0 deletions
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‎

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+
Bug fix: Close modals when calling `Clerk.navigate()` or `Clerk.setActive({redirectUrl})`.

‎eslint.config.mjs‎

Lines changed: 83 additions & 0 deletions
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‎

Lines changed: 37 additions & 0 deletions
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‎

Lines changed: 5 additions & 0 deletions
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‎

Lines changed: 5 additions & 0 deletions
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‎

Lines changed: 5 additions & 0 deletions
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‎

Lines changed: 4 additions & 2 deletions
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‎

Lines changed: 4 additions & 4 deletions
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

0 commit comments

Comments
 (0)