Skip to content

Commit

Permalink
fix(nextjs): Use separate buffers for each awaitable navigation type (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
panteliselef authored Jun 4, 2024
1 parent 3c2b666 commit c5bd714
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 16 deletions.
5 changes: 5 additions & 0 deletions .changeset/three-eels-battle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/nextjs': patch
---

Bug fix: Correctly update history state when on internal navigations.
2 changes: 1 addition & 1 deletion integration/tests/navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ testAgainstRunningApps({ withPattern: ['next.appRouter.withEmailCodes'] })('navi
await u.po.expect.toBeSignedIn();
});

test.skip('sign in with path routing navigates to previous page', async ({ page, context }) => {
test('sign in with path routing navigates to previous page', async ({ page, context }) => {
const u = createTestUtils({ app, page, context });
await u.po.signIn.goTo();
await u.po.signIn.waitForMounted();
Expand Down
1 change: 1 addition & 0 deletions packages/nextjs/src/app-router/client/useAwaitablePush.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ export const useAwaitablePush = () => {
return useInternalNavFun({
windowNav: typeof window !== 'undefined' ? window.history.pushState.bind(window.history) : undefined,
routerNav: router.push.bind(router),
name: 'push',
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ export const useAwaitableReplace = () => {
return useInternalNavFun({
windowNav: typeof window !== 'undefined' ? window.history.replaceState.bind(window.history) : undefined,
routerNav: router.replace.bind(router),
name: 'replace',
});
};
41 changes: 26 additions & 15 deletions packages/nextjs/src/app-router/client/useInternalNavFun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,40 @@ import type { NextClerkProviderProps } from '../../types';

declare global {
interface Window {
__clerk_internal_navFun: NonNullable<
NextClerkProviderProps['routerPush'] | NextClerkProviderProps['routerReplace']
__clerk_internal_navigations: Record<
string,
{
fun: NonNullable<NextClerkProviderProps['routerPush'] | NextClerkProviderProps['routerReplace']>;
promisesBuffer: Array<() => void> | undefined;
}
>;
__clerk_internal_navPromisesBuffer: Array<() => void> | undefined;
}
}

const getClerkNavigationObject = (name: string) => {
window.__clerk_internal_navigations ??= {};
// @ts-ignore
window.__clerk_internal_navigations[name] ??= {};
return window.__clerk_internal_navigations[name];
};

export const useInternalNavFun = (props: {
windowNav: typeof window.history.pushState | typeof window.history.replaceState | undefined;
routerNav: AppRouterInstance['push'] | AppRouterInstance['replace'];
name: string;
}) => {
const { windowNav, routerNav } = props;
const { windowNav, routerNav, name } = props;
const pathname = usePathname();
const [isPending, startTransition] = useTransition();

if (windowNav) {
window.__clerk_internal_navFun = (to, opts) => {
getClerkNavigationObject(name).fun = (to, opts) => {
return new Promise<void>(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);
// 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
getClerkNavigationObject(name).promisesBuffer ??= [];
getClerkNavigationObject(name).promisesBuffer?.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
Expand All @@ -54,8 +63,8 @@ export const useInternalNavFun = (props: {
}

const flushPromises = () => {
window.__clerk_internal_navPromisesBuffer?.forEach(resolve => resolve());
window.__clerk_internal_navPromisesBuffer = [];
getClerkNavigationObject(name).promisesBuffer?.forEach(resolve => resolve());
getClerkNavigationObject(name).promisesBuffer = [];
};

// Flush any pending promises on mount/unmount
Expand All @@ -72,6 +81,8 @@ export const useInternalNavFun = (props: {
}, [pathname, isPending]);

return useCallback((to: string) => {
return window.__clerk_internal_navFun(to);
return getClerkNavigationObject(name).fun(to);
// We are not expecting name to change
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
};

0 comments on commit c5bd714

Please sign in to comment.