fix(clerk-js): Sync BaseRouter state on pushState/replaceState (core 2)

.changeset/fix-router-sync-popup-oauth.md

@@ -0,0 +1,5 @@
+---
+'@clerk/clerk-js': patch
+---
+
+Fix BaseRouter state not syncing after popup OAuth by observing `pushState`/`replaceState` changes in addition to `popstate`

integration/presets/longRunningApps.ts

@@ -60,6 +60,7 @@ export const createLongRunningApps = () => {
     { id: 'react.vite.withEmailCodes', config: react.vite, env: envs.withEmailCodes },
     { id: 'react.vite.withEmailCodes_persist_client', config: react.vite, env: envs.withEmailCodes_destroy_client },
     { id: 'react.vite.withEmailLinks', config: react.vite, env: envs.withEmailLinks },
+    { id: 'react.vite.withLegalConsent', config: react.vite, env: envs.withLegalConsent },
     { id: 'vue.vite', config: vue.vite, env: envs.withCustomRoles },
 
     /**

integration/templates/react-vite/src/buttons/index.tsx

@@ -11,6 +11,15 @@ export default function Home() {
         Sign in button (force)
       </SignInButton>
 
+      <SignInButton
+        mode='modal'
+        oauthFlow='popup'
+        forceRedirectUrl='/protected'
+        signUpForceRedirectUrl='/protected'
+      >
+        Sign in button (force, popup)
+      </SignInButton>
+
       <SignInButton
         mode='modal'
         fallbackRedirectUrl='/protected'

integration/templates/react-vite/src/main.tsx

@@ -6,6 +6,7 @@ import { createBrowserRouter, Outlet, RouterProvider, useNavigate } from 'react-
 import App from './App.tsx';
 import Protected from './protected';
 import SignIn from './sign-in';
+import SignInPopup from './sign-in-popup';
 import SignUp from './sign-up';
 import UserProfile from './user';
 import UserProfileCustom from './custom-user-profile';
@@ -57,6 +58,10 @@ const router = createBrowserRouter([
         path: '/sign-in/*',
         element: <SignIn />,
       },
+      {
+        path: '/sign-in-popup/*',
+        element: <SignInPopup />,
+      },
       {
         path: '/sign-up/*',
         element: <SignUp />,

integration/templates/react-vite/src/sign-in-popup/index.tsx

@@ -0,0 +1,15 @@
+import { SignIn } from '@clerk/clerk-react';
+
+export default function Page() {
+  return (
+    <div>
+      <SignIn
+        path={'/sign-in-popup'}
+        signUpUrl={'/sign-up'}
+        oauthFlow='popup'
+        fallbackRedirectUrl='/protected'
+        fallback={<>Loading sign in</>}
+      />
+    </div>
+  );
+}

integration/tests/oauth-flows.test.ts

@@ -181,6 +181,63 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('oauth flo
   });
 });
 
+testAgainstRunningApps({ withPattern: ['react.vite.withLegalConsent'] })(
+  'oauth popup with path-based routing @react',
+  ({ app }) => {
+    test.describe.configure({ mode: 'serial' });
+
+    let fakeUser: FakeUser;
+
+    test.beforeAll(async () => {
+      const client = createClerkClient({
+        secretKey: instanceKeys.get('oauth-provider').sk,
+        publishableKey: instanceKeys.get('oauth-provider').pk,
+      });
+      const users = createUserService(client);
+      fakeUser = users.createFakeUser({
+        withUsername: true,
+      });
+      await users.createBapiUser(fakeUser);
+    });
+
+    test.afterAll(async () => {
+      const u = createTestUtils({ app });
+      await fakeUser.deleteIfExists();
+      await u.services.users.deleteIfExists({ email: fakeUser.email });
+      await app.teardown();
+    });
+
+    test('popup OAuth navigates through sso-callback with path-based routing', async ({ page, context }) => {
+      const u = createTestUtils({ app, page, context });
+
+      await u.page.goToRelative('/sign-in-popup');
+      await u.page.waitForClerkJsLoaded();
+      await u.po.signIn.waitForMounted();
+
+      const popupPromise = context.waitForEvent('page');
+      await u.page.getByRole('button', { name: 'E2E OAuth Provider' }).click();
+      const popup = await popupPromise;
+      const popupUtils = createTestUtils({ app, page: popup, context });
+      await popupUtils.page.getByText('Sign in to oauth-provider').waitFor();
+
+      // Complete OAuth in the popup
+      await popupUtils.po.signIn.setIdentifier(fakeUser.email);
+      await popupUtils.po.signIn.continue();
+      await popupUtils.po.signIn.enterTestOtpCode();
+
+      // Because the user is new to the app and legal consent is required,
+      // the sign-up can't complete in the popup. The popup sends return_url
+      // back to the parent, which navigates to /sso-callback via pushState.
+      await u.page.getByRole('heading', { name: 'Legal consent' }).waitFor();
+      await u.page.getByLabel(/I agree to the/).check();
+      await u.po.signIn.continue();
+
+      await u.page.waitForAppUrl('/protected');
+      await u.po.expect.toBeSignedIn();
+    });
+  },
+);
+
 testAgainstRunningApps({ withEnv: [appConfigs.envs.withLegalConsent] })(
   'oauth flows with legal consent @nextjs',
   ({ app }) => {

packages/clerk-js/src/ui/hooks/index.ts

@@ -17,4 +17,3 @@ export * from './useSafeState';
 export * from './useScrollLock';
 export * from './useSearchInput';
 export * from './useTotalEnabledAuthMethods';
-export * from './useWindowEventListener';

packages/clerk-js/src/ui/router/BaseRouter.tsx

@@ -1,21 +1,140 @@
 import { useClerk } from '@clerk/shared/react';
 import type { NavigateOptions } from '@clerk/shared/types';
 import React from 'react';
+import { flushSync } from 'react-dom';
 
 import { getQueryParams, stringifyQueryParams, trimTrailingSlash } from '../../utils';
-import { useWindowEventListener } from '../hooks';
 import { newPaths } from './newPaths';
 import { match } from './pathToRegexp';
 import { Route } from './Route';
 import { RouteContext } from './RouteContext';
 
+// Custom events that don't exist on WindowEventMap but are handled
+// by wrapping history.pushState/replaceState in the fallback path.
+type HistoryEvent = 'pushstate' | 'replacestate';
+type RefreshEvent = keyof WindowEventMap | HistoryEvent;
+type NavigationType = 'push' | 'replace' | 'traverse';
+
+const isWindowRefreshEvent = (event: RefreshEvent): event is keyof WindowEventMap => {
+  return event !== 'pushstate' && event !== 'replacestate';
+};
+
+// Maps refresh events to Navigation API navigationType values.
+const eventToNavigationType: Partial<Record<RefreshEvent, NavigationType>> = {
+  popstate: 'traverse',
+  pushstate: 'push',
+  replacestate: 'replace',
+};
+
+// Global subscription sets for the history monkey-patching fallback.
+// Using a single patch with subscriber sets avoids conflicts when
+// multiple BaseRouter instances mount simultaneously.
+const pushStateSubscribers = new Set<() => void>();
+const replaceStateSubscribers = new Set<() => void>();
+let originalPushState: History['pushState'] | null = null;
+let originalReplaceState: History['replaceState'] | null = null;
+
+function ensurePushStatePatched(): void {
+  if (originalPushState) {
+    return;
+  }
+  const original = history.pushState.bind(history);
+  originalPushState = original;
+  history.pushState = (...args: Parameters<History['pushState']>) => {
+    original(...args);
+    pushStateSubscribers.forEach(fn => fn());
+  };
+}
+
+function ensureReplaceStatePatched(): void {
+  if (originalReplaceState) {
+    return;
+  }
+  const original = history.replaceState.bind(history);
+  originalReplaceState = original;
+  history.replaceState = (...args: Parameters<History['replaceState']>) => {
+    original(...args);
+    replaceStateSubscribers.forEach(fn => fn());
+  };
+}
+
+/**
+ * Observes history changes so the router's internal state stays in sync
+ * with the URL. Uses the Navigation API when available, falling back to
+ * monkey-patching history.pushState/replaceState plus native window events.
+ *
+ * Note: `events` should be a stable array reference to avoid
+ * re-subscribing on every render.
+ */
+function useHistoryChangeObserver(events: Array<RefreshEvent> | undefined, callback: () => void): void {
+  const callbackRef = React.useRef(callback);
+  callbackRef.current = callback;
+
+  React.useEffect(() => {
+    if (!events) {
+      return;
+    }
+
+    const notify = () => callbackRef.current();
+    const windowEvents = events.filter(isWindowRefreshEvent);
+    const navigationTypes = events
+      .map(e => eventToNavigationType[e])
+      .filter((type): type is NavigationType => Boolean(type));
+
+    const hasNavigationAPI =
+      typeof window !== 'undefined' &&
+      'navigation' in window &&
+      typeof (window as any).navigation?.addEventListener === 'function';
+
+    if (hasNavigationAPI) {
+      const nav = (window as any).navigation;
+      const allowedTypes = new Set(navigationTypes);
+      const handler = (e: { navigationType: NavigationType }) => {
+        if (allowedTypes.has(e.navigationType)) {
+          Promise.resolve().then(notify);
+        }
+      };
+      nav.addEventListener('currententrychange', handler);
+
+      // Events without a navigationType mapping (e.g. hashchange) still
+      // need native listeners even when the Navigation API is available.
+      const unmappedEvents = windowEvents.filter(e => !eventToNavigationType[e]);
+      unmappedEvents.forEach(e => window.addEventListener(e, notify));
+
+      return () => {
+        nav.removeEventListener('currententrychange', handler);
+        unmappedEvents.forEach(e => window.removeEventListener(e, notify));
+      };
+    }
+
+    // Fallback: use global subscriber sets for pushState/replaceState
+    // so that multiple BaseRouter instances don't conflict.
+    if (events.includes('pushstate')) {
+      ensurePushStatePatched();
+      pushStateSubscribers.add(notify);
+    }
+    if (events.includes('replacestate')) {
+      ensureReplaceStatePatched();
+      replaceStateSubscribers.add(notify);
+    }
+
+    windowEvents.forEach(e => window.addEventListener(e, notify));
+
+    return () => {
+      pushStateSubscribers.delete(notify);
+      replaceStateSubscribers.delete(notify);
+      windowEvents.forEach(e => window.removeEventListener(e, notify));
+    };
+  }, [events]);
+}
+
 interface BaseRouterProps {
   basePath: string;
   startPath: string;
   getPath: () => string;
   getQueryString: () => string;
   internalNavigate: (toURL: URL, options?: NavigateOptions) => Promise<any> | any;
-  refreshEvents?: Array<keyof WindowEventMap>;
+  refreshEvents?: Array<RefreshEvent>;
   preservedParams?: string[];
   urlStateParam?: {
     startPath: string;
@@ -86,7 +205,23 @@ export const BaseRouter = ({
     }
   }, [currentPath, currentQueryString, getPath, getQueryString]);
 
-  useWindowEventListener(refreshEvents, refresh);
+  // Suppresses the history observer during baseNavigate's internal navigation.
+  // Without this, the observer's microtask triggers a render before setActive's
+  // #updateAccessors sets clerk.session, causing task guards to see stale state.
+  const isNavigatingRef = React.useRef(false);
+
+  const observerRefresh = React.useCallback((): void => {
+    if (isNavigatingRef.current) {
+      return;
+    }
+    const newPath = getPath();
+    if (basePath && !newPath.startsWith('/' + basePath)) {
+      return;
+    }
+    refresh();
+  }, [basePath, getPath, refresh]);
+
+  useHistoryChangeObserver(refreshEvents, observerRefresh);
 
   // TODO: Look into the real possible types of globalNavigate
   const baseNavigate = async (toURL: URL | undefined): Promise<unknown> => {
@@ -116,9 +251,20 @@ export const BaseRouter = ({
 
       toURL.search = stringifyQueryParams(toQueryParams);
     }
-    const internalNavRes = await internalNavigate(toURL, { metadata: { navigationType: 'internal' } });
-    setRouteParts({ path: toURL.pathname, queryString: toURL.search });
-    return internalNavRes;
+    isNavigatingRef.current = true;
+    try {
+      const internalNavRes = await internalNavigate(toURL, { metadata: { navigationType: 'internal' } });
+      // We need to flushSync to guarantee the re-render happens before handing things back to the caller,
+      // otherwise setActive might emit, and children re-render with the old navigation state.
+      // An alternative solution here could be to return a deferred promise, set that to state together
+      // with the routeParts and resolve it in an effect. That way we could avoid the flushSync performance penalty.
+      flushSync(() => {
+        setRouteParts({ path: toURL.pathname, queryString: toURL.search });
+      });
+      return internalNavRes;
+    } finally {
+      isNavigatingRef.current = false;
+    }
   };
 
   return (

packages/clerk-js/src/ui/router/PathRouter.tsx

@@ -59,7 +59,7 @@ export const PathRouter = ({ basePath, preservedParams, children }: PathRouterPr
       getPath={getPath}
       getQueryString={getQueryString}
       internalNavigate={internalNavigate}
-      refreshEvents={['popstate']}
+      refreshEvents={['pushstate', 'replacestate', 'popstate']}
       preservedParams={preservedParams}
     >
       {children}