From 97f1bfca56b4df0303d8e39f2369e667e9a9af43 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Tue, 4 Feb 2025 11:41:03 +0200 Subject: [PATCH 1/7] chore(clerk-js): Eslint rules to prevent incorrect usage of Clerk.setActive and Clerk.navigate inside UI components --- eslint.config.mjs | 127 ++++++++++++++++++ .../clerk-js/src/ui/router/BaseRouter.tsx | 2 + .../clerk-js/src/ui/router/PathRouter.tsx | 2 + 3 files changed, 131 insertions(+) diff --git a/eslint.config.mjs b/eslint.config.mjs index 3ecda8961a9..32a7c42532b 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -19,6 +19,117 @@ const ECMA_VERSION = 2021, TEST_FILES = ['**/*.test.js', '**/*.test.jsx', '**/*.test.ts', '**/*.test.tsx', '**/test/**', '**/__tests__/**'], TYPESCRIPT_FILES = ['**/*.cts', '**/*.mts', '**/*.ts', '**/*.tsx']; +const noSetActiveRedirectUrl = { + meta: { + type: 'problem', + docs: { + description: 'Disallow calling `setActive` with `{ redirectUrl }` as a parameter.', + recommended: false, + }, + messages: { + noRedirectUrl: 'Calling `setActive` with `{ redirectUrl }` as an argument is not allowed.', + }, + schema: [], + }, + create(context) { + return { + CallExpression(node) { + // Detect direct function calls: `setActive({ redirectUrl })` + const isDirectCall = node.callee.type === 'Identifier' && node.callee.name === 'setActive'; + + // Detect property calls: `clerk.setActive({ redirectUrl })` or `this.setActive({ redirectUrl })` + const isObjectCall = node.callee.type === 'MemberExpression' && node.callee.property.name === 'setActive'; + + if (!isDirectCall && !isObjectCall) { + return; // Exit if it's not a `setActive` call + } + + // Ensure the first argument is an object containing `{ redirectUrl }` + const firstArg = node.arguments[0]; + + if ( + firstArg && + firstArg.type === 'ObjectExpression' && + firstArg.properties.some(prop => prop.key.name === 'redirectUrl') + ) { + context.report({ + node: firstArg, + messageId: 'noRedirectUrl', + }); + } + }, + }; + }, +}; + +const noNavigateUseClerk = { + meta: { + type: 'problem', + docs: { + description: 'Disallow any usage of `navigate` from `useClerk()`', + recommended: false, + }, + messages: { + noNavigate: 'Usage of `navigate` from `useClerk()` is not allowed. Use `useRouter() instead`.', + }, + schema: [], + }, + create(context) { + const sourceCode = context.getSourceCode(); + + return { + // Case 1: Destructuring `navigate` from `useClerk()` + VariableDeclarator(node) { + if ( + node.id.type === 'ObjectPattern' && // Checks if it's an object destructuring + node.init?.type === 'CallExpression' && + node.init.callee.name === 'useClerk' + ) { + for (const property of node.id.properties) { + if (property.type === 'Property' && property.key.name === 'navigate') { + context.report({ + node: property, + messageId: 'noNavigate', + }); + } + } + } + }, + + // Case 2 & 3: Accessing `navigate` on a variable or directly calling `useClerk().navigate` + MemberExpression(node) { + if ( + node.property.name === 'navigate' && + node.object.type === 'CallExpression' && + node.object.callee.name === 'useClerk' + ) { + // Case 3: Direct `useClerk().navigate` + context.report({ + node, + messageId: 'noNavigate', + }); + } else if (node.property.name === 'navigate' && node.object.type === 'Identifier') { + // Case 2: `clerk.navigate` where `clerk` is assigned `useClerk()` + const scope = sourceCode.scopeManager.acquire(node); + if (!scope) return; + + const variable = scope.variables.find(v => v.name === node.object.name); + + if ( + variable?.defs?.[0]?.node?.init?.type === 'CallExpression' && + variable.defs[0].node.init.callee.name === 'useClerk' + ) { + context.report({ + node, + messageId: 'noNavigate', + }); + } + } + }, + }; + }, +}; + export default tseslint.config([ { name: 'repo/ignores', @@ -285,6 +396,22 @@ export default tseslint.config([ 'react-hooks/rules-of-hooks': 'warn', }, }, + { + name: 'packages/clerk-js', + files: ['packages/clerk-js/src/ui/**/*'], + plugins: { + 'custom-rules': { + rules: { + 'no-navigate-useClerk': noNavigateUseClerk, + 'no-setActive-redirectUrl': noSetActiveRedirectUrl, + }, + }, + }, + rules: { + 'custom-rules/no-navigate-useClerk': 'error', + 'custom-rules/no-setActive-redirectUrl': 'error', + }, + }, { name: 'packages/expo-passkeys', files: ['packages/expo-passkeys/src/**/*'], diff --git a/packages/clerk-js/src/ui/router/BaseRouter.tsx b/packages/clerk-js/src/ui/router/BaseRouter.tsx index 9cdafc3e581..de77fe12653 100644 --- a/packages/clerk-js/src/ui/router/BaseRouter.tsx +++ b/packages/clerk-js/src/ui/router/BaseRouter.tsx @@ -40,6 +40,8 @@ export const BaseRouter = ({ urlStateParam, children, }: BaseRouterProps): JSX.Element => { + // Disabling is acceptable since this is a Router component + // eslint-disable-next-line custom-rules/no-navigate-useClerk const { navigate: externalNavigate } = useClerk(); const [routeParts, setRouteParts] = React.useState({ diff --git a/packages/clerk-js/src/ui/router/PathRouter.tsx b/packages/clerk-js/src/ui/router/PathRouter.tsx index ac4c81109b3..6c1edc35045 100644 --- a/packages/clerk-js/src/ui/router/PathRouter.tsx +++ b/packages/clerk-js/src/ui/router/PathRouter.tsx @@ -12,6 +12,8 @@ interface PathRouterProps { } export const PathRouter = ({ basePath, preservedParams, children }: PathRouterProps): JSX.Element | null => { + // Disabling is acceptable since this is a Router component + // eslint-disable-next-line custom-rules/no-navigate-useClerk const { navigate } = useClerk(); const [stripped, setStripped] = React.useState(false); From 482de37f20513cbde2ef80ff9f6dcfbbc7055e92 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Tue, 4 Feb 2025 11:44:53 +0200 Subject: [PATCH 2/7] chore(clerk-js): Use scoped context to pass provide beforeEmit to setActive This way we do not need to change the public signature of setActive, or remove the deprecation of beforeEmit, since developers that consume the sdk should still use `redirectUrl`. --- packages/clerk-js/src/core/clerk.ts | 13 +++++++--- .../components/UserProfile/DeleteUserForm.tsx | 15 +++++------ packages/clerk-js/src/utils/scopedContext.ts | 25 +++++++++++++++++++ packages/types/src/clerk.ts | 10 ++++++++ 4 files changed, 53 insertions(+), 10 deletions(-) create mode 100644 packages/clerk-js/src/utils/scopedContext.ts diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 4eaddfe4fb4..44eda782bc6 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -15,6 +15,7 @@ import type { AuthenticateWithGoogleOneTapParams, AuthenticateWithMetamaskParams, AuthenticateWithOKXWalletParams, + BeforeEmitCallback, Clerk as ClerkInterface, ClerkAPIError, ClerkAuthenticateWithWeb3Params, @@ -100,6 +101,7 @@ import { import { assertNoLegacyProp } from '../utils/assertNoLegacyProp'; import { memoizeListenerCallback } from '../utils/memoizeStateListenerCallback'; import { RedirectUrls } from '../utils/redirectUrls'; +import { createScopedContext } from '../utils/scopedContext'; import { AuthCookieService } from './auth/AuthCookieService'; import { CaptchaHeartbeat } from './auth/CaptchaHeartbeat'; import { CLERK_SATELLITE_URL, CLERK_SUFFIXED_COOKIES, CLERK_SYNCED, ERROR_CODES } from './constants'; @@ -170,6 +172,8 @@ export class Clerk implements ClerkInterface { public __internal_country?: string | null; public telemetry: TelemetryCollector | undefined; + public __internal_setActiveContext = createScopedContext<{ beforeEmit: BeforeEmitCallback }>(); + protected internal_last_error: ClerkAPIError | null = null; // converted to protected environment to support `updateEnvironment` type assertion protected environment?: EnvironmentResource | null; @@ -906,13 +910,16 @@ export class Clerk implements ClerkInterface { // automatic reloading when reloading shouldn't be happening. const beforeUnloadTracker = this.#options.standardBrowser ? createBeforeUnloadTracker() : undefined; if (beforeEmit) { - beforeUnloadTracker?.startTracking(); - this.#setTransitiveState(); deprecated( 'Clerk.setActive({beforeEmit})', 'Use the `redirectUrl` property instead. Example `Clerk.setActive({redirectUrl:"/"})`', ); - await beforeEmit(newSession); + } + const __beforeEmit = beforeEmit || this.__internal_setActiveContext.get()?.beforeEmit; + if (__beforeEmit) { + beforeUnloadTracker?.startTracking(); + this.#setTransitiveState(); + await __beforeEmit(newSession); beforeUnloadTracker?.stopTracking(); } diff --git a/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx b/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx index 5a90cfadd95..bda3f04323e 100644 --- a/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx +++ b/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx @@ -11,14 +11,14 @@ type DeleteUserFormProps = FormProps; export const DeleteUserForm = withCardStateProvider((props: DeleteUserFormProps) => { const { onReset } = props; const card = useCardState(); - const { afterSignOutUrl, afterMultiSessionSingleSignOutUrl } = useSignOutContext(); + const { navigateAfterMultiSessionSingleSignOutUrl, navigateAfterSignOut } = useSignOutContext(); const { user } = useUser(); const { t } = useLocalizations(); const { otherSessions } = useMultipleSessions({ user }); - const { setActive } = useClerk(); + const { setActive, __internal_setActiveContext } = useClerk(); const [deleteUserWithReverification] = useReverification(() => user?.delete()); - const confirmationField = useFormControl('deleteConfirmation', '', { + const confirmationField = useFormControl('deleteConfirmation', 'Delete account', { type: 'text', label: localizationKeys('userProfile.deletePage.actionDescription'), isRequired: true, @@ -36,11 +36,12 @@ export const DeleteUserForm = withCardStateProvider((props: DeleteUserFormProps) try { await deleteUserWithReverification(); - const redirectUrl = otherSessions.length === 0 ? afterSignOutUrl : afterMultiSessionSingleSignOutUrl; + const beforeEmit = otherSessions.length === 0 ? navigateAfterSignOut : navigateAfterMultiSessionSingleSignOutUrl; - return await setActive({ - session: null, - redirectUrl, + return __internal_setActiveContext.run({ beforeEmit }, () => { + return setActive({ + session: null, + }); }); } catch (e) { handleError(e, [], card.setError); diff --git a/packages/clerk-js/src/utils/scopedContext.ts b/packages/clerk-js/src/utils/scopedContext.ts new file mode 100644 index 00000000000..35e48e93a60 --- /dev/null +++ b/packages/clerk-js/src/utils/scopedContext.ts @@ -0,0 +1,25 @@ +type ScopedContext = { + run(context: T, fn: () => R): Promise>; + get(): T | undefined; +}; + +function createScopedContext(): ScopedContext { + const contextStack: T[] = []; + + return { + async run(context: T, fn: () => R): Promise> { + contextStack.push(context); + try { + return await fn(); + } finally { + contextStack.pop(); + } + }, + get(): T | undefined { + return contextStack[contextStack.length - 1]; + }, + }; +} + +export { createScopedContext }; +export type { ScopedContext }; diff --git a/packages/types/src/clerk.ts b/packages/types/src/clerk.ts index 474a0a279e6..ebcddefd4b4 100644 --- a/packages/types/src/clerk.ts +++ b/packages/types/src/clerk.ts @@ -605,8 +605,18 @@ export interface Clerk { * This funtion is used to reload the initial resources (Environment/Client) from the Frontend API. **/ __internal_reloadInitialResources: () => Promise; + + /** + * + */ + __internal_setActiveContext: ScopedContext<{ beforeEmit: BeforeEmitCallback }>; } +type ScopedContext = { + run(context: T, fn: () => R): Promise>; + get(): T | undefined; +}; + export type HandleOAuthCallbackParams = TransferableOption & SignInForceRedirectUrl & SignInFallbackRedirectUrl & From b2326eab8f66af1b882abd57e5fa05ad31c55ce5 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Tue, 4 Feb 2025 14:45:56 +0200 Subject: [PATCH 3/7] add unit tests for createScopedContext --- .../src/utils/__tests__/scopedContext.test.ts | 170 ++++++++++++++++++ packages/clerk-js/src/utils/scopedContext.ts | 9 +- 2 files changed, 175 insertions(+), 4 deletions(-) create mode 100644 packages/clerk-js/src/utils/__tests__/scopedContext.test.ts diff --git a/packages/clerk-js/src/utils/__tests__/scopedContext.test.ts b/packages/clerk-js/src/utils/__tests__/scopedContext.test.ts new file mode 100644 index 00000000000..f4eb232b7e5 --- /dev/null +++ b/packages/clerk-js/src/utils/__tests__/scopedContext.test.ts @@ -0,0 +1,170 @@ +import { createScopedContext } from '../scopedContext'; + +describe('createScopedContext', () => { + const globalScopedContext = createScopedContext<{ value: string }>(); + + it('initially undefined', () => { + const scopedContext = createScopedContext<{ name: string }>(); + expect(scopedContext.get()).toBeUndefined(); + }); + + it('sync', () => { + const scopedContext = createScopedContext<{ name: string }>(); + void scopedContext.run({ name: 'test' }, () => { + expect(scopedContext.get()).toEqual({ name: 'test' }); + }); + }); + + it('sync callback return', async () => { + const scopedContext = createScopedContext<{ name: string }>(); + const result = scopedContext.run({ name: 'test' }, () => { + return scopedContext.get(); + }); + expect(await result).toEqual({ name: 'test' }); + }); + + it('async', () => { + const scopedContext = createScopedContext<{ name: string }>(); + void scopedContext.run({ name: 'test' }, async () => { + expect(scopedContext.get()).toEqual({ name: 'test' }); + await new Promise(resolve => setTimeout(resolve, 100)); + expect(scopedContext.get()).toEqual({ name: 'test' }); + }); + }); + + it('async callback return', async () => { + const scopedContext = createScopedContext<{ name: string }>(); + + const result = scopedContext.run({ name: 'test' }, async () => { + await new Promise(resolve => setTimeout(resolve, 100)); + return scopedContext.get(); + }); + expect(await result).toEqual({ name: 'test' }); + }); + + it('value inside setTimeout is `undefined`', () => { + const scopedContext = createScopedContext<{ name: string }>(); + + void scopedContext.run({ name: 'test' }, () => { + expect(scopedContext.get()).toEqual({ name: 'test' }); + setTimeout(() => { + expect(scopedContext.get()).toBeUndefined(); + }, 0); + }); + }); + + it('nested .run() and .get()', async () => { + const scopedContext = createScopedContext<{ name: string }>(); + + void scopedContext.run({ name: 'test' }, async () => { + void scopedContext.run({ name: 'before' }, async () => { + expect(scopedContext.get()).toEqual({ name: 'before' }); + }); + + setTimeout(() => { + void scopedContext.run({ name: 'insideTimeout' }, () => { + expect(scopedContext.get()).toEqual({ name: 'insideTimeout' }); + }); + }, 100); + + await new Promise(resolve => setTimeout(resolve, 100)); + + expect(scopedContext.get()).toEqual({ name: 'test' }); + void scopedContext.run({ name: 'after' }, async () => { + expect(scopedContext.get()).toEqual({ name: 'after' }); + }); + }); + }); + + it('global context', async () => { + expect(globalScopedContext.get()).toBeUndefined(); + + await globalScopedContext.run({ value: 'test0' }, () => + expect(globalScopedContext.get()).toEqual({ value: 'test0' }), + ); + + await globalScopedContext.run({ value: 'test0' }, async () => { + expect(globalScopedContext.get()).toEqual({ value: 'test0' }); + + const res = await globalScopedContext.run({ value: 'test0_0' }, async () => { + expect(globalScopedContext.get()).toEqual({ value: 'test0_0' }); + + await globalScopedContext.run({ value: 'test0_1' }, () => { + expect(globalScopedContext.get()).toEqual({ value: 'test0_1' }); + }); + + expect(globalScopedContext.get()).toEqual({ value: 'test0_0' }); + return globalScopedContext.get(); + }); + expect(globalScopedContext.get()).toEqual({ value: 'test0' }); + expect(res).toEqual({ value: 'test0_0' }); + }); + + expect(globalScopedContext.get()).toBeUndefined(); + // THIS SHOULD PASS expect(globalScopedContext.get()).toBeUndefined(); + // THIS SHOULD FAIL expect(globalScopedContext.get()).toEqual({ value: 'test0' }); + + await globalScopedContext.run({ value: 'test1' }, async () => { + expect(globalScopedContext.get()).toEqual({ value: 'test1' }); + await new Promise(resolve => setTimeout(resolve, 100)); + expect(globalScopedContext.get()).toEqual({ value: 'test1' }); + }); + + expect(globalScopedContext.get()).toBeUndefined(); + // THIS SHOULD PASS expect(globalScopedContext.get()).toBeUndefined(); + // THIS SHOULD FAIL expect(globalScopedContext.get()).toEqual({ value: 'test1' }); + + await globalScopedContext.run({ value: 'test2' }, async () => { + expect(globalScopedContext.get()).toEqual({ value: 'test2' }); + await new Promise(resolve => setTimeout(resolve, 10)); + expect(globalScopedContext.get()).toEqual({ value: 'test2' }); + }); + }); + + it('global context nested level 3', async () => { + expect(globalScopedContext.get()).toBeUndefined(); + + await globalScopedContext.run({ value: 'test0' }, async () => { + expect(globalScopedContext.get()).toEqual({ value: 'test0' }); + + const res = await globalScopedContext.run({ value: 'test1' }, async () => { + expect(globalScopedContext.get()).toEqual({ value: 'test1' }); + + await globalScopedContext.run({ value: 'test2' }, () => { + expect(globalScopedContext.get()).toEqual({ value: 'test2' }); + }); + + expect(globalScopedContext.get()).toEqual({ value: 'test1' }); + return globalScopedContext.get(); + }); + expect(globalScopedContext.get()).toEqual({ value: 'test0' }); + expect(res).toEqual({ value: 'test1' }); + }); + + expect(globalScopedContext.get()).toBeUndefined(); + }); + + /** + * This test highlights the limitations for the current implementation. + */ + it('should fail when overlapping asynchronous run calls are not awaited in place and interfere with each other', async () => { + let contextValue1; + let contextValue2; + + const promise1 = globalScopedContext.run({ value: 'first' }, async () => { + await new Promise(resolve => setTimeout(resolve, 100)); + contextValue1 = globalScopedContext.get(); + expect(contextValue1).not.toEqual({ value: 'first' }); + }); + + const promise2 = await globalScopedContext.run({ value: 'second' }, async () => { + expect(globalScopedContext.get()).toEqual({ value: 'second' }); + await new Promise(resolve => setTimeout(resolve, 100)); + contextValue2 = globalScopedContext.get(); + expect(contextValue2).not.toEqual({ value: 'second' }); + }); + + // Wait for both asynchronous operations to complete. + await Promise.all([promise1, promise2]); + }); +}); diff --git a/packages/clerk-js/src/utils/scopedContext.ts b/packages/clerk-js/src/utils/scopedContext.ts index 35e48e93a60..38811baf1c5 100644 --- a/packages/clerk-js/src/utils/scopedContext.ts +++ b/packages/clerk-js/src/utils/scopedContext.ts @@ -4,19 +4,20 @@ type ScopedContext = { }; function createScopedContext(): ScopedContext { - const contextStack: T[] = []; + let currentContext: T | undefined; return { async run(context: T, fn: () => R): Promise> { - contextStack.push(context); + const previousContext = currentContext; + currentContext = context; try { return await fn(); } finally { - contextStack.pop(); + currentContext = previousContext; } }, get(): T | undefined { - return contextStack[contextStack.length - 1]; + return currentContext; }, }; } From 2caf9eca0dd54b3bbdb794a43b61d341b0094779 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Tue, 4 Feb 2025 14:49:20 +0200 Subject: [PATCH 4/7] beforeEmit has priority over redirectUrl --- packages/clerk-js/src/core/clerk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 44eda782bc6..655719377d4 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -923,7 +923,7 @@ export class Clerk implements ClerkInterface { beforeUnloadTracker?.stopTracking(); } - if (redirectUrl && !beforeEmit) { + if (redirectUrl && !__beforeEmit) { beforeUnloadTracker?.startTracking(); this.#setTransitiveState(); From 4a7a152af30048f6b6599a9932ff89376830c428 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Tue, 4 Feb 2025 15:03:06 +0200 Subject: [PATCH 5/7] exclude __internal_setActiveContext from IsomorphicClerk --- packages/react/src/isomorphicClerk.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react/src/isomorphicClerk.ts b/packages/react/src/isomorphicClerk.ts index a71e7b52681..c4830a81ca3 100644 --- a/packages/react/src/isomorphicClerk.ts +++ b/packages/react/src/isomorphicClerk.ts @@ -123,6 +123,7 @@ type IsomorphicLoadedClerk = Without< | 'client' | '__internal_getCachedResources' | '__internal_reloadInitialResources' + | '__internal_setActiveContext' > & { // TODO: Align return type and parms handleRedirectCallback: (params: HandleOAuthCallbackParams) => void; From 6f6c016d7a2faeaf876d8e616e6f540ecb75d340 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Tue, 4 Feb 2025 15:03:54 +0200 Subject: [PATCH 6/7] temp changeset --- .changeset/strange-eels-poke.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/strange-eels-poke.md diff --git a/.changeset/strange-eels-poke.md b/.changeset/strange-eels-poke.md new file mode 100644 index 00000000000..07f74e80749 --- /dev/null +++ b/.changeset/strange-eels-poke.md @@ -0,0 +1,7 @@ +--- +'@clerk/clerk-js': patch +'@clerk/clerk-react': patch +'@clerk/types': patch +--- + +TODO From 5bc5a88033223240e07581cd7caab63a8265e636 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Mon, 3 Feb 2025 14:43:03 +0200 Subject: [PATCH 7/7] chore(e2e): Check if modal closes after user deletion --- .../testUtils/userProfilePageObject.ts | 6 ++- integration/tests/user-profile.test.ts | 52 +++++++++++++++++++ .../components/UserProfile/DeleteUserForm.tsx | 2 +- 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/integration/testUtils/userProfilePageObject.ts b/integration/testUtils/userProfilePageObject.ts index bf8473ed4af..ea0e1f4dea3 100644 --- a/integration/testUtils/userProfilePageObject.ts +++ b/integration/testUtils/userProfilePageObject.ts @@ -66,8 +66,10 @@ export const createUserProfileComponentPageObject = (testArgs: TestArgs) => { typeEmailAddress: (value: string) => { return page.getByLabel(/Email address/i).fill(value); }, - waitForUserProfileModal: () => { - return page.waitForSelector('.cl-modalContent > .cl-userProfile-root', { state: 'visible' }); + waitForUserProfileModal: (state?: 'open' | 'closed') => { + return page.waitForSelector('.cl-modalContent:has(.cl-userProfile-root)', { + state: state === 'closed' ? 'detached' : 'attached', + }); }, }; return self; diff --git a/integration/tests/user-profile.test.ts b/integration/tests/user-profile.test.ts index b97174a6f79..bff3b0427d2 100644 --- a/integration/tests/user-profile.test.ts +++ b/integration/tests/user-profile.test.ts @@ -311,4 +311,56 @@ export default function Page() { expect(sessionCookieList.length).toBe(0); }); + + test('closes the modal after delete', async ({ page, context }) => { + const m = createTestUtils({ app }); + const delFakeUser = m.services.users.createFakeUser({ + withUsername: true, + fictionalEmail: true, + withPhoneNumber: true, + }); + await m.services.users.createBapiUser({ + ...delFakeUser, + username: undefined, + phoneNumber: undefined, + }); + + const u = createTestUtils({ app, page, context }); + await u.po.signIn.goTo(); + await u.po.signIn.waitForMounted(); + await u.po.signIn.signInWithEmailAndInstantPassword({ email: delFakeUser.email, password: delFakeUser.password }); + await u.po.expect.toBeSignedIn(); + + await u.page.goToAppHome(); + + await u.po.userButton.waitForMounted(); + await u.po.userButton.toggleTrigger(); + await u.po.userButton.triggerManageAccount(); + + await u.po.userProfile.waitForUserProfileModal(); + await u.po.userProfile.switchToSecurityTab(); + + await u.page + .getByRole('button', { + name: /delete account/i, + }) + .click(); + + await u.page.locator('input[name=deleteConfirmation]').fill('Delete account'); + + await u.page + .getByRole('button', { + name: /delete account/i, + }) + .click(); + + await u.po.expect.toBeSignedOut(); + await u.po.userProfile.waitForUserProfileModal('closed'); + + await u.page.waitForAppUrl('/'); + + // Make sure that the session cookie is deleted + const sessionCookieList = (await u.page.context().cookies()).filter(cookie => cookie.name.startsWith('__session')); + expect(sessionCookieList.length).toBe(0); + }); }); diff --git a/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx b/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx index bda3f04323e..8d1cacc95ed 100644 --- a/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx +++ b/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx @@ -18,7 +18,7 @@ export const DeleteUserForm = withCardStateProvider((props: DeleteUserFormProps) const { setActive, __internal_setActiveContext } = useClerk(); const [deleteUserWithReverification] = useReverification(() => user?.delete()); - const confirmationField = useFormControl('deleteConfirmation', 'Delete account', { + const confirmationField = useFormControl('deleteConfirmation', '', { type: 'text', label: localizationKeys('userProfile.deletePage.actionDescription'), isRequired: true,