From 97f1bfca56b4df0303d8e39f2369e667e9a9af43 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Tue, 4 Feb 2025 11:41:03 +0200 Subject: [PATCH 01/17] 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 02/17] 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 03/17] 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 04/17] 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 05/17] 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 06/17] 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 07/17] 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, From 527f0ac701e9b085683fc9f13fac2ed24e956a86 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Wed, 5 Feb 2025 12:56:02 +0200 Subject: [PATCH 08/17] fix(nextjs): Shallow routing for in-component navigations --- .../src/app-router/client/ClerkProvider.tsx | 2 ++ .../src/app-router/client/useInternalNavFun.ts | 6 +++--- packages/nextjs/src/global.d.ts | 16 +++++++++++++--- packages/types/src/snapshots.ts | 4 +--- packages/types/src/utils.ts | 8 ++++++++ 5 files changed, 27 insertions(+), 9 deletions(-) diff --git a/packages/nextjs/src/app-router/client/ClerkProvider.tsx b/packages/nextjs/src/app-router/client/ClerkProvider.tsx index b9a9cac154d..c12109385ca 100644 --- a/packages/nextjs/src/app-router/client/ClerkProvider.tsx +++ b/packages/nextjs/src/app-router/client/ClerkProvider.tsx @@ -98,7 +98,9 @@ const NextClientClerkProvider = (props: NextClerkProviderProps) => { const mergedProps = mergeNextClerkPropsWithEnv({ ...props, + // @ts-expect-error Error because of the stricter types of internal `push` routerPush: push, + // @ts-expect-error Error because of the stricter types of internal `replace` routerReplace: replace, }); diff --git a/packages/nextjs/src/app-router/client/useInternalNavFun.ts b/packages/nextjs/src/app-router/client/useInternalNavFun.ts index e1731b58fb0..780b335facc 100644 --- a/packages/nextjs/src/app-router/client/useInternalNavFun.ts +++ b/packages/nextjs/src/app-router/client/useInternalNavFun.ts @@ -15,7 +15,7 @@ export const useInternalNavFun = (props: { windowNav: typeof window.history.pushState | typeof window.history.replaceState | undefined; routerNav: AppRouterInstance['push'] | AppRouterInstance['replace']; name: string; -}) => { +}): NavigationFunction => { const { windowNav, routerNav, name } = props; const pathname = usePathname(); const [isPending, startTransition] = useTransition(); @@ -68,8 +68,8 @@ export const useInternalNavFun = (props: { } }, [pathname, isPending]); - return useCallback((to: string) => { - return getClerkNavigationObject(name).fun(to); + return useCallback((to, metadata) => { + return getClerkNavigationObject(name).fun(to, metadata); // We are not expecting name to change // eslint-disable-next-line react-hooks/exhaustive-deps }, []); diff --git a/packages/nextjs/src/global.d.ts b/packages/nextjs/src/global.d.ts index e9672e5b939..8757bee7f05 100644 --- a/packages/nextjs/src/global.d.ts +++ b/packages/nextjs/src/global.d.ts @@ -14,14 +14,24 @@ declare namespace NodeJS { } } -// eslint-disable-next-line @typescript-eslint/consistent-type-imports -type NextClerkProviderProps = import('./types').NextClerkProviderProps; +type RequireMetadata any> = T extends ( + to: infer To, + metadata?: infer Metadata, +) => infer R + ? (to: To, metadata: Metadata) => R + : never; + +type NavigationFunction = + // eslint-disable-next-line @typescript-eslint/consistent-type-imports + | RequireMetadata> + // eslint-disable-next-line @typescript-eslint/consistent-type-imports + | RequireMetadata>; interface Window { __clerk_internal_navigations: Record< string, { - fun: NonNullable; + fun: NavigationFunction; promisesBuffer: Array<() => void> | undefined; } >; diff --git a/packages/types/src/snapshots.ts b/packages/types/src/snapshots.ts index e32ff390912..78c5715c21e 100644 --- a/packages/types/src/snapshots.ts +++ b/packages/types/src/snapshots.ts @@ -30,9 +30,7 @@ import type { import type { OrganizationSettingsJSON } from './organizationSettings'; import type { SignInJSON } from './signIn'; import type { UserSettingsJSON } from './userSettings'; -import type { Nullable } from './utils'; - -type Override = Omit & U; +import type { Nullable, Override } from './utils'; export type SignInJSONSnapshot = Override< Nullable, diff --git a/packages/types/src/utils.ts b/packages/types/src/utils.ts index 88227403440..861df403ae5 100644 --- a/packages/types/src/utils.ts +++ b/packages/types/src/utils.ts @@ -100,3 +100,11 @@ export type Autocomplete = U | (T & Record = { [P in keyof T as Exclude]: T[P]; }; + +/** + * Overrides the type of existing properties + * const obj = { a: string, b: number } as const; + * type Value = Override + * Value contains: { a:string, b: string } + */ +export type Override = Omit & U; From 79bd7872b076607845b76e69dc8e665d4afd6b7c Mon Sep 17 00:00:00 2001 From: panteliselef Date: Wed, 5 Feb 2025 15:34:25 +0200 Subject: [PATCH 09/17] fix(clerk-js,types): Notify UI Component modals to close after `setActive` --- packages/clerk-js/src/core/clerk.ts | 28 +++++++++++++------ .../components/UserProfile/DeleteUserForm.tsx | 16 ++++++----- .../clerk-js/src/ui/lazyModules/providers.tsx | 2 +- .../clerk-js/src/ui/router/BaseRouter.tsx | 15 +++++----- .../clerk-js/src/ui/router/VirtualRouter.tsx | 19 +++++++++++-- packages/types/src/clerk.ts | 2 ++ 6 files changed, 55 insertions(+), 27 deletions(-) diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 655719377d4..a618fa17d4f 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -191,6 +191,7 @@ export class Clerk implements ClerkInterface { #loaded = false; #listeners: Array<(emission: Resources) => void> = []; + #externalNavigationListeners: Array<() => void> = []; #options: ClerkOptions = {}; #pageLifecycle: ReturnType | null = null; #touchThrottledUntil = 0; @@ -947,7 +948,6 @@ export class Clerk implements ClerkInterface { this.#emit(); await onAfterSetActive(); - this.#resetComponentsState(); }; public addListener = (listener: ListenerCallback): UnsubscribeCallback => { @@ -969,11 +969,24 @@ export class Clerk implements ClerkInterface { return unsubscribe; }; + public __internal_externalNavigationListener = (listener: () => void): UnsubscribeCallback => { + this.#externalNavigationListeners.push(listener); + const unsubscribe = () => { + this.#externalNavigationListeners = this.#externalNavigationListeners.filter(l => l !== listener); + }; + return unsubscribe; + }; + public navigate = async (to: string | undefined, options?: NavigateOptions): Promise => { if (!to || !inBrowser()) { return; } + setTimeout(() => { + // Notify after the navigation, in order to visit a page with the updated context set above. + this.#notifyExternalNavigationListeners(); + }, 0); + let toURL = new URL(to, window.location.href); if (!this.#allowedRedirectProtocols.includes(toURL.protocol)) { @@ -2041,15 +2054,14 @@ export class Clerk implements ClerkInterface { } }; - #broadcastSignOutEvent = () => { - this.#broadcastChannel?.postMessage({ type: 'signout' }); + #notifyExternalNavigationListeners = (): void => { + for (const listener of this.#externalNavigationListeners) { + listener(); + } }; - #resetComponentsState = () => { - if (Clerk.mountComponentRenderer) { - this.closeSignUp(); - this.closeSignIn(); - } + #broadcastSignOutEvent = () => { + this.#broadcastChannel?.postMessage({ type: 'signout' }); }; #setTransitiveState = () => { diff --git a/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx b/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx index 8d1cacc95ed..7f2bd867dbc 100644 --- a/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx +++ b/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx @@ -11,11 +11,11 @@ type DeleteUserFormProps = FormProps; export const DeleteUserForm = withCardStateProvider((props: DeleteUserFormProps) => { const { onReset } = props; const card = useCardState(); - const { navigateAfterMultiSessionSingleSignOutUrl, navigateAfterSignOut } = useSignOutContext(); + const { afterSignOutUrl, afterMultiSessionSingleSignOutUrl } = useSignOutContext(); const { user } = useUser(); const { t } = useLocalizations(); const { otherSessions } = useMultipleSessions({ user }); - const { setActive, __internal_setActiveContext } = useClerk(); + const { setActive } = useClerk(); const [deleteUserWithReverification] = useReverification(() => user?.delete()); const confirmationField = useFormControl('deleteConfirmation', '', { @@ -36,13 +36,15 @@ export const DeleteUserForm = withCardStateProvider((props: DeleteUserFormProps) try { await deleteUserWithReverification(); - const beforeEmit = otherSessions.length === 0 ? navigateAfterSignOut : navigateAfterMultiSessionSingleSignOutUrl; + const redirectUrl = otherSessions.length === 0 ? afterSignOutUrl : afterMultiSessionSingleSignOutUrl; - return __internal_setActiveContext.run({ beforeEmit }, () => { - return setActive({ - session: null, - }); + // return __internal_setActiveContext.run({ beforeEmit }, () => { + // eslint-disable-next-line custom-rules/no-setActive-redirectUrl + return setActive({ + session: null, + redirectUrl, }); + // }); } catch (e) { handleError(e, [], card.setError); } diff --git a/packages/clerk-js/src/ui/lazyModules/providers.tsx b/packages/clerk-js/src/ui/lazyModules/providers.tsx index 8156534a846..e7d01c78b91 100644 --- a/packages/clerk-js/src/ui/lazyModules/providers.tsx +++ b/packages/clerk-js/src/ui/lazyModules/providers.tsx @@ -80,7 +80,7 @@ type LazyModalRendererProps = React.PropsWithChildren< flowName?: FlowMetadata['flow']; startPath?: string; onClose?: ModalProps['handleClose']; - onExternalNavigate?: () => any; + onExternalNavigate: () => any; modalContainerSx?: ThemableCssProp; modalContentSx?: ThemableCssProp; canCloseModal?: boolean; diff --git a/packages/clerk-js/src/ui/router/BaseRouter.tsx b/packages/clerk-js/src/ui/router/BaseRouter.tsx index de77fe12653..1874116aa04 100644 --- a/packages/clerk-js/src/ui/router/BaseRouter.tsx +++ b/packages/clerk-js/src/ui/router/BaseRouter.tsx @@ -15,7 +15,6 @@ interface BaseRouterProps { getPath: () => string; getQueryString: () => string; internalNavigate: (toURL: URL, options?: NavigateOptions) => Promise | any; - onExternalNavigate?: () => any; refreshEvents?: Array; preservedParams?: string[]; urlStateParam?: { @@ -34,7 +33,6 @@ export const BaseRouter = ({ getPath, getQueryString, internalNavigate, - onExternalNavigate, refreshEvents, preservedParams, urlStateParam, @@ -42,7 +40,7 @@ export const BaseRouter = ({ }: 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 { navigate: clerkNavigate } = useClerk(); const [routeParts, setRouteParts] = React.useState({ path: getPath(), @@ -96,11 +94,12 @@ export const BaseRouter = ({ return; } - if (toURL.origin !== window.location.origin || !toURL.pathname.startsWith('/' + basePath)) { - if (onExternalNavigate) { - onExternalNavigate(); - } - const res = await externalNavigate(toURL.href); + const isCrossOrigin = toURL.origin !== window.location.origin; + const isOutsideOfUIComponent = !toURL.pathname.startsWith('/' + basePath); + + if (isOutsideOfUIComponent || isCrossOrigin) { + const res = await clerkNavigate(toURL.href); + // TODO: Since we are closing the modal, why do we need to refresh ? wouldn't that unmount everything causing the state to refresh ? refresh(); return res; } diff --git a/packages/clerk-js/src/ui/router/VirtualRouter.tsx b/packages/clerk-js/src/ui/router/VirtualRouter.tsx index a589a33c3cf..4b3f61f4aa9 100644 --- a/packages/clerk-js/src/ui/router/VirtualRouter.tsx +++ b/packages/clerk-js/src/ui/router/VirtualRouter.tsx @@ -1,4 +1,5 @@ -import React from 'react'; +import { useClerk } from '@clerk/shared/react'; +import React, { useEffect } from 'react'; import { useClerkModalStateParams } from '../hooks'; import { BaseRouter } from './BaseRouter'; @@ -7,7 +8,7 @@ export const VIRTUAL_ROUTER_BASE_PATH = 'CLERK-ROUTER/VIRTUAL'; interface VirtualRouterProps { startPath: string; preservedParams?: string[]; - onExternalNavigate?: () => any; + onExternalNavigate?: () => void; children: React.ReactNode; } @@ -17,11 +18,24 @@ export const VirtualRouter = ({ onExternalNavigate, children, }: VirtualRouterProps): JSX.Element => { + const { __internal_externalNavigationListener } = useClerk(); const [currentURL, setCurrentURL] = React.useState( new URL('/' + VIRTUAL_ROUTER_BASE_PATH + startPath, window.location.origin), ); const { urlStateParam, removeQueryParam } = useClerkModalStateParams(); + useEffect(() => { + let unsubscribe = () => {}; + if (onExternalNavigate) { + unsubscribe = __internal_externalNavigationListener(onExternalNavigate); + } + return () => { + unsubscribe(); + }; + // We are not expecting `onExternalNavigate` to change + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + if (urlStateParam.componentName) { removeQueryParam(); } @@ -44,7 +58,6 @@ export const VirtualRouter = ({ startPath={startPath} getQueryString={getQueryString} internalNavigate={internalNavigate} - onExternalNavigate={onExternalNavigate} preservedParams={preservedParams} urlStateParam={urlStateParam} > diff --git a/packages/types/src/clerk.ts b/packages/types/src/clerk.ts index ebcddefd4b4..4b65c9c1ccb 100644 --- a/packages/types/src/clerk.ts +++ b/packages/types/src/clerk.ts @@ -391,6 +391,8 @@ export interface Clerk { */ addListener: (callback: ListenerCallback) => UnsubscribeCallback; + __internal_externalNavigationListener: (callback: () => void) => UnsubscribeCallback; + /** * Set the active session and organization explicitly. * From d425e4402c8a065eac77760cce440911d591f7ad Mon Sep 17 00:00:00 2001 From: panteliselef Date: Thu, 6 Feb 2025 19:04:22 +0200 Subject: [PATCH 10/17] cleanup from previous pr --- eslint.config.mjs | 48 +---- packages/clerk-js/src/core/clerk.ts | 11 +- .../components/UserProfile/DeleteUserForm.tsx | 5 +- .../src/utils/__tests__/scopedContext.test.ts | 170 ------------------ packages/clerk-js/src/utils/scopedContext.ts | 26 --- .../src/app-router/client/ClerkProvider.tsx | 2 - .../app-router/client/useInternalNavFun.ts | 6 +- packages/nextjs/src/global.d.ts | 16 +- packages/react/src/isomorphicClerk.ts | 1 - packages/types/src/clerk.ts | 10 -- packages/types/src/snapshots.ts | 4 +- packages/types/src/utils.ts | 8 - 12 files changed, 14 insertions(+), 293 deletions(-) delete mode 100644 packages/clerk-js/src/utils/__tests__/scopedContext.test.ts delete mode 100644 packages/clerk-js/src/utils/scopedContext.ts diff --git a/eslint.config.mjs b/eslint.config.mjs index 32a7c42532b..02ce960cdbf 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -19,49 +19,6 @@ 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', @@ -70,7 +27,8 @@ const noNavigateUseClerk = { recommended: false, }, messages: { - noNavigate: 'Usage of `navigate` from `useClerk()` is not allowed. Use `useRouter() instead`.', + noNavigate: + 'Usage of `navigate` from `useClerk()` is not allowed.\nUse `useRouter().navigate` to navigate in-between flows or `setActive({ redirectUrl })`.', }, schema: [], }, @@ -403,13 +361,11 @@ export default tseslint.config([ 'custom-rules': { rules: { 'no-navigate-useClerk': noNavigateUseClerk, - 'no-setActive-redirectUrl': noSetActiveRedirectUrl, }, }, }, rules: { 'custom-rules/no-navigate-useClerk': 'error', - 'custom-rules/no-setActive-redirectUrl': 'error', }, }, { diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index a618fa17d4f..5a218743481 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -15,7 +15,6 @@ import type { AuthenticateWithGoogleOneTapParams, AuthenticateWithMetamaskParams, AuthenticateWithOKXWalletParams, - BeforeEmitCallback, Clerk as ClerkInterface, ClerkAPIError, ClerkAuthenticateWithWeb3Params, @@ -101,7 +100,6 @@ 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'; @@ -172,8 +170,6 @@ 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; @@ -915,16 +911,13 @@ export class Clerk implements ClerkInterface { 'Clerk.setActive({beforeEmit})', 'Use the `redirectUrl` property instead. Example `Clerk.setActive({redirectUrl:"/"})`', ); - } - const __beforeEmit = beforeEmit || this.__internal_setActiveContext.get()?.beforeEmit; - if (__beforeEmit) { beforeUnloadTracker?.startTracking(); this.#setTransitiveState(); - await __beforeEmit(newSession); + await beforeEmit(newSession); beforeUnloadTracker?.stopTracking(); } - if (redirectUrl && !__beforeEmit) { + if (redirectUrl && !beforeEmit) { beforeUnloadTracker?.startTracking(); this.#setTransitiveState(); diff --git a/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx b/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx index 7f2bd867dbc..5a90cfadd95 100644 --- a/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx +++ b/packages/clerk-js/src/ui/components/UserProfile/DeleteUserForm.tsx @@ -38,13 +38,10 @@ export const DeleteUserForm = withCardStateProvider((props: DeleteUserFormProps) await deleteUserWithReverification(); const redirectUrl = otherSessions.length === 0 ? afterSignOutUrl : afterMultiSessionSingleSignOutUrl; - // return __internal_setActiveContext.run({ beforeEmit }, () => { - // eslint-disable-next-line custom-rules/no-setActive-redirectUrl - return setActive({ + return await setActive({ session: null, redirectUrl, }); - // }); } catch (e) { handleError(e, [], card.setError); } diff --git a/packages/clerk-js/src/utils/__tests__/scopedContext.test.ts b/packages/clerk-js/src/utils/__tests__/scopedContext.test.ts deleted file mode 100644 index f4eb232b7e5..00000000000 --- a/packages/clerk-js/src/utils/__tests__/scopedContext.test.ts +++ /dev/null @@ -1,170 +0,0 @@ -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 deleted file mode 100644 index 38811baf1c5..00000000000 --- a/packages/clerk-js/src/utils/scopedContext.ts +++ /dev/null @@ -1,26 +0,0 @@ -type ScopedContext = { - run(context: T, fn: () => R): Promise>; - get(): T | undefined; -}; - -function createScopedContext(): ScopedContext { - let currentContext: T | undefined; - - return { - async run(context: T, fn: () => R): Promise> { - const previousContext = currentContext; - currentContext = context; - try { - return await fn(); - } finally { - currentContext = previousContext; - } - }, - get(): T | undefined { - return currentContext; - }, - }; -} - -export { createScopedContext }; -export type { ScopedContext }; diff --git a/packages/nextjs/src/app-router/client/ClerkProvider.tsx b/packages/nextjs/src/app-router/client/ClerkProvider.tsx index c12109385ca..b9a9cac154d 100644 --- a/packages/nextjs/src/app-router/client/ClerkProvider.tsx +++ b/packages/nextjs/src/app-router/client/ClerkProvider.tsx @@ -98,9 +98,7 @@ const NextClientClerkProvider = (props: NextClerkProviderProps) => { const mergedProps = mergeNextClerkPropsWithEnv({ ...props, - // @ts-expect-error Error because of the stricter types of internal `push` routerPush: push, - // @ts-expect-error Error because of the stricter types of internal `replace` routerReplace: replace, }); diff --git a/packages/nextjs/src/app-router/client/useInternalNavFun.ts b/packages/nextjs/src/app-router/client/useInternalNavFun.ts index 780b335facc..e1731b58fb0 100644 --- a/packages/nextjs/src/app-router/client/useInternalNavFun.ts +++ b/packages/nextjs/src/app-router/client/useInternalNavFun.ts @@ -15,7 +15,7 @@ export const useInternalNavFun = (props: { windowNav: typeof window.history.pushState | typeof window.history.replaceState | undefined; routerNav: AppRouterInstance['push'] | AppRouterInstance['replace']; name: string; -}): NavigationFunction => { +}) => { const { windowNav, routerNav, name } = props; const pathname = usePathname(); const [isPending, startTransition] = useTransition(); @@ -68,8 +68,8 @@ export const useInternalNavFun = (props: { } }, [pathname, isPending]); - return useCallback((to, metadata) => { - return getClerkNavigationObject(name).fun(to, metadata); + return useCallback((to: string) => { + return getClerkNavigationObject(name).fun(to); // We are not expecting name to change // eslint-disable-next-line react-hooks/exhaustive-deps }, []); diff --git a/packages/nextjs/src/global.d.ts b/packages/nextjs/src/global.d.ts index 8757bee7f05..e9672e5b939 100644 --- a/packages/nextjs/src/global.d.ts +++ b/packages/nextjs/src/global.d.ts @@ -14,24 +14,14 @@ declare namespace NodeJS { } } -type RequireMetadata any> = T extends ( - to: infer To, - metadata?: infer Metadata, -) => infer R - ? (to: To, metadata: Metadata) => R - : never; - -type NavigationFunction = - // eslint-disable-next-line @typescript-eslint/consistent-type-imports - | RequireMetadata> - // eslint-disable-next-line @typescript-eslint/consistent-type-imports - | RequireMetadata>; +// eslint-disable-next-line @typescript-eslint/consistent-type-imports +type NextClerkProviderProps = import('./types').NextClerkProviderProps; interface Window { __clerk_internal_navigations: Record< string, { - fun: NavigationFunction; + fun: NonNullable; promisesBuffer: Array<() => void> | undefined; } >; diff --git a/packages/react/src/isomorphicClerk.ts b/packages/react/src/isomorphicClerk.ts index c4830a81ca3..a71e7b52681 100644 --- a/packages/react/src/isomorphicClerk.ts +++ b/packages/react/src/isomorphicClerk.ts @@ -123,7 +123,6 @@ type IsomorphicLoadedClerk = Without< | 'client' | '__internal_getCachedResources' | '__internal_reloadInitialResources' - | '__internal_setActiveContext' > & { // TODO: Align return type and parms handleRedirectCallback: (params: HandleOAuthCallbackParams) => void; diff --git a/packages/types/src/clerk.ts b/packages/types/src/clerk.ts index 4b65c9c1ccb..a09ba213681 100644 --- a/packages/types/src/clerk.ts +++ b/packages/types/src/clerk.ts @@ -607,18 +607,8 @@ 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 & diff --git a/packages/types/src/snapshots.ts b/packages/types/src/snapshots.ts index 78c5715c21e..e32ff390912 100644 --- a/packages/types/src/snapshots.ts +++ b/packages/types/src/snapshots.ts @@ -30,7 +30,9 @@ import type { import type { OrganizationSettingsJSON } from './organizationSettings'; import type { SignInJSON } from './signIn'; import type { UserSettingsJSON } from './userSettings'; -import type { Nullable, Override } from './utils'; +import type { Nullable } from './utils'; + +type Override = Omit & U; export type SignInJSONSnapshot = Override< Nullable, diff --git a/packages/types/src/utils.ts b/packages/types/src/utils.ts index 861df403ae5..88227403440 100644 --- a/packages/types/src/utils.ts +++ b/packages/types/src/utils.ts @@ -100,11 +100,3 @@ export type Autocomplete = U | (T & Record = { [P in keyof T as Exclude]: T[P]; }; - -/** - * Overrides the type of existing properties - * const obj = { a: string, b: number } as const; - * type Value = Override - * Value contains: { a:string, b: string } - */ -export type Override = Omit & U; From 8b5c0718b76f34a7ee6d4313325a88406e31ece0 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Thu, 6 Feb 2025 19:20:46 +0200 Subject: [PATCH 11/17] fix clerk-react types --- packages/react/src/isomorphicClerk.ts | 1 + packages/types/src/clerk.ts | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/packages/react/src/isomorphicClerk.ts b/packages/react/src/isomorphicClerk.ts index a71e7b52681..fb837958b62 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_externalNavigationListener' > & { // TODO: Align return type and parms handleRedirectCallback: (params: HandleOAuthCallbackParams) => void; diff --git a/packages/types/src/clerk.ts b/packages/types/src/clerk.ts index a09ba213681..539ded85fb5 100644 --- a/packages/types/src/clerk.ts +++ b/packages/types/src/clerk.ts @@ -391,6 +391,10 @@ export interface Clerk { */ addListener: (callback: ListenerCallback) => UnsubscribeCallback; + /** + * Internal function that notifies modal UI components when a navigation event occurs, + * allowing them to close if necessary. + */ __internal_externalNavigationListener: (callback: () => void) => UnsubscribeCallback; /** From 6c68ec43be9e097458a7ee369b8d6ee36147e862 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Mon, 10 Feb 2025 13:55:11 +0200 Subject: [PATCH 12/17] address pr comments --- integration/testUtils/signInPageObject.ts | 5 +++ integration/testUtils/signUpPageObject.ts | 5 +++ integration/tests/oauth-flows.test.ts | 8 ++-- integration/tests/sign-in-flow.test.ts | 10 +++++ integration/tests/sign-in-or-up-flow.test.ts | 43 +++++++++++++++++++ integration/tests/sign-up-flow.test.ts | 33 +++++++++++++- packages/clerk-js/src/core/clerk.ts | 18 ++++---- .../clerk-js/src/ui/lazyModules/providers.tsx | 2 +- .../clerk-js/src/ui/router/VirtualRouter.tsx | 4 +- packages/types/src/clerk.ts | 6 +-- 10 files changed, 115 insertions(+), 19 deletions(-) diff --git a/integration/testUtils/signInPageObject.ts b/integration/testUtils/signInPageObject.ts index ffee57fd46a..5c000c539e4 100644 --- a/integration/testUtils/signInPageObject.ts +++ b/integration/testUtils/signInPageObject.ts @@ -24,6 +24,11 @@ export const createSignInComponentPageObject = (testArgs: TestArgs) => { waitForMounted: (selector = '.cl-signIn-root') => { return page.waitForSelector(selector, { state: 'attached' }); }, + waitForModal: (state?: 'open' | 'closed') => { + return page.waitForSelector('.cl-modalContent:has(.cl-signIn-root)', { + state: state === 'closed' ? 'detached' : 'attached', + }); + }, setIdentifier: (val: string) => { return self.getIdentifierInput().fill(val); }, diff --git a/integration/testUtils/signUpPageObject.ts b/integration/testUtils/signUpPageObject.ts index b134a7c2188..d1038c5c9bc 100644 --- a/integration/testUtils/signUpPageObject.ts +++ b/integration/testUtils/signUpPageObject.ts @@ -27,6 +27,11 @@ export const createSignUpComponentPageObject = (testArgs: TestArgs) => { waitForMounted: (selector = '.cl-signUp-root') => { return page.waitForSelector(selector, { state: 'attached' }); }, + waitForModal: (state?: 'open' | 'closed') => { + return page.waitForSelector('.cl-modalContent:has(.cl-signUp-root)', { + state: state === 'closed' ? 'detached' : 'attached', + }); + }, signUpWithOauth: (provider: string) => { return page.getByRole('button', { name: new RegExp(`continue with ${provider}`, 'gi') }); }, diff --git a/integration/tests/oauth-flows.test.ts b/integration/tests/oauth-flows.test.ts index e60b611de94..45a24e70348 100644 --- a/integration/tests/oauth-flows.test.ts +++ b/integration/tests/oauth-flows.test.ts @@ -1,10 +1,10 @@ -import { test } from '@playwright/test'; import { createClerkClient } from '@clerk/backend'; +import { test } from '@playwright/test'; import { appConfigs } from '../presets'; +import { instanceKeys } from '../presets/envs'; import type { FakeUser } from '../testUtils'; import { createTestUtils, testAgainstRunningApps } from '../testUtils'; -import { instanceKeys } from '../presets/envs'; import { createUserService } from '../testUtils/usersService'; testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('oauth flows @nextjs', ({ app }) => { @@ -78,7 +78,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('oauth flo await u.page.getByText('Sign in button (force)').click(); - await u.po.signIn.waitForMounted(); + await u.po.signIn.waitForModal(); await u.page.getByRole('button', { name: 'E2E OAuth Provider' }).click(); await u.page.getByText('Sign in to oauth-provider').waitFor(); @@ -103,7 +103,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('oauth flo await u.page.getByText('Sign up button (force)').click(); - await u.po.signUp.waitForMounted(); + await u.po.signUp.waitForModal(); await u.page.getByRole('button', { name: 'E2E OAuth Provider' }).click(); await u.page.getByText('Sign in to oauth-provider').waitFor(); diff --git a/integration/tests/sign-in-flow.test.ts b/integration/tests/sign-in-flow.test.ts index 29243df5c45..84e01e62fed 100644 --- a/integration/tests/sign-in-flow.test.ts +++ b/integration/tests/sign-in-flow.test.ts @@ -40,6 +40,16 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('sign in f await u.po.expect.toBeSignedIn(); }); + test('(modal) sign in with email and instant password', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + await u.page.goToRelative('/buttons'); + await u.page.getByText('Sign in button (force)').click(); + await u.po.signIn.waitForModal(); + await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password }); + await u.po.expect.toBeSignedIn(); + await u.po.signIn.waitForModal('closed'); + }); + test('sign in with email code', async ({ page, context }) => { const u = createTestUtils({ app, page, context }); await u.po.signIn.goTo(); diff --git a/integration/tests/sign-in-or-up-flow.test.ts b/integration/tests/sign-in-or-up-flow.test.ts index 5c67a10db33..eac74cc6379 100644 --- a/integration/tests/sign-in-or-up-flow.test.ts +++ b/integration/tests/sign-in-or-up-flow.test.ts @@ -61,6 +61,20 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withSignInOrUpFlow] })('sign- await u.po.expect.toBeSignedIn(); }); + test('(modal) sign in with email code', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + await u.page.goToRelative('/buttons'); + await u.page.getByText('Sign in button (fallback)').click(); + await u.po.signIn.waitForModal(); + await u.po.signIn.getIdentifierInput().fill(fakeUser.email); + await u.po.signIn.continue(); + await u.po.signIn.getUseAnotherMethodLink().click(); + await u.po.signIn.getAltMethodsEmailCodeButton().click(); + await u.po.signIn.enterTestOtpCode(); + await u.po.expect.toBeSignedIn(); + await u.po.signIn.waitForModal('closed'); + }); + test('sign in with phone number and password', async ({ page, context }) => { const u = createTestUtils({ app, page, context }); await u.po.signIn.goTo(); @@ -221,6 +235,35 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withSignInOrUpFlow] })('sign- await fakeUser.deleteIfExists(); }); + test('(modal) sign up with username, email, and password', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + const fakeUser = u.services.users.createFakeUser({ + fictionalEmail: true, + withPassword: true, + withUsername: true, + }); + + await u.page.goToRelative('/buttons'); + await u.page.getByText('Sign in button (fallback)').click(); + await u.po.signIn.waitForModal(); + await u.po.signIn.setIdentifier(fakeUser.username); + await u.po.signIn.continue(); + + const prefilledUsername = u.po.signUp.getUsernameInput(); + await expect(prefilledUsername).toHaveValue(fakeUser.username); + + await u.po.signUp.setEmailAddress(fakeUser.email); + await u.po.signUp.setPassword(fakeUser.password); + await u.po.signUp.continue(); + + await u.po.signUp.enterTestOtpCode(); + + await u.po.expect.toBeSignedIn(); + await u.po.signIn.waitForModal('closed'); + + await fakeUser.deleteIfExists(); + }); + test('sign up, sign out and sign in again', async ({ page, context }) => { const u = createTestUtils({ app, page, context }); const fakeUser = u.services.users.createFakeUser({ diff --git a/integration/tests/sign-up-flow.test.ts b/integration/tests/sign-up-flow.test.ts index cedba70f217..af9df350f4c 100644 --- a/integration/tests/sign-up-flow.test.ts +++ b/integration/tests/sign-up-flow.test.ts @@ -4,7 +4,7 @@ import { appConfigs } from '../presets'; import { createTestUtils, testAgainstRunningApps } from '../testUtils'; testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('sign up flow @generic @nextjs', ({ app }) => { - test.describe.configure({ mode: 'serial' }); + test.describe.configure({ mode: 'parallel' }); test.afterAll(async () => { await app.teardown(); @@ -90,6 +90,37 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('sign up f await fakeUser.deleteIfExists(); }); + test('(modal) can sign up with phone number', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + const fakeUser = u.services.users.createFakeUser({ + fictionalEmail: true, + withPhoneNumber: true, + withUsername: true, + }); + + // Open modal + await u.page.goToRelative('/buttons'); + await u.page.getByText('Sign up button (fallback)').click(); + await u.po.signUp.waitForModal(); + + // Fill in sign up form + await u.po.signUp.signUp({ + email: fakeUser.email, + phoneNumber: fakeUser.phoneNumber, + password: fakeUser.password, + }); + + // Verify email + await u.po.signUp.enterTestOtpCode(); + // Verify phone number + await u.po.signUp.enterTestOtpCode(); + + // Check if user is signed in + await u.po.expect.toBeSignedIn(); + await u.po.signUp.waitForModal('closed'); + await fakeUser.deleteIfExists(); + }); + test('sign up with first name, last name, email, phone and password', async ({ page, context }) => { const u = createTestUtils({ app, page, context }); const fakeUser = u.services.users.createFakeUser({ diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index b0820d21d92..8e70cfbbd25 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -187,7 +187,7 @@ export class Clerk implements ClerkInterface { #loaded = false; #listeners: Array<(emission: Resources) => void> = []; - #externalNavigationListeners: Array<() => void> = []; + #navigationListeners: Array<() => void> = []; #options: ClerkOptions = {}; #pageLifecycle: ReturnType | null = null; #touchThrottledUntil = 0; @@ -962,10 +962,10 @@ export class Clerk implements ClerkInterface { return unsubscribe; }; - public __internal_externalNavigationListener = (listener: () => void): UnsubscribeCallback => { - this.#externalNavigationListeners.push(listener); + public __internal_addNavigationListener = (listener: () => void): UnsubscribeCallback => { + this.#navigationListeners.push(listener); const unsubscribe = () => { - this.#externalNavigationListeners = this.#externalNavigationListeners.filter(l => l !== listener); + this.#navigationListeners = this.#navigationListeners.filter(l => l !== listener); }; return unsubscribe; }; @@ -975,9 +975,11 @@ export class Clerk implements ClerkInterface { return; } + /** + * Trigger all navigation listeners. In order for modal UI components to close. + */ setTimeout(() => { - // Notify after the navigation, in order to visit a page with the updated context set above. - this.#notifyExternalNavigationListeners(); + this.#emitNavigationListeners(); }, 0); let toURL = new URL(to, window.location.href); @@ -2056,8 +2058,8 @@ export class Clerk implements ClerkInterface { } }; - #notifyExternalNavigationListeners = (): void => { - for (const listener of this.#externalNavigationListeners) { + #emitNavigationListeners = (): void => { + for (const listener of this.#navigationListeners) { listener(); } }; diff --git a/packages/clerk-js/src/ui/lazyModules/providers.tsx b/packages/clerk-js/src/ui/lazyModules/providers.tsx index e5cd0f9fccc..3708005f8ca 100644 --- a/packages/clerk-js/src/ui/lazyModules/providers.tsx +++ b/packages/clerk-js/src/ui/lazyModules/providers.tsx @@ -84,7 +84,7 @@ type LazyModalRendererProps = React.PropsWithChildren< flowName?: FlowMetadata['flow']; startPath?: string; onClose?: ModalProps['handleClose']; - onExternalNavigate: () => any; + onExternalNavigate: () => void; modalContainerSx?: ThemableCssProp; modalContentSx?: ThemableCssProp; canCloseModal?: boolean; diff --git a/packages/clerk-js/src/ui/router/VirtualRouter.tsx b/packages/clerk-js/src/ui/router/VirtualRouter.tsx index 4b3f61f4aa9..3b152d4fd24 100644 --- a/packages/clerk-js/src/ui/router/VirtualRouter.tsx +++ b/packages/clerk-js/src/ui/router/VirtualRouter.tsx @@ -18,7 +18,7 @@ export const VirtualRouter = ({ onExternalNavigate, children, }: VirtualRouterProps): JSX.Element => { - const { __internal_externalNavigationListener } = useClerk(); + const { __internal_addNavigationListener } = useClerk(); const [currentURL, setCurrentURL] = React.useState( new URL('/' + VIRTUAL_ROUTER_BASE_PATH + startPath, window.location.origin), ); @@ -27,7 +27,7 @@ export const VirtualRouter = ({ useEffect(() => { let unsubscribe = () => {}; if (onExternalNavigate) { - unsubscribe = __internal_externalNavigationListener(onExternalNavigate); + unsubscribe = __internal_addNavigationListener(onExternalNavigate); } return () => { unsubscribe(); diff --git a/packages/types/src/clerk.ts b/packages/types/src/clerk.ts index 539ded85fb5..25cdf43bfc4 100644 --- a/packages/types/src/clerk.ts +++ b/packages/types/src/clerk.ts @@ -392,10 +392,10 @@ export interface Clerk { addListener: (callback: ListenerCallback) => UnsubscribeCallback; /** - * Internal function that notifies modal UI components when a navigation event occurs, - * allowing them to close if necessary. + * Registers and internal listener that triggers a callback each time `Clerk.navigate` is called. + * Its purpose is to notify modal UI components when a navigation event occurs, allowing them to close if necessary. */ - __internal_externalNavigationListener: (callback: () => void) => UnsubscribeCallback; + __internal_addNavigationListener: (callback: () => void) => UnsubscribeCallback; /** * Set the active session and organization explicitly. From 8d6c6ce904b003103cbd4aac0632ccb678289478 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Mon, 10 Feb 2025 13:58:58 +0200 Subject: [PATCH 13/17] address pr comments --- packages/react/src/isomorphicClerk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/isomorphicClerk.ts b/packages/react/src/isomorphicClerk.ts index fb837958b62..736570ae8a8 100644 --- a/packages/react/src/isomorphicClerk.ts +++ b/packages/react/src/isomorphicClerk.ts @@ -123,7 +123,7 @@ type IsomorphicLoadedClerk = Without< | 'client' | '__internal_getCachedResources' | '__internal_reloadInitialResources' - | '__internal_externalNavigationListener' + | '__internal_addNavigationListener' > & { // TODO: Align return type and parms handleRedirectCallback: (params: HandleOAuthCallbackParams) => void; From 917bba93aa480ae811602d4bf13ac438fa61bcc0 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Mon, 10 Feb 2025 15:30:46 +0200 Subject: [PATCH 14/17] add missing page --- .../react-vite/src/buttons/index.tsx | 37 +++++++++++++++++++ integration/templates/react-vite/src/main.tsx | 5 +++ 2 files changed, 42 insertions(+) create mode 100644 integration/templates/react-vite/src/buttons/index.tsx diff --git a/integration/templates/react-vite/src/buttons/index.tsx b/integration/templates/react-vite/src/buttons/index.tsx new file mode 100644 index 00000000000..5aa32d433cf --- /dev/null +++ b/integration/templates/react-vite/src/buttons/index.tsx @@ -0,0 +1,37 @@ +import { SignInButton, SignUpButton } from '@clerk/clerk-react'; + +export default function Home() { + return ( +
+ + Sign in button (force) + + + + Sign in button (fallback) + + + + Sign up button (force) + + + + Sign up button (fallback) + +
+ ); +} diff --git a/integration/templates/react-vite/src/main.tsx b/integration/templates/react-vite/src/main.tsx index 15d8b3c56f4..20f64bdb9e4 100644 --- a/integration/templates/react-vite/src/main.tsx +++ b/integration/templates/react-vite/src/main.tsx @@ -18,6 +18,7 @@ import OrganizationProfile from './organization-profile'; import OrganizationList from './organization-list'; import CreateOrganization from './create-organization'; import OrganizationSwitcher from './organization-switcher'; +import Buttons from './buttons'; const Root = () => { const navigate = useNavigate(); @@ -68,6 +69,10 @@ const router = createBrowserRouter([ path: '/protected', element: , }, + { + path: '/buttons', + element: , + }, { path: '/custom-user-profile/*', element: , From e364340e0ef0f7153936de1dffb832e3a85a9a84 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Tue, 11 Feb 2025 09:58:04 +0200 Subject: [PATCH 15/17] Update packages/types/src/clerk.ts Co-authored-by: Dylan Staley <88163+dstaley@users.noreply.github.com> --- packages/types/src/clerk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/types/src/clerk.ts b/packages/types/src/clerk.ts index 25cdf43bfc4..668c8985b9f 100644 --- a/packages/types/src/clerk.ts +++ b/packages/types/src/clerk.ts @@ -392,7 +392,7 @@ export interface Clerk { addListener: (callback: ListenerCallback) => UnsubscribeCallback; /** - * Registers and internal listener that triggers a callback each time `Clerk.navigate` is called. + * Registers an internal listener that triggers a callback each time `Clerk.navigate` is called. * Its purpose is to notify modal UI components when a navigation event occurs, allowing them to close if necessary. */ __internal_addNavigationListener: (callback: () => void) => UnsubscribeCallback; From ef0e05850a840097b47606d9766d5cf8b3d1b133 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Wed, 12 Feb 2025 10:19:34 +0200 Subject: [PATCH 16/17] add changesets --- .changeset/beige-colts-fold.md | 5 +++++ .changeset/eighty-pigs-sniff.md | 5 +++++ .changeset/strange-eels-poke.md | 7 ------- .changeset/violet-fishes-provide.md | 5 +++++ 4 files changed, 15 insertions(+), 7 deletions(-) create mode 100644 .changeset/beige-colts-fold.md create mode 100644 .changeset/eighty-pigs-sniff.md delete mode 100644 .changeset/strange-eels-poke.md create mode 100644 .changeset/violet-fishes-provide.md diff --git a/.changeset/beige-colts-fold.md b/.changeset/beige-colts-fold.md new file mode 100644 index 00000000000..195057ddbd2 --- /dev/null +++ b/.changeset/beige-colts-fold.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-react': patch +--- + +Exclude `__internal_addNavigationListener` from `IsomorphicClerk`. diff --git a/.changeset/eighty-pigs-sniff.md b/.changeset/eighty-pigs-sniff.md new file mode 100644 index 00000000000..282fa523f34 --- /dev/null +++ b/.changeset/eighty-pigs-sniff.md @@ -0,0 +1,5 @@ +--- +'@clerk/types': minor +--- + +Introduce `__internal_addNavigationListener` method the `Clerk` singleton. diff --git a/.changeset/strange-eels-poke.md b/.changeset/strange-eels-poke.md deleted file mode 100644 index 07f74e80749..00000000000 --- a/.changeset/strange-eels-poke.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -'@clerk/clerk-js': patch -'@clerk/clerk-react': patch -'@clerk/types': patch ---- - -TODO diff --git a/.changeset/violet-fishes-provide.md b/.changeset/violet-fishes-provide.md new file mode 100644 index 00000000000..e12835b555c --- /dev/null +++ b/.changeset/violet-fishes-provide.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': patch +--- + +Bug fix: Close modals when calling `Clerk.navigate()` or `Clerk.setActive({redirectUrl})`. From 8c66b9876a2061bdb21e088682dcb77a14d1bb56 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Wed, 12 Feb 2025 13:30:59 +0200 Subject: [PATCH 17/17] remove `onExternalNavigate` from mountedBlankCaptchaModal --- packages/clerk-js/src/ui/Components.tsx | 4 +++- packages/clerk-js/src/ui/lazyModules/providers.tsx | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/clerk-js/src/ui/Components.tsx b/packages/clerk-js/src/ui/Components.tsx index eac3243a9b5..bf6851fd3a4 100644 --- a/packages/clerk-js/src/ui/Components.tsx +++ b/packages/clerk-js/src/ui/Components.tsx @@ -463,13 +463,15 @@ const Components = (props: ComponentsProps) => { ); const mountedBlankCaptchaModal = ( + /** + * Captcha modal should not close on `Clerk.navigate()`, hence we are not passing `onExternalNavigate`. + */ componentsControls.closeModal('blankCaptcha')} - onExternalNavigate={() => componentsControls.closeModal('blankCaptcha')} startPath={buildVirtualRouterUrl({ base: '/blank-captcha', path: urlStateParam?.path })} componentName={'BlankCaptchaModal'} canCloseModal={false} diff --git a/packages/clerk-js/src/ui/lazyModules/providers.tsx b/packages/clerk-js/src/ui/lazyModules/providers.tsx index 3708005f8ca..61162e7ee32 100644 --- a/packages/clerk-js/src/ui/lazyModules/providers.tsx +++ b/packages/clerk-js/src/ui/lazyModules/providers.tsx @@ -84,7 +84,7 @@ type LazyModalRendererProps = React.PropsWithChildren< flowName?: FlowMetadata['flow']; startPath?: string; onClose?: ModalProps['handleClose']; - onExternalNavigate: () => void; + onExternalNavigate?: () => void; modalContainerSx?: ThemableCssProp; modalContentSx?: ThemableCssProp; canCloseModal?: boolean;