From 5470ea6e58bb7d80c02efa93313ee671a84a9a25 Mon Sep 17 00:00:00 2001 From: Nikos Douvlis Date: Thu, 29 Feb 2024 00:11:59 +0200 Subject: [PATCH 1/2] fix(nextjs,clerk-js): Make useAwaitableNavigate handle navigations between pages reliably We need to use window to store the reference to the buffer, as ClerkProvider might be unmounted and remounted during navigations. If we use a ref, it will be reset when ClerkProvider is unmounted --- .changeset/silly-tips-cry.md | 6 ++ .changeset/unlucky-crabs-grab.md | 5 ++ packages/clerk-js/src/core/clerk.ts | 6 +- packages/clerk-js/src/globals.d.ts | 4 +- .../src/app-router/client/ClerkProvider.tsx | 10 ++-- .../client/invalidateCacheAction.ts | 25 +++++++++ .../app-router/client/useAwaitableNavigate.ts | 56 ++++++++++++------- packages/nextjs/src/global.d.ts | 4 +- 8 files changed, 84 insertions(+), 32 deletions(-) create mode 100644 .changeset/silly-tips-cry.md create mode 100644 .changeset/unlucky-crabs-grab.md create mode 100644 packages/nextjs/src/app-router/client/invalidateCacheAction.ts diff --git a/.changeset/silly-tips-cry.md b/.changeset/silly-tips-cry.md new file mode 100644 index 00000000000..d4ed4ffd849 --- /dev/null +++ b/.changeset/silly-tips-cry.md @@ -0,0 +1,6 @@ +--- +'@clerk/clerk-js': patch +'@clerk/nextjs': patch +--- + +Make useAwaitableNavigate handle navigations between pages reliably diff --git a/.changeset/unlucky-crabs-grab.md b/.changeset/unlucky-crabs-grab.md new file mode 100644 index 00000000000..ee110f7d190 --- /dev/null +++ b/.changeset/unlucky-crabs-grab.md @@ -0,0 +1,5 @@ +--- +'@clerk/nextjs': patch +--- + +Make useAwaitableNavigate handle navigations between pages reliably diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 81c38721bd6..47c5c9ef0f0 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -605,7 +605,7 @@ export default class Clerk implements ClerkInterface { ); } - type SetActiveHook = () => void; + type SetActiveHook = () => void | Promise; const onBeforeSetActive: SetActiveHook = typeof window !== 'undefined' && typeof window.__unstable__onBeforeSetActive === 'function' ? window.__unstable__onBeforeSetActive @@ -640,7 +640,7 @@ export default class Clerk implements ClerkInterface { this.#broadcastSignOutEvent(); } - onBeforeSetActive(); + await onBeforeSetActive(); //1. setLastActiveSession to passed user session (add a param). // Note that this will also update the session's active organization @@ -672,7 +672,7 @@ export default class Clerk implements ClerkInterface { this.#setAccessors(newSession); this.#emit(); - onAfterSetActive(); + await onAfterSetActive(); this.#resetComponentsState(); }; diff --git a/packages/clerk-js/src/globals.d.ts b/packages/clerk-js/src/globals.d.ts index a3a7c377570..26ef5a8a3c2 100644 --- a/packages/clerk-js/src/globals.d.ts +++ b/packages/clerk-js/src/globals.d.ts @@ -4,8 +4,8 @@ declare global { const __PKG_VERSION__: string; interface Window { - __unstable__onBeforeSetActive: () => void; - __unstable__onAfterSetActive: () => void; + __unstable__onBeforeSetActive: () => Promise | void; + __unstable__onAfterSetActive: () => Promise | void; } } diff --git a/packages/nextjs/src/app-router/client/ClerkProvider.tsx b/packages/nextjs/src/app-router/client/ClerkProvider.tsx index 8fe3ca193ab..aaa5edfd3b7 100644 --- a/packages/nextjs/src/app-router/client/ClerkProvider.tsx +++ b/packages/nextjs/src/app-router/client/ClerkProvider.tsx @@ -7,6 +7,7 @@ import { ClerkNextOptionsProvider } from '../../client-boundary/NextOptionsConte import { useSafeLayoutEffect } from '../../client-boundary/useSafeLayoutEffect'; import type { NextClerkProviderProps } from '../../types'; import { mergeNextClerkPropsWithEnv } from '../../utils/mergeNextClerkPropsWithEnv'; +import { invalidateCacheAction } from './invalidateCacheAction'; import { useAwaitableNavigate } from './useAwaitableNavigate'; declare global { @@ -23,14 +24,13 @@ export const ClientClerkProvider = (props: NextClerkProviderProps) => { useSafeLayoutEffect(() => { window.__unstable__onBeforeSetActive = () => { - if (__unstable_invokeMiddlewareOnAuthStateChange) { - router.refresh(); - router.push(window.location.href); - } + return invalidateCacheAction(); }; window.__unstable__onAfterSetActive = () => { - router.refresh(); + if (__unstable_invokeMiddlewareOnAuthStateChange) { + return router.refresh(); + } }; }, []); diff --git a/packages/nextjs/src/app-router/client/invalidateCacheAction.ts b/packages/nextjs/src/app-router/client/invalidateCacheAction.ts new file mode 100644 index 00000000000..20da4218107 --- /dev/null +++ b/packages/nextjs/src/app-router/client/invalidateCacheAction.ts @@ -0,0 +1,25 @@ +'use server'; +import { cookies } from 'next/headers'; + +/** + * This function is used as a workaround to clear the client-side routing cache + * AppRouter uses before we fire an auth state update. + * We need to invalidate the cache in case the user is navigating to a page that + * was previously cached using the auth state that was active at the time. + * + * For example, if we did not invalidate the flow, the following scenario would be broken: + * - The middleware is configured in such a way that it redirects you back to the same page if a certain condition is true (eg, you need to pick an org) + * - The user has a component in the page + * - The UB is mounted with afterSignOutUrl=/ + * - The user clicks the Link. A nav to / happens, a 307 to the current page is returned so a navigation does not take place. The / navigation is now cached as a 307 to the current page + * - The user clicks sign out + * - We call router.refresh() + * - We navigate to / but its cached and instead, we 'redirect' to the current page + * + * For more information on cache invalidation, see: + * https://nextjs.org/docs/app/building-your-application/caching#invalidation-1 + */ +export const invalidateCacheAction = async () => { + cookies().delete(`__clerk_random_cookie_${Date.now()}`); + return Promise.resolve(); +}; diff --git a/packages/nextjs/src/app-router/client/useAwaitableNavigate.ts b/packages/nextjs/src/app-router/client/useAwaitableNavigate.ts index ea0381c1933..b40c553f529 100644 --- a/packages/nextjs/src/app-router/client/useAwaitableNavigate.ts +++ b/packages/nextjs/src/app-router/client/useAwaitableNavigate.ts @@ -1,46 +1,62 @@ 'use client'; -import { useRouter } from 'next/navigation'; -import { useEffect, useRef, useTransition } from 'react'; +import { usePathname, useRouter } from 'next/navigation'; +import { useCallback, useEffect, useTransition } from 'react'; -type NavigateFunction = ReturnType['push']; +declare global { + interface Window { + __clerk_internal_navPromisesBuffer: Array<() => void> | undefined; + __clerk_internal_navFun: (to: string) => Promise; + } +} /** * Creates an "awaitable" navigation function that will do its best effort to wait for Next.js to finish its route transition. - * * This is accomplished by wrapping the call to `router.push` in `startTransition()`, which should rely on React to coordinate the pending state. We key off of * `isPending` to flush the stored promises and ensure the navigates "resolve". */ export const useAwaitableNavigate = () => { // eslint-disable-next-line @typescript-eslint/unbound-method const { push } = useRouter(); + const pathname = usePathname(); const [isPending, startTransition] = useTransition(); - const clerkNavRef = useRef<(...args: Parameters) => Promise>(); - const clerkNavPromiseBuffer = useRef<(() => void)[]>([]); - // Set the navigation function reference only once - if (!clerkNavRef.current) { - clerkNavRef.current = (to, opts) => { + if (typeof window !== 'undefined') { + window.__clerk_internal_navFun = to => { return new Promise(res => { - clerkNavPromiseBuffer.current.push(res); + if (!window.__clerk_internal_navPromisesBuffer) { + // We need to use window to store the reference to the buffer, + // as ClerkProvider might be unmounted and remounted during navigations + // If we use a ref, it will be reset when ClerkProvider is unmounted + window.__clerk_internal_navPromisesBuffer = []; + } + window.__clerk_internal_navPromisesBuffer.push(res); startTransition(() => { - push(to, opts); + push(to); }); }); }; } - // Handle flushing the promise buffer when pending is false. If pending is false and there are promises in the buffer we should be able to safely flush them. + const flushPromises = () => { + window.__clerk_internal_navPromisesBuffer?.forEach(resolve => resolve()); + window.__clerk_internal_navPromisesBuffer = []; + }; + + // Flush any pending promises on mount/unmount useEffect(() => { - if (isPending) { - return; - } + flushPromises(); + return flushPromises; + }, []); - if (clerkNavPromiseBuffer?.current?.length) { - clerkNavPromiseBuffer.current.forEach(resolve => resolve()); + // Handle flushing the promise buffer when a navigation happens + useEffect(() => { + if (!isPending) { + flushPromises(); } - clerkNavPromiseBuffer.current = []; - }, [isPending]); + }, [pathname, isPending]); - return clerkNavRef.current; + return useCallback((to: string) => { + return window.__clerk_internal_navFun(to); + }, []); }; diff --git a/packages/nextjs/src/global.d.ts b/packages/nextjs/src/global.d.ts index f239fdc121f..7163d53529e 100644 --- a/packages/nextjs/src/global.d.ts +++ b/packages/nextjs/src/global.d.ts @@ -1,7 +1,7 @@ declare global { interface Window { - __unstable__onBeforeSetActive: () => void; - __unstable__onAfterSetActive: () => void; + __unstable__onBeforeSetActive: () => void | Promise; + __unstable__onAfterSetActive: () => void | Promise; } } From 86eb8017a4349d98c0f84ce080e6f893248a09ff Mon Sep 17 00:00:00 2001 From: Nikos Douvlis Date: Fri, 1 Mar 2024 02:28:24 +0200 Subject: [PATCH 2/2] fix(nextjs,clerk-js): Make useAwaitableNavigate handle navigations between pages reliably We need to use window to store the reference to the buffer, as ClerkProvider might be unmounted and remounted during navigations. If we use a ref, it will be reset when ClerkProvider is unmounted --- .../src/app-router/client/ClerkProvider.tsx | 37 +++++++++++++++++-- .../client/invalidateCacheAction.ts | 25 ------------- 2 files changed, 34 insertions(+), 28 deletions(-) delete mode 100644 packages/nextjs/src/app-router/client/invalidateCacheAction.ts diff --git a/packages/nextjs/src/app-router/client/ClerkProvider.tsx b/packages/nextjs/src/app-router/client/ClerkProvider.tsx index aaa5edfd3b7..0e361ca508d 100644 --- a/packages/nextjs/src/app-router/client/ClerkProvider.tsx +++ b/packages/nextjs/src/app-router/client/ClerkProvider.tsx @@ -1,19 +1,19 @@ 'use client'; import { ClerkProvider as ReactClerkProvider } from '@clerk/clerk-react'; import { useRouter } from 'next/navigation'; -import React from 'react'; +import React, { useEffect, useTransition } from 'react'; import { ClerkNextOptionsProvider } from '../../client-boundary/NextOptionsContext'; import { useSafeLayoutEffect } from '../../client-boundary/useSafeLayoutEffect'; import type { NextClerkProviderProps } from '../../types'; import { mergeNextClerkPropsWithEnv } from '../../utils/mergeNextClerkPropsWithEnv'; -import { invalidateCacheAction } from './invalidateCacheAction'; import { useAwaitableNavigate } from './useAwaitableNavigate'; declare global { export interface Window { __clerk_nav_await: Array<(value: void) => void>; __clerk_nav: (to: string) => Promise; + __clerk_internal_invalidateCachePromise: () => void | undefined; } } @@ -21,10 +21,41 @@ export const ClientClerkProvider = (props: NextClerkProviderProps) => { const { __unstable_invokeMiddlewareOnAuthStateChange = true } = props; const router = useRouter(); const navigate = useAwaitableNavigate(); + const [isPending, startTransition] = useTransition(); + + useEffect(() => { + if (!isPending) { + window.__clerk_internal_invalidateCachePromise?.(); + } + }, [isPending]); useSafeLayoutEffect(() => { window.__unstable__onBeforeSetActive = () => { - return invalidateCacheAction(); + /** + * We need to invalidate the cache in case the user is navigating to a page that + * was previously cached using the auth state that was active at the time. + * + * We also need to await for the invalidation to happen before we navigate, + * otherwise the navigation will use the cached page. + * + * For example, if we did not invalidate the flow, the following scenario would be broken: + * - The middleware is configured in such a way that it redirects you back to the same page if a certain condition is true (eg, you need to pick an org) + * - The user has a component in the page + * - The UB is mounted with afterSignOutUrl=/ + * - The user clicks the Link. A nav to / happens, a 307 to the current page is returned so a navigation does not take place. The / navigation is now cached as a 307 to the current page + * - The user clicks sign out + * - We call router.refresh() + * - We navigate to / but its cached and instead, we 'redirect' to the current page + * + * For more information on cache invalidation, see: + * https://nextjs.org/docs/app/building-your-application/caching#invalidation-1 + */ + return new Promise(res => { + window.__clerk_internal_invalidateCachePromise = res; + startTransition(() => { + router.refresh(); + }); + }); }; window.__unstable__onAfterSetActive = () => { diff --git a/packages/nextjs/src/app-router/client/invalidateCacheAction.ts b/packages/nextjs/src/app-router/client/invalidateCacheAction.ts deleted file mode 100644 index 20da4218107..00000000000 --- a/packages/nextjs/src/app-router/client/invalidateCacheAction.ts +++ /dev/null @@ -1,25 +0,0 @@ -'use server'; -import { cookies } from 'next/headers'; - -/** - * This function is used as a workaround to clear the client-side routing cache - * AppRouter uses before we fire an auth state update. - * We need to invalidate the cache in case the user is navigating to a page that - * was previously cached using the auth state that was active at the time. - * - * For example, if we did not invalidate the flow, the following scenario would be broken: - * - The middleware is configured in such a way that it redirects you back to the same page if a certain condition is true (eg, you need to pick an org) - * - The user has a component in the page - * - The UB is mounted with afterSignOutUrl=/ - * - The user clicks the Link. A nav to / happens, a 307 to the current page is returned so a navigation does not take place. The / navigation is now cached as a 307 to the current page - * - The user clicks sign out - * - We call router.refresh() - * - We navigate to / but its cached and instead, we 'redirect' to the current page - * - * For more information on cache invalidation, see: - * https://nextjs.org/docs/app/building-your-application/caching#invalidation-1 - */ -export const invalidateCacheAction = async () => { - cookies().delete(`__clerk_random_cookie_${Date.now()}`); - return Promise.resolve(); -};