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..0e361ca508d 100644 --- a/packages/nextjs/src/app-router/client/ClerkProvider.tsx +++ b/packages/nextjs/src/app-router/client/ClerkProvider.tsx @@ -1,7 +1,7 @@ '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'; @@ -13,6 +13,7 @@ declare global { export interface Window { __clerk_nav_await: Array<(value: void) => void>; __clerk_nav: (to: string) => Promise; + __clerk_internal_invalidateCachePromise: () => void | undefined; } } @@ -20,17 +21,47 @@ 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 = () => { - if (__unstable_invokeMiddlewareOnAuthStateChange) { - router.refresh(); - router.push(window.location.href); - } + /** + * 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 = () => { - router.refresh(); + if (__unstable_invokeMiddlewareOnAuthStateChange) { + return router.refresh(); + } }; }, []); 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; } }