Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/silly-tips-cry.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
---
'@clerk/clerk-js': patch
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
5 changes: 5 additions & 0 deletions .changeset/unlucky-crabs-grab.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
6 changes: 3 additions & 3 deletions packages/clerk-js/src/core/clerk.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -605,7 +605,7 @@ export default class Clerk implements ClerkInterface {
);
}

type SetActiveHook = () => void;
type SetActiveHook = () => void | Promise<void>;
const onBeforeSetActive: SetActiveHook =
typeof window !== 'undefined' && typeof window.__unstable__onBeforeSetActive === 'function'
? window.__unstable__onBeforeSetActive
Expand DownExpand Up@@ -640,7 +640,7 @@ export default 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
Expand DownExpand Up@@ -672,7 +672,7 @@ export default class Clerk implements ClerkInterface {

this.#setAccessors(newSession);
this.#emit();
onAfterSetActive();
await onAfterSetActive();
this.#resetComponentsState();
};

Expand Down
4 changes: 2 additions & 2 deletions packages/clerk-js/src/globals.d.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,8 +4,8 @@ declare global {
const __PKG_VERSION__: string;

interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => Promise<void> | void;
__unstable__onAfterSetActive: () => Promise<void> | void;
}
}

Expand Down
43 changes: 37 additions & 6 deletions packages/nextjs/src/app-router/client/ClerkProvider.tsx
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -13,24 +13,55 @@ declare global {
export interface Window {
__clerk_nav_await: Array<(value: void) => void>;
__clerk_nav: (to: string) => Promise<void>;
__clerk_internal_invalidateCachePromise: () => void | undefined;
}
}

export const ClientClerkProvider = (props: NextClerkProviderProps) => {
const { __unstable_invokeMiddlewareOnAuthStateChange = true } = 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 <Link href=/> 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();
}
};
}, []);

Expand Down
56 changes: 36 additions & 20 deletions packages/nextjs/src/app-router/client/useAwaitableNavigate.ts
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,62 @@
'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';

type NavigateFunction = ReturnType<typeof useRouter>['push'];
declare global {
interface Window {
__clerk_internal_navPromisesBuffer: Array<() => void> | undefined;
__clerk_internal_navFun: (to: string) => Promise<void>;
}
}

/**
* 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 = () => {
// eslint-disable-next-line @typescript-eslint/unbound-method
const { push } = useRouter();
const pathname = usePathname();
const [isPending, startTransition] = useTransition();
const clerkNavRef = useRef<(...args: Parameters<NavigateFunction>) => Promise<void>>();
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 => {
return new Promise<void>(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(() => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe removing the transition here will re-introduce the original issue, which is that state updates were happening during the router transition, or that the setActive call was not completing at all because of the navigation triggered by onBeforeSetActive.

Is there a way we can retain the use of the transition while keeping the buffer on the window?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll most likely keep using the transition API - this is removed temporarily so I can cut a snapshot version for testing

@nikosdouvlisnikosdouvlisFeb 29, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setActive call was not completing at all because of the navigation triggered by onBeforeSetActive

Regarding this one, I never managed to reproduce a failed setActive because of us not listening to isPending

I think the issue was caused by the components being unmounted during navigation but I do agree that keep using transition state is the way to go here

push(to, opts);
push(to);
});
});
};
}

// 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();
}
clerkNavPromiseBuffer.current = [];
}, [isPending]);
}, [pathname, isPending]);

return clerkNavRef.current;
return useCallback((to: string) => {
return window.__clerk_internal_navFun(to);
}, []);
};
4 changes: 2 additions & 2 deletions packages/nextjs/src/global.d.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
declare global {
interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => void | Promise<void>;
__unstable__onAfterSetActive: () => void | Promise<void>;
}
}

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/silly-tips-cry.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
---
'@clerk/clerk-js': patch
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
5 changes: 5 additions & 0 deletions .changeset/unlucky-crabs-grab.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
6 changes: 3 additions & 3 deletions packages/clerk-js/src/core/clerk.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -605,7 +605,7 @@ export default class Clerk implements ClerkInterface {
);
}

type SetActiveHook = () => void;
type SetActiveHook = () => void | Promise<void>;
const onBeforeSetActive: SetActiveHook =
typeof window !== 'undefined' && typeof window.__unstable__onBeforeSetActive === 'function'
? window.__unstable__onBeforeSetActive
Expand DownExpand Up@@ -640,7 +640,7 @@ export default 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
Expand DownExpand Up@@ -672,7 +672,7 @@ export default class Clerk implements ClerkInterface {

this.#setAccessors(newSession);
this.#emit();
onAfterSetActive();
await onAfterSetActive();
this.#resetComponentsState();
};

Expand Down
4 changes: 2 additions & 2 deletions packages/clerk-js/src/globals.d.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,8 +4,8 @@ declare global {
const __PKG_VERSION__: string;

interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => Promise<void> | void;
__unstable__onAfterSetActive: () => Promise<void> | void;
}
}

Expand Down
43 changes: 37 additions & 6 deletions packages/nextjs/src/app-router/client/ClerkProvider.tsx
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -13,24 +13,55 @@ declare global {
export interface Window {
__clerk_nav_await: Array<(value: void) => void>;
__clerk_nav: (to: string) => Promise<void>;
__clerk_internal_invalidateCachePromise: () => void | undefined;
}
}

export const ClientClerkProvider = (props: NextClerkProviderProps) => {
const { __unstable_invokeMiddlewareOnAuthStateChange = true } = 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 <Link href=/> 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();
}
};
}, []);

Expand Down
56 changes: 36 additions & 20 deletions packages/nextjs/src/app-router/client/useAwaitableNavigate.ts
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,62 @@
'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';

type NavigateFunction = ReturnType<typeof useRouter>['push'];
declare global {
interface Window {
__clerk_internal_navPromisesBuffer: Array<() => void> | undefined;
__clerk_internal_navFun: (to: string) => Promise<void>;
}
}

/**
* 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 = () => {
// eslint-disable-next-line @typescript-eslint/unbound-method
const { push } = useRouter();
const pathname = usePathname();
const [isPending, startTransition] = useTransition();
const clerkNavRef = useRef<(...args: Parameters<NavigateFunction>) => Promise<void>>();
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 => {
return new Promise<void>(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(() => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe removing the transition here will re-introduce the original issue, which is that state updates were happening during the router transition, or that the setActive call was not completing at all because of the navigation triggered by onBeforeSetActive.

Is there a way we can retain the use of the transition while keeping the buffer on the window?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll most likely keep using the transition API - this is removed temporarily so I can cut a snapshot version for testing

@nikosdouvlisnikosdouvlisFeb 29, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setActive call was not completing at all because of the navigation triggered by onBeforeSetActive

Regarding this one, I never managed to reproduce a failed setActive because of us not listening to isPending

I think the issue was caused by the components being unmounted during navigation but I do agree that keep using transition state is the way to go here

push(to, opts);
push(to);
});
});
};
}

// 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();
}
clerkNavPromiseBuffer.current = [];
}, [isPending]);
}, [pathname, isPending]);

return clerkNavRef.current;
return useCallback((to: string) => {
return window.__clerk_internal_navFun(to);
}, []);
};
4 changes: 2 additions & 2 deletions packages/nextjs/src/global.d.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
declare global {
interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => void | Promise<void>;
__unstable__onAfterSetActive: () => void | Promise<void>;
}
}

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/silly-tips-cry.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
---
'@clerk/clerk-js': patch
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
5 changes: 5 additions & 0 deletions .changeset/unlucky-crabs-grab.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
6 changes: 3 additions & 3 deletions packages/clerk-js/src/core/clerk.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -605,7 +605,7 @@ export default class Clerk implements ClerkInterface {
);
}

type SetActiveHook = () => void;
type SetActiveHook = () => void | Promise<void>;
const onBeforeSetActive: SetActiveHook =
typeof window !== 'undefined' && typeof window.__unstable__onBeforeSetActive === 'function'
? window.__unstable__onBeforeSetActive
Expand DownExpand Up@@ -640,7 +640,7 @@ export default 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
Expand DownExpand Up@@ -672,7 +672,7 @@ export default class Clerk implements ClerkInterface {

this.#setAccessors(newSession);
this.#emit();
onAfterSetActive();
await onAfterSetActive();
this.#resetComponentsState();
};

Expand Down
4 changes: 2 additions & 2 deletions packages/clerk-js/src/globals.d.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,8 +4,8 @@ declare global {
const __PKG_VERSION__: string;

interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => Promise<void> | void;
__unstable__onAfterSetActive: () => Promise<void> | void;
}
}

Expand Down
43 changes: 37 additions & 6 deletions packages/nextjs/src/app-router/client/ClerkProvider.tsx
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -13,24 +13,55 @@ declare global {
export interface Window {
__clerk_nav_await: Array<(value: void) => void>;
__clerk_nav: (to: string) => Promise<void>;
__clerk_internal_invalidateCachePromise: () => void | undefined;
}
}

export const ClientClerkProvider = (props: NextClerkProviderProps) => {
const { __unstable_invokeMiddlewareOnAuthStateChange = true } = 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 <Link href=/> 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();
}
};
}, []);

Expand Down
56 changes: 36 additions & 20 deletions packages/nextjs/src/app-router/client/useAwaitableNavigate.ts
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,62 @@
'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';

type NavigateFunction = ReturnType<typeof useRouter>['push'];
declare global {
interface Window {
__clerk_internal_navPromisesBuffer: Array<() => void> | undefined;
__clerk_internal_navFun: (to: string) => Promise<void>;
}
}

/**
* 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 = () => {
// eslint-disable-next-line @typescript-eslint/unbound-method
const { push } = useRouter();
const pathname = usePathname();
const [isPending, startTransition] = useTransition();
const clerkNavRef = useRef<(...args: Parameters<NavigateFunction>) => Promise<void>>();
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 => {
return new Promise<void>(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(() => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe removing the transition here will re-introduce the original issue, which is that state updates were happening during the router transition, or that the setActive call was not completing at all because of the navigation triggered by onBeforeSetActive.

Is there a way we can retain the use of the transition while keeping the buffer on the window?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll most likely keep using the transition API - this is removed temporarily so I can cut a snapshot version for testing

@nikosdouvlisnikosdouvlisFeb 29, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setActive call was not completing at all because of the navigation triggered by onBeforeSetActive

Regarding this one, I never managed to reproduce a failed setActive because of us not listening to isPending

I think the issue was caused by the components being unmounted during navigation but I do agree that keep using transition state is the way to go here

push(to, opts);
push(to);
});
});
};
}

// 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();
}
clerkNavPromiseBuffer.current = [];
}, [isPending]);
}, [pathname, isPending]);

return clerkNavRef.current;
return useCallback((to: string) => {
return window.__clerk_internal_navFun(to);
}, []);
};
4 changes: 2 additions & 2 deletions packages/nextjs/src/global.d.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
declare global {
interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => void | Promise<void>;
__unstable__onAfterSetActive: () => void | Promise<void>;
}
}

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/silly-tips-cry.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
---
'@clerk/clerk-js': patch
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
5 changes: 5 additions & 0 deletions .changeset/unlucky-crabs-grab.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
6 changes: 3 additions & 3 deletions packages/clerk-js/src/core/clerk.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -605,7 +605,7 @@ export default class Clerk implements ClerkInterface {
);
}

type SetActiveHook = () => void;
type SetActiveHook = () => void | Promise<void>;
const onBeforeSetActive: SetActiveHook =
typeof window !== 'undefined' && typeof window.__unstable__onBeforeSetActive === 'function'
? window.__unstable__onBeforeSetActive
Expand DownExpand Up@@ -640,7 +640,7 @@ export default 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
Expand DownExpand Up@@ -672,7 +672,7 @@ export default class Clerk implements ClerkInterface {

this.#setAccessors(newSession);
this.#emit();
onAfterSetActive();
await onAfterSetActive();
this.#resetComponentsState();
};

Expand Down
4 changes: 2 additions & 2 deletions packages/clerk-js/src/globals.d.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,8 +4,8 @@ declare global {
const __PKG_VERSION__: string;

interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => Promise<void> | void;
__unstable__onAfterSetActive: () => Promise<void> | void;
}
}

Expand Down
43 changes: 37 additions & 6 deletions packages/nextjs/src/app-router/client/ClerkProvider.tsx
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -13,24 +13,55 @@ declare global {
export interface Window {
__clerk_nav_await: Array<(value: void) => void>;
__clerk_nav: (to: string) => Promise<void>;
__clerk_internal_invalidateCachePromise: () => void | undefined;
}
}

export const ClientClerkProvider = (props: NextClerkProviderProps) => {
const { __unstable_invokeMiddlewareOnAuthStateChange = true } = 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 <Link href=/> 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();
}
};
}, []);

Expand Down
56 changes: 36 additions & 20 deletions packages/nextjs/src/app-router/client/useAwaitableNavigate.ts
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,62 @@
'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';

type NavigateFunction = ReturnType<typeof useRouter>['push'];
declare global {
interface Window {
__clerk_internal_navPromisesBuffer: Array<() => void> | undefined;
__clerk_internal_navFun: (to: string) => Promise<void>;
}
}

/**
* 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 = () => {
// eslint-disable-next-line @typescript-eslint/unbound-method
const { push } = useRouter();
const pathname = usePathname();
const [isPending, startTransition] = useTransition();
const clerkNavRef = useRef<(...args: Parameters<NavigateFunction>) => Promise<void>>();
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 => {
return new Promise<void>(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(() => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe removing the transition here will re-introduce the original issue, which is that state updates were happening during the router transition, or that the setActive call was not completing at all because of the navigation triggered by onBeforeSetActive.

Is there a way we can retain the use of the transition while keeping the buffer on the window?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll most likely keep using the transition API - this is removed temporarily so I can cut a snapshot version for testing

@nikosdouvlisnikosdouvlisFeb 29, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setActive call was not completing at all because of the navigation triggered by onBeforeSetActive

Regarding this one, I never managed to reproduce a failed setActive because of us not listening to isPending

I think the issue was caused by the components being unmounted during navigation but I do agree that keep using transition state is the way to go here

push(to, opts);
push(to);
});
});
};
}

// 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();
}
clerkNavPromiseBuffer.current = [];
}, [isPending]);
}, [pathname, isPending]);

return clerkNavRef.current;
return useCallback((to: string) => {
return window.__clerk_internal_navFun(to);
}, []);
};
4 changes: 2 additions & 2 deletions packages/nextjs/src/global.d.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
declare global {
interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => void | Promise<void>;
__unstable__onAfterSetActive: () => void | Promise<void>;
}
}

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/silly-tips-cry.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
---
'@clerk/clerk-js': patch
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
5 changes: 5 additions & 0 deletions .changeset/unlucky-crabs-grab.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
6 changes: 3 additions & 3 deletions packages/clerk-js/src/core/clerk.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -605,7 +605,7 @@ export default class Clerk implements ClerkInterface {
);
}

type SetActiveHook = () => void;
type SetActiveHook = () => void | Promise<void>;
const onBeforeSetActive: SetActiveHook =
typeof window !== 'undefined' && typeof window.__unstable__onBeforeSetActive === 'function'
? window.__unstable__onBeforeSetActive
Expand DownExpand Up@@ -640,7 +640,7 @@ export default 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
Expand DownExpand Up@@ -672,7 +672,7 @@ export default class Clerk implements ClerkInterface {

this.#setAccessors(newSession);
this.#emit();
onAfterSetActive();
await onAfterSetActive();
this.#resetComponentsState();
};

Expand Down
4 changes: 2 additions & 2 deletions packages/clerk-js/src/globals.d.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,8 +4,8 @@ declare global {
const __PKG_VERSION__: string;

interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => Promise<void> | void;
__unstable__onAfterSetActive: () => Promise<void> | void;
}
}

Expand Down
43 changes: 37 additions & 6 deletions packages/nextjs/src/app-router/client/ClerkProvider.tsx
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -13,24 +13,55 @@ declare global {
export interface Window {
__clerk_nav_await: Array<(value: void) => void>;
__clerk_nav: (to: string) => Promise<void>;
__clerk_internal_invalidateCachePromise: () => void | undefined;
}
}

export const ClientClerkProvider = (props: NextClerkProviderProps) => {
const { __unstable_invokeMiddlewareOnAuthStateChange = true } = 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 <Link href=/> 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();
}
};
}, []);

Expand Down
56 changes: 36 additions & 20 deletions packages/nextjs/src/app-router/client/useAwaitableNavigate.ts
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,62 @@
'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';

type NavigateFunction = ReturnType<typeof useRouter>['push'];
declare global {
interface Window {
__clerk_internal_navPromisesBuffer: Array<() => void> | undefined;
__clerk_internal_navFun: (to: string) => Promise<void>;
}
}

/**
* 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 = () => {
// eslint-disable-next-line @typescript-eslint/unbound-method
const { push } = useRouter();
const pathname = usePathname();
const [isPending, startTransition] = useTransition();
const clerkNavRef = useRef<(...args: Parameters<NavigateFunction>) => Promise<void>>();
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 => {
return new Promise<void>(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(() => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe removing the transition here will re-introduce the original issue, which is that state updates were happening during the router transition, or that the setActive call was not completing at all because of the navigation triggered by onBeforeSetActive.

Is there a way we can retain the use of the transition while keeping the buffer on the window?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll most likely keep using the transition API - this is removed temporarily so I can cut a snapshot version for testing

@nikosdouvlisnikosdouvlisFeb 29, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setActive call was not completing at all because of the navigation triggered by onBeforeSetActive

Regarding this one, I never managed to reproduce a failed setActive because of us not listening to isPending

I think the issue was caused by the components being unmounted during navigation but I do agree that keep using transition state is the way to go here

push(to, opts);
push(to);
});
});
};
}

// 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();
}
clerkNavPromiseBuffer.current = [];
}, [isPending]);
}, [pathname, isPending]);

return clerkNavRef.current;
return useCallback((to: string) => {
return window.__clerk_internal_navFun(to);
}, []);
};
4 changes: 2 additions & 2 deletions packages/nextjs/src/global.d.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
declare global {
interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => void | Promise<void>;
__unstable__onAfterSetActive: () => void | Promise<void>;
}
}

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/silly-tips-cry.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
---
'@clerk/clerk-js': patch
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
5 changes: 5 additions & 0 deletions .changeset/unlucky-crabs-grab.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
6 changes: 3 additions & 3 deletions packages/clerk-js/src/core/clerk.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -605,7 +605,7 @@ export default class Clerk implements ClerkInterface {
);
}

type SetActiveHook = () => void;
type SetActiveHook = () => void | Promise<void>;
const onBeforeSetActive: SetActiveHook =
typeof window !== 'undefined' && typeof window.__unstable__onBeforeSetActive === 'function'
? window.__unstable__onBeforeSetActive
Expand DownExpand Up@@ -640,7 +640,7 @@ export default 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
Expand DownExpand Up@@ -672,7 +672,7 @@ export default class Clerk implements ClerkInterface {

this.#setAccessors(newSession);
this.#emit();
onAfterSetActive();
await onAfterSetActive();
this.#resetComponentsState();
};

Expand Down
4 changes: 2 additions & 2 deletions packages/clerk-js/src/globals.d.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,8 +4,8 @@ declare global {
const __PKG_VERSION__: string;

interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => Promise<void> | void;
__unstable__onAfterSetActive: () => Promise<void> | void;
}
}

Expand Down
43 changes: 37 additions & 6 deletions packages/nextjs/src/app-router/client/ClerkProvider.tsx
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -13,24 +13,55 @@ declare global {
export interface Window {
__clerk_nav_await: Array<(value: void) => void>;
__clerk_nav: (to: string) => Promise<void>;
__clerk_internal_invalidateCachePromise: () => void | undefined;
}
}

export const ClientClerkProvider = (props: NextClerkProviderProps) => {
const { __unstable_invokeMiddlewareOnAuthStateChange = true } = 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 <Link href=/> 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();
}
};
}, []);

Expand Down
56 changes: 36 additions & 20 deletions packages/nextjs/src/app-router/client/useAwaitableNavigate.ts
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,62 @@
'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';

type NavigateFunction = ReturnType<typeof useRouter>['push'];
declare global {
interface Window {
__clerk_internal_navPromisesBuffer: Array<() => void> | undefined;
__clerk_internal_navFun: (to: string) => Promise<void>;
}
}

/**
* 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 = () => {
// eslint-disable-next-line @typescript-eslint/unbound-method
const { push } = useRouter();
const pathname = usePathname();
const [isPending, startTransition] = useTransition();
const clerkNavRef = useRef<(...args: Parameters<NavigateFunction>) => Promise<void>>();
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 => {
return new Promise<void>(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(() => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe removing the transition here will re-introduce the original issue, which is that state updates were happening during the router transition, or that the setActive call was not completing at all because of the navigation triggered by onBeforeSetActive.

Is there a way we can retain the use of the transition while keeping the buffer on the window?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll most likely keep using the transition API - this is removed temporarily so I can cut a snapshot version for testing

@nikosdouvlisnikosdouvlisFeb 29, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setActive call was not completing at all because of the navigation triggered by onBeforeSetActive

Regarding this one, I never managed to reproduce a failed setActive because of us not listening to isPending

I think the issue was caused by the components being unmounted during navigation but I do agree that keep using transition state is the way to go here

push(to, opts);
push(to);
});
});
};
}

// 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();
}
clerkNavPromiseBuffer.current = [];
}, [isPending]);
}, [pathname, isPending]);

return clerkNavRef.current;
return useCallback((to: string) => {
return window.__clerk_internal_navFun(to);
}, []);
};
4 changes: 2 additions & 2 deletions packages/nextjs/src/global.d.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
declare global {
interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => void | Promise<void>;
__unstable__onAfterSetActive: () => void | Promise<void>;
}
}

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/silly-tips-cry.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
---
'@clerk/clerk-js': patch
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
5 changes: 5 additions & 0 deletions .changeset/unlucky-crabs-grab.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
6 changes: 3 additions & 3 deletions packages/clerk-js/src/core/clerk.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -605,7 +605,7 @@ export default class Clerk implements ClerkInterface {
);
}

type SetActiveHook = () => void;
type SetActiveHook = () => void | Promise<void>;
const onBeforeSetActive: SetActiveHook =
typeof window !== 'undefined' && typeof window.__unstable__onBeforeSetActive === 'function'
? window.__unstable__onBeforeSetActive
Expand DownExpand Up@@ -640,7 +640,7 @@ export default 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
Expand DownExpand Up@@ -672,7 +672,7 @@ export default class Clerk implements ClerkInterface {

this.#setAccessors(newSession);
this.#emit();
onAfterSetActive();
await onAfterSetActive();
this.#resetComponentsState();
};

Expand Down
4 changes: 2 additions & 2 deletions packages/clerk-js/src/globals.d.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,8 +4,8 @@ declare global {
const __PKG_VERSION__: string;

interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => Promise<void> | void;
__unstable__onAfterSetActive: () => Promise<void> | void;
}
}

Expand Down
43 changes: 37 additions & 6 deletions packages/nextjs/src/app-router/client/ClerkProvider.tsx
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -13,24 +13,55 @@ declare global {
export interface Window {
__clerk_nav_await: Array<(value: void) => void>;
__clerk_nav: (to: string) => Promise<void>;
__clerk_internal_invalidateCachePromise: () => void | undefined;
}
}

export const ClientClerkProvider = (props: NextClerkProviderProps) => {
const { __unstable_invokeMiddlewareOnAuthStateChange = true } = 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 <Link href=/> 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();
}
};
}, []);

Expand Down
56 changes: 36 additions & 20 deletions packages/nextjs/src/app-router/client/useAwaitableNavigate.ts
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,62 @@
'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';

type NavigateFunction = ReturnType<typeof useRouter>['push'];
declare global {
interface Window {
__clerk_internal_navPromisesBuffer: Array<() => void> | undefined;
__clerk_internal_navFun: (to: string) => Promise<void>;
}
}

/**
* 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 = () => {
// eslint-disable-next-line @typescript-eslint/unbound-method
const { push } = useRouter();
const pathname = usePathname();
const [isPending, startTransition] = useTransition();
const clerkNavRef = useRef<(...args: Parameters<NavigateFunction>) => Promise<void>>();
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 => {
return new Promise<void>(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(() => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe removing the transition here will re-introduce the original issue, which is that state updates were happening during the router transition, or that the setActive call was not completing at all because of the navigation triggered by onBeforeSetActive.

Is there a way we can retain the use of the transition while keeping the buffer on the window?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll most likely keep using the transition API - this is removed temporarily so I can cut a snapshot version for testing

@nikosdouvlisnikosdouvlisFeb 29, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setActive call was not completing at all because of the navigation triggered by onBeforeSetActive

Regarding this one, I never managed to reproduce a failed setActive because of us not listening to isPending

I think the issue was caused by the components being unmounted during navigation but I do agree that keep using transition state is the way to go here

push(to, opts);
push(to);
});
});
};
}

// 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();
}
clerkNavPromiseBuffer.current = [];
}, [isPending]);
}, [pathname, isPending]);

return clerkNavRef.current;
return useCallback((to: string) => {
return window.__clerk_internal_navFun(to);
}, []);
};
4 changes: 2 additions & 2 deletions packages/nextjs/src/global.d.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
declare global {
interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => void | Promise<void>;
__unstable__onAfterSetActive: () => void | Promise<void>;
}
}

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/silly-tips-cry.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
---
'@clerk/clerk-js': patch
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
5 changes: 5 additions & 0 deletions .changeset/unlucky-crabs-grab.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
'@clerk/nextjs': patch
---

Make useAwaitableNavigate handle navigations between pages reliably
6 changes: 3 additions & 3 deletions packages/clerk-js/src/core/clerk.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -605,7 +605,7 @@ export default class Clerk implements ClerkInterface {
);
}

type SetActiveHook = () => void;
type SetActiveHook = () => void | Promise<void>;
const onBeforeSetActive: SetActiveHook =
typeof window !== 'undefined' && typeof window.__unstable__onBeforeSetActive === 'function'
? window.__unstable__onBeforeSetActive
Expand DownExpand Up@@ -640,7 +640,7 @@ export default 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
Expand DownExpand Up@@ -672,7 +672,7 @@ export default class Clerk implements ClerkInterface {

this.#setAccessors(newSession);
this.#emit();
onAfterSetActive();
await onAfterSetActive();
this.#resetComponentsState();
};

Expand Down
4 changes: 2 additions & 2 deletions packages/clerk-js/src/globals.d.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,8 +4,8 @@ declare global {
const __PKG_VERSION__: string;

interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => Promise<void> | void;
__unstable__onAfterSetActive: () => Promise<void> | void;
}
}

Expand Down
43 changes: 37 additions & 6 deletions packages/nextjs/src/app-router/client/ClerkProvider.tsx
Original file line numberDiff line numberDiff line change
@@ -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';
Expand All@@ -13,24 +13,55 @@ declare global {
export interface Window {
__clerk_nav_await: Array<(value: void) => void>;
__clerk_nav: (to: string) => Promise<void>;
__clerk_internal_invalidateCachePromise: () => void | undefined;
}
}

export const ClientClerkProvider = (props: NextClerkProviderProps) => {
const { __unstable_invokeMiddlewareOnAuthStateChange = true } = 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 <Link href=/> 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();
}
};
}, []);

Expand Down
56 changes: 36 additions & 20 deletions packages/nextjs/src/app-router/client/useAwaitableNavigate.ts
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,62 @@
'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';

type NavigateFunction = ReturnType<typeof useRouter>['push'];
declare global {
interface Window {
__clerk_internal_navPromisesBuffer: Array<() => void> | undefined;
__clerk_internal_navFun: (to: string) => Promise<void>;
}
}

/**
* 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 = () => {
// eslint-disable-next-line @typescript-eslint/unbound-method
const { push } = useRouter();
const pathname = usePathname();
const [isPending, startTransition] = useTransition();
const clerkNavRef = useRef<(...args: Parameters<NavigateFunction>) => Promise<void>>();
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 => {
return new Promise<void>(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(() => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe removing the transition here will re-introduce the original issue, which is that state updates were happening during the router transition, or that the setActive call was not completing at all because of the navigation triggered by onBeforeSetActive.

Is there a way we can retain the use of the transition while keeping the buffer on the window?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll most likely keep using the transition API - this is removed temporarily so I can cut a snapshot version for testing

@nikosdouvlisnikosdouvlisFeb 29, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setActive call was not completing at all because of the navigation triggered by onBeforeSetActive

Regarding this one, I never managed to reproduce a failed setActive because of us not listening to isPending

I think the issue was caused by the components being unmounted during navigation but I do agree that keep using transition state is the way to go here

push(to, opts);
push(to);
});
});
};
}

// 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();
}
clerkNavPromiseBuffer.current = [];
}, [isPending]);
}, [pathname, isPending]);

return clerkNavRef.current;
return useCallback((to: string) => {
return window.__clerk_internal_navFun(to);
}, []);
};
4 changes: 2 additions & 2 deletions packages/nextjs/src/global.d.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
declare global {
interface Window {
__unstable__onBeforeSetActive: () => void;
__unstable__onAfterSetActive: () => void;
__unstable__onBeforeSetActive: () => void | Promise<void>;
__unstable__onAfterSetActive: () => void | Promise<void>;
}
}

Expand Down