Skip to content

Commit 899fb96

Browse files
brkalowclaude
andauthored
fix(ui): Fix HashRouter not responding to popup OAuth navigations (#7944)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c438fa5 commit 899fb96

6 files changed

Lines changed: 92 additions & 5 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
---
4+
5+
Fix HashRouter not responding to popup OAuth navigations by adding `pushstate`/`replacestate` to refresh events and suppressing the history observer during external navigation.

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import App from './App.tsx';
77
import Protected from './protected';
88
import SignIn from './sign-in';
99
import SignInPopup from './sign-in-popup';
10+
import SignInHashPopup from './sign-in-hash-popup';
1011
import SignUp from './sign-up';
1112
import UserProfile from './user';
1213
import UserProfileCustom from './custom-user-profile';
@@ -66,6 +67,10 @@ const router = createBrowserRouter([
6667
path: '/sign-in-popup/*',
6768
element: <SignInPopup />,
6869
},
70+
{
71+
path: '/sign-in-hash-popup',
72+
element: <SignInHashPopup />,
73+
},
6974
{
7075
path: '/sign-up/*',
7176
element: <SignUp />,
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { SignIn } from '@clerk/react';
2+
3+
export default function Page() {
4+
return (
5+
<div>
6+
<SignIn
7+
routing='hash'
8+
signUpUrl={'/sign-up'}
9+
oauthFlow='popup'
10+
fallbackRedirectUrl='/protected'
11+
fallback={<>Loading sign in</>}
12+
/>
13+
</div>
14+
);
15+
}

integration/tests/oauth-flows.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,66 @@ testAgainstRunningApps({ withPattern: ['react.vite.withLegalConsent'] })(
238238
},
239239
);
240240

241+
testAgainstRunningApps({ withPattern: ['react.vite.withLegalConsent'] })(
242+
'oauth popup with hash-based routing @react',
243+
({ app }) => {
244+
test.describe.configure({ mode: 'serial' });
245+
246+
let fakeUser: FakeUser;
247+
248+
test.beforeAll(async () => {
249+
const client = createClerkClient({
250+
secretKey: instanceKeys.get('oauth-provider').sk,
251+
publishableKey: instanceKeys.get('oauth-provider').pk,
252+
});
253+
const users = createUserService(client);
254+
fakeUser = users.createFakeUser({
255+
withUsername: true,
256+
});
257+
await users.createBapiUser(fakeUser);
258+
});
259+
260+
test.afterAll(async () => {
261+
const u = createTestUtils({ app });
262+
await fakeUser.deleteIfExists();
263+
await u.services.users.deleteIfExists({ email: fakeUser.email });
264+
await app.teardown();
265+
});
266+
267+
test('popup OAuth navigates through sso-callback with hash-based routing', async ({ page, context }) => {
268+
const u = createTestUtils({ app, page, context });
269+
270+
await u.page.goToRelative('/sign-in-hash-popup');
271+
await u.page.waitForClerkJsLoaded();
272+
await u.po.signIn.waitForMounted();
273+
274+
const popupPromise = context.waitForEvent('page');
275+
await u.page.getByRole('button', { name: 'E2E OAuth Provider' }).click();
276+
const popup = await popupPromise;
277+
const popupUtils = createTestUtils({ app, page: popup, context });
278+
await popupUtils.page.getByText('Sign in to oauth-provider').waitFor();
279+
280+
// Complete OAuth in the popup
281+
await popupUtils.po.signIn.setIdentifier(fakeUser.email);
282+
await popupUtils.po.signIn.continue();
283+
await popupUtils.po.signIn.enterTestOtpCode();
284+
285+
// Because the user is new to the app and legal consent is required,
286+
// the sign-up can't complete in the popup. The popup sends return_url
287+
// back to the parent, which navigates to /sso-callback via pushState.
288+
// With hash routing, the HashRouter must detect this pushState change
289+
// to render the sso-callback route. hashchange does not fire for
290+
// pushState, so the router needs pushstate/replacestate listeners.
291+
await u.page.getByRole('heading', { name: 'Legal consent' }).waitFor();
292+
await u.page.getByLabel(/I agree to the/).check();
293+
await u.po.signIn.continue();
294+
295+
await u.page.getByText('SignedIn').waitFor();
296+
await u.po.expect.toBeSignedIn();
297+
});
298+
},
299+
);
300+
241301
testAgainstRunningApps({ withEnv: [appConfigs.envs.withLegalConsent] })(
242302
'oauth flows with legal consent @nextjs',
243303
({ app }) => {

packages/ui/src/router/BaseRouter.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,12 @@ export const BaseRouter = ({
232232
const isOutsideOfUIComponent = !toURL.pathname.startsWith('/' + basePath);
233233

234234
if (isOutsideOfUIComponent || isCrossOrigin) {
235-
const res = await clerkNavigate(toURL.href);
236-
// TODO: Since we are closing the modal, why do we need to refresh ? wouldn't that unmount everything causing the state to refresh ?
237-
refresh();
238-
return res;
235+
isNavigatingRef.current = true;
236+
try {
237+
return await clerkNavigate(toURL.href);
238+
} finally {
239+
isNavigatingRef.current = false;
240+
}
239241
}
240242

241243
// For internal navigation, preserve any query params

packages/ui/src/router/HashRouter.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export const HashRouter = ({ preservedParams, children }: HashRouterProps): JSX.
4444
startPath={''}
4545
getQueryString={getQueryString}
4646
internalNavigate={internalNavigate}
47-
refreshEvents={['popstate', 'hashchange']}
47+
refreshEvents={['pushstate', 'replacestate', 'popstate', 'hashchange']}
4848
preservedParams={preservedParams}
4949
>
5050
{children}

0 commit comments

Comments
 (0)