diff --git a/.changeset/olive-files-listen.md b/.changeset/olive-files-listen.md new file mode 100644 index 00000000000..16c184196a4 --- /dev/null +++ b/.changeset/olive-files-listen.md @@ -0,0 +1,7 @@ +--- +'@clerk/clerk-js': patch +'@clerk/nextjs': patch +'@clerk/types': patch +--- + +Prevent Clerk component flickering when mounted in a Next.js app using App Router diff --git a/packages/clerk-js/src/core/clerk.redirects.test.ts b/packages/clerk-js/src/core/clerk.redirects.test.ts index c79f9be886f..b225f758959 100644 --- a/packages/clerk-js/src/core/clerk.redirects.test.ts +++ b/packages/clerk-js/src/core/clerk.redirects.test.ts @@ -126,56 +126,56 @@ describe('Clerk singleton - Redirects', () => { await clerkForProductionInstance.redirectToSignIn({ redirectUrl: 'https://www.example.com/' }); await clerkForDevelopmentInstance.redirectToSignIn({ redirectUrl: 'https://www.example.com/' }); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/sign-in#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); - expect(mockNavigate).toHaveBeenNthCalledWith(2, '/sign-in#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-in#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); + expect(mockNavigate.mock.calls[1][0]).toBe('/sign-in#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); }); it('redirects to signUpUrl', async () => { await clerkForProductionInstance.redirectToSignUp({ redirectUrl: 'https://www.example.com/' }); await clerkForDevelopmentInstance.redirectToSignUp({ redirectUrl: 'https://www.example.com/' }); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/sign-up#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); - expect(mockNavigate).toHaveBeenNthCalledWith(2, '/sign-up#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-up#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); + expect(mockNavigate.mock.calls[1][0]).toBe('/sign-up#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); }); it('redirects to userProfileUrl', async () => { await clerkForProductionInstance.redirectToUserProfile(); await clerkForDevelopmentInstance.redirectToUserProfile(); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/user-profile'); - expect(mockNavigate).toHaveBeenNthCalledWith(2, '/user-profile'); + expect(mockNavigate.mock.calls[0][0]).toBe('/user-profile'); + expect(mockNavigate.mock.calls[1][0]).toBe('/user-profile'); }); it('redirects to afterSignUp', async () => { await clerkForProductionInstance.redirectToAfterSignUp(); await clerkForDevelopmentInstance.redirectToAfterSignUp(); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/'); - expect(mockNavigate).toHaveBeenNthCalledWith(2, '/'); + expect(mockNavigate.mock.calls[0][0]).toBe('/'); + expect(mockNavigate.mock.calls[1][0]).toBe('/'); }); it('redirects to afterSignIn', async () => { await clerkForProductionInstance.redirectToAfterSignIn(); await clerkForDevelopmentInstance.redirectToAfterSignIn(); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/'); - expect(mockNavigate).toHaveBeenNthCalledWith(2, '/'); + expect(mockNavigate.mock.calls[0][0]).toBe('/'); + expect(mockNavigate.mock.calls[1][0]).toBe('/'); }); it('redirects to create organization', async () => { await clerkForProductionInstance.redirectToCreateOrganization(); await clerkForDevelopmentInstance.redirectToCreateOrganization(); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/create-organization'); - expect(mockNavigate).toHaveBeenNthCalledWith(2, '/create-organization'); + expect(mockNavigate.mock.calls[0][0]).toBe('/create-organization'); + expect(mockNavigate.mock.calls[1][0]).toBe('/create-organization'); }); it('redirects to organization profile', async () => { await clerkForProductionInstance.redirectToOrganizationProfile(); await clerkForDevelopmentInstance.redirectToOrganizationProfile(); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/organization-profile'); - expect(mockNavigate).toHaveBeenNthCalledWith(2, '/organization-profile'); + expect(mockNavigate.mock.calls[0][0]).toBe('/organization-profile'); + expect(mockNavigate.mock.calls[1][0]).toBe('/organization-profile'); }); }); diff --git a/packages/clerk-js/src/core/clerk.test.ts b/packages/clerk-js/src/core/clerk.test.ts index f0e3ea1a2a3..3804f0197c9 100644 --- a/packages/clerk-js/src/core/clerk.test.ts +++ b/packages/clerk-js/src/core/clerk.test.ts @@ -571,7 +571,7 @@ describe('Clerk singleton', () => { const res = sut.navigate(toUrl); expect(res.then).toBeDefined(); expect(mockHref).not.toHaveBeenCalled(); - expect(mockNavigate).toHaveBeenCalledWith('/path#hash'); + expect(mockNavigate.mock.calls[0][0]).toBe('/path#hash'); expect(logSpy).not.toBeCalled(); }); @@ -591,7 +591,7 @@ describe('Clerk singleton', () => { const res = sut.navigate(toUrl); expect(res.then).toBeDefined(); expect(mockHref).not.toHaveBeenCalled(); - expect(mockNavigate).toHaveBeenCalledWith('/path#hash'); + expect(mockNavigate.mock.calls[0][0]).toBe('/path#hash'); expect(logSpy).toBeCalledTimes(1); expect(logSpy).toBeCalledWith(`Clerk is navigating to: ${toUrl}`); @@ -721,7 +721,7 @@ describe('Clerk singleton', () => { await waitFor(() => { expect(mockSignUpCreate).toHaveBeenCalledWith({ transfer: true }); - expect(mockNavigate).toHaveBeenCalledWith('/sign-up#/continue'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-up#/continue'); }); }); @@ -934,7 +934,7 @@ describe('Clerk singleton', () => { await sut.handleRedirectCallback(); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/sign-in#/factor-two'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-in#/factor-two'); }); }); @@ -977,7 +977,7 @@ describe('Clerk singleton', () => { }); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/custom-2fa'); + expect(mockNavigate.mock.calls[0][0]).toBe('/custom-2fa'); }); }); @@ -1032,7 +1032,7 @@ describe('Clerk singleton', () => { }); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/custom-sign-in'); + expect(mockNavigate.mock.calls[0][0]).toBe('/custom-sign-in'); }); }); @@ -1088,7 +1088,7 @@ describe('Clerk singleton', () => { } as any); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/custom-sign-in'); + expect(mockNavigate.mock.calls[0][0]).toBe('/custom-sign-in'); }); }); @@ -1136,7 +1136,7 @@ describe('Clerk singleton', () => { await sut.handleRedirectCallback(); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/sign-up'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-up'); }); }); @@ -1184,7 +1184,7 @@ describe('Clerk singleton', () => { await sut.handleRedirectCallback(); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/sign-up'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-up'); }); }); @@ -1226,7 +1226,7 @@ describe('Clerk singleton', () => { await sut.handleRedirectCallback(); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/sign-up#/continue'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-up#/continue'); }); }); @@ -1273,7 +1273,7 @@ describe('Clerk singleton', () => { await sut.handleRedirectCallback(); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/sign-up#/verify-email-address'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-up#/verify-email-address'); }); }); @@ -1305,7 +1305,7 @@ describe('Clerk singleton', () => { await sut.handleRedirectCallback(); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/sign-in#/factor-one'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-in#/factor-one'); }); }); @@ -1353,7 +1353,7 @@ describe('Clerk singleton', () => { await sut.handleRedirectCallback(); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/sign-in#/factor-one'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-in#/factor-one'); }); }); @@ -1412,7 +1412,7 @@ describe('Clerk singleton', () => { await sut.handleRedirectCallback(); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/sign-up'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-up'); }); }); @@ -1463,7 +1463,7 @@ describe('Clerk singleton', () => { await sut.handleRedirectCallback(); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/sign-in'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-in'); }); }); @@ -1501,7 +1501,7 @@ describe('Clerk singleton', () => { await sut.handleRedirectCallback(); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/sign-in#/reset-password'); + expect(mockNavigate.mock.calls[0][0]).toBe('/sign-in#/reset-password'); }); }); }); @@ -1568,7 +1568,7 @@ describe('Clerk singleton', () => { await waitFor(() => { expect(mockSetActive).not.toHaveBeenCalled(); - expect(mockNavigate).toHaveBeenCalledWith(redirectUrl); + expect(mockNavigate.mock.calls[0][0]).toBe(redirectUrl); }); }); @@ -1628,7 +1628,7 @@ describe('Clerk singleton', () => { await waitFor(() => { expect(mockSetActive).not.toHaveBeenCalled(); - expect(mockNavigate).toHaveBeenCalledWith(redirectUrl); + expect(mockNavigate.mock.calls[0][0]).toBe(redirectUrl); }); }); diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 30531c3aaf6..2d0058cdb21 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -726,8 +726,9 @@ export class Clerk implements ClerkInterface { return; } + const metadata = options?.metadata ? { __internal_metadata: options?.metadata } : undefined; // React router only wants the path, search or hash portion. - return await customNavigate(stripOrigin(toURL)); + return await customNavigate(stripOrigin(toURL), metadata); }; public buildUrlWithAuth(to: string): string { diff --git a/packages/clerk-js/src/ui/router/BaseRouter.tsx b/packages/clerk-js/src/ui/router/BaseRouter.tsx index 2a76d9e6072..aad77cf2964 100644 --- a/packages/clerk-js/src/ui/router/BaseRouter.tsx +++ b/packages/clerk-js/src/ui/router/BaseRouter.tsx @@ -1,4 +1,5 @@ import { useClerk } from '@clerk/shared/react'; +import type { NavigateOptions } from '@clerk/types'; import qs from 'qs'; import React from 'react'; @@ -14,7 +15,7 @@ interface BaseRouterProps { startPath: string; getPath: () => string; getQueryString: () => string; - internalNavigate: (toURL: URL) => Promise | any; + internalNavigate: (toURL: URL, options?: NavigateOptions) => Promise | any; onExternalNavigate?: () => any; refreshEvents?: Array; preservedParams?: string[]; @@ -114,7 +115,7 @@ export const BaseRouter = ({ }); toURL.search = qs.stringify(toQueryParams); } - const internalNavRes = await internalNavigate(toURL); + const internalNavRes = await internalNavigate(toURL, { metadata: { navigationType: 'internal' } }); setRouteParts({ path: toURL.pathname, queryString: toURL.search }); return internalNavRes; }; diff --git a/packages/nextjs/src/app-router/client/useAwaitableNavigate.ts b/packages/nextjs/src/app-router/client/useAwaitableNavigate.ts index cbb9fbbf0a9..6508e9d1e6d 100644 --- a/packages/nextjs/src/app-router/client/useAwaitableNavigate.ts +++ b/packages/nextjs/src/app-router/client/useAwaitableNavigate.ts @@ -3,7 +3,7 @@ import { useRouter } from 'next/navigation'; import { useEffect, useRef, useTransition } from 'react'; -type NavigateFunction = ReturnType['push']; +import type { NextClerkProviderProps } from '../../types'; /** * Creates an "awaitable" navigation function that will do its best effort to wait for Next.js to finish its route transition. @@ -12,31 +12,48 @@ type NavigateFunction = ReturnType['push']; * `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 router = useRouter(); const [isPending, startTransition] = useTransition(); - const clerkNavRef = useRef<(...args: Parameters) => Promise>(); + const clerkNavRef = useRef>(); const clerkNavPromiseBuffer = useRef<(() => void)[]>([]); // Set the navigation function reference only once if (!clerkNavRef.current) { - clerkNavRef.current = (to, opts) => { + clerkNavRef.current = ((to, opts) => { return new Promise(res => { clerkNavPromiseBuffer.current.push(res); startTransition(() => { - push(to, opts); + // 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 + // without unmounting/remounting the page or fetching data from the server. + if (opts?.__internal_metadata?.navigationType === 'internal') { + // In 14.1.0, useSearchParams becomes reactive to shallow updates, + // but only if passing `null` as the history state. + // Older versions need to maintain the history state for push to work, + // without affecting how the Next router works. + const state = ((window as any).next?.version ?? '') < '14.1.0' ? history.state : null; + window.history.pushState(state, '', to); + } else { + // If the navigation is external (usually when navigating away from the component but still within the app), + // we should use the Next.js router to navigate as it will handle updating the URL and also + // fetching the new page if necessary. + router.push(to); + } }); }); - }; + }) 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. useEffect(() => { - if (isPending) return; + if (isPending) { + return; + } if (clerkNavPromiseBuffer?.current?.length) { clerkNavPromiseBuffer.current.forEach(resolve => resolve()); } + clerkNavPromiseBuffer.current = []; }, [isPending]); diff --git a/packages/nextjs/src/client-boundary/usePathnameWithoutCatchAll.tsx b/packages/nextjs/src/client-boundary/usePathnameWithoutCatchAll.tsx index 0a1c4eb99c9..b30ca8e3101 100644 --- a/packages/nextjs/src/client-boundary/usePathnameWithoutCatchAll.tsx +++ b/packages/nextjs/src/client-boundary/usePathnameWithoutCatchAll.tsx @@ -1,6 +1,8 @@ import { useRouter } from 'next/compat/router'; +import React from 'react'; export const usePathnameWithoutWithCatchAll = () => { + const pathRef = React.useRef(); // The compat version of useRouter returns null instead of throwing an error // when used inside app router instead of pages router // we use it to detect if the component is used inside pages or app router @@ -8,9 +10,14 @@ export const usePathnameWithoutWithCatchAll = () => { const pagesRouter = useRouter(); if (pagesRouter) { - // in pages router things are simpler as the pathname includes the catch all route - // which starts with [[... and we can just remove it - return pagesRouter.pathname.replace(/\/\[\[\.\.\..*/, ''); + if (pathRef.current) { + return pathRef.current; + } else { + // in pages router things are simpler as the pathname includes the catch all route + // which starts with [[... and we can just remove it + pathRef.current = pagesRouter.pathname.replace(/\/\[\[\.\.\..*/, ''); + return pathRef.current; + } } // require is used to avoid importing next/navigation when the pages router is used, @@ -38,5 +45,10 @@ export const usePathnameWithoutWithCatchAll = () => { .flat(Infinity); // so we end up with the pathname where the components are mounted at // eg /user/123/profile/security will return /user/123/profile as the path - return `/${pathParts.slice(0, pathParts.length - catchAllParams.length).join('/')}`; + if (pathRef.current) { + return pathRef.current; + } else { + pathRef.current = `/${pathParts.slice(0, pathParts.length - catchAllParams.length).join('/')}`; + return pathRef.current; + } }; diff --git a/packages/types/src/clerk.ts b/packages/types/src/clerk.ts index ddf766ad244..72b83c578b9 100644 --- a/packages/types/src/clerk.ts +++ b/packages/types/src/clerk.ts @@ -510,8 +510,8 @@ type ClerkOptionsNavigation = routerReplace?: never; } | { - routerPush: (to: string) => Promise | unknown; - routerReplace: (to: string) => Promise | unknown; + routerPush: RouterFn; + routerReplace: RouterFn; routerDebug?: boolean; }; @@ -546,6 +546,7 @@ export type ClerkOptions = ClerkOptionsNavigation & export interface NavigateOptions { replace?: boolean; + metadata?: RouterMetadata; } export interface Resources { @@ -557,6 +558,35 @@ export interface Resources { export type RoutingStrategy = 'path' | 'hash' | 'virtual'; +/** + * Internal is a navigation type that affects the component + * + */ +type NavigationType = + /** + * Internal navigations affect the components and alter the + * part of the URL that comes after the `path` passed to the component. + * eg + * going from /sign-in to /sign-in/factor-one is an internal navigation + */ + | 'internal' + /** + * Internal navigations affect the components and alter the + * part of the URL that comes before the `path` passed to the component. + * eg + * going from /sign-in to / is an external navigation + */ + | 'external' + /** + * Window navigations are navigations towards a different origin + * and are not handled by the Clerk component or the host app router. + */ + | 'window'; + +type RouterMetadata = { routing?: RoutingStrategy; navigationType?: NavigationType }; + +type RouterFn = (to: string, metadata?: { __internal_metadata?: RouterMetadata }) => Promise | unknown; + export type WithoutRouting = Omit; export type SignInInitialValues = {