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 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/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/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 4eaddfe4fb4..655719377d4 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,17 +910,20 @@ 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(); } - 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 5a90cfadd95..8d1cacc95ed 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 { 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', '', { @@ -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/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); 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 new file mode 100644 index 00000000000..38811baf1c5 --- /dev/null +++ b/packages/clerk-js/src/utils/scopedContext.ts @@ -0,0 +1,26 @@ +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/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; 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 &