From 816cd1275fe795aed50fba09e54932e327b93981 Mon Sep 17 00:00:00 2001 From: Nikos Douvlis Date: Fri, 1 Mar 2024 00:32:56 +0200 Subject: [PATCH] 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 ++ packages/clerk-js/src/core/clerk.ts | 6 +- packages/clerk-js/src/globals.d.ts | 4 +- .../src/app-router/client/ClerkProvider.tsx | 43 ++++++++++++-- .../app-router/client/useAwaitableNavigate.ts | 57 ++++++++++++------- packages/nextjs/src/global.d.ts | 4 +- 6 files changed, 87 insertions(+), 33 deletions(-) create mode 100644 .changeset/silly-tips-cry.md 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/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 2d0058cdb21..352fca69eec 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -618,7 +618,7 @@ export 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 @@ -653,7 +653,7 @@ export 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 @@ -685,7 +685,7 @@ export 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 bbe67d0c557..37927fcbc2e 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, children } = 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 6508e9d1e6d..0fc080d8c87 100644 --- a/packages/nextjs/src/app-router/client/useAwaitableNavigate.ts +++ b/packages/nextjs/src/app-router/client/useAwaitableNavigate.ts @@ -1,27 +1,37 @@ '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'; import type { NextClerkProviderProps } from '../../types'; +declare global { + interface Window { + __clerk_internal_navPromisesBuffer: Array<() => void> | undefined; + __clerk_internal_navFun: NonNullable; + } +} + /** * 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 = () => { const router = useRouter(); + const pathname = usePathname(); const [isPending, startTransition] = useTransition(); - const clerkNavRef = useRef>(); - 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, opts) => { 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(() => { // If the navigation is internal, we should use the history API to navigate // as this is the way to perform a shallow navigation in Next.js App Router @@ -41,21 +51,28 @@ export const useAwaitableNavigate = () => { } }); }); - }) as NextClerkProviderProps['routerPush']; + }; } - // 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(); } + }, [pathname, isPending]); - clerkNavPromiseBuffer.current = []; - }, [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 829fd7122d8..ea622434dd1 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; } }