merge dual-router navigation guard

apps/studio/TANSTACK_MIGRATION.md

@@ -455,10 +455,9 @@ inline.
   `router.subscribe(tsEvent, …)`. Forwards Next's `(url, { shallow })`
   args. Maps `routeChangeStart` / `routeChangeComplete` /
   `beforeHistoryChange` / `hashChangeStart` / `hashChangeComplete`.
-  **Known gap:** Next's throw-from-`routeChangeStart`-to-cancel pattern
-  isn't supportable — `subscribe` is fire-and-forget.
-  `usePreventNavigationOnUnsavedChanges` relies on it and needs
-  migrating to TanStack's `useBlocker` separately.
+  **Cancellation:** Next's throw-from-`routeChangeStart`-to-cancel pattern
+  isn't supportable because `subscribe` is fire-and-forget.
+  `usePreventNavigationOnUnsavedChanges` blocks through TanStack history instead.
 - `api.ts` — `toWebHandler(nextHandler)`. See **API routes → Shim
   coverage** above.
 - `link.tsx`, `navigation.ts`, `dynamic.tsx`, `image.tsx`,
@@ -604,7 +603,7 @@ for the Vite pipeline:
 
 - Switch `routes/index.tsx` redirects from `href` to `to` — all targets
   now live in the TanStack tree.
-- Migrate `usePreventNavigationOnUnsavedChanges` from `router.events.on('routeChangeStart', …)` (throw-to-cancel pattern) to TanStack's `useBlocker`.
+- [x] Make `usePreventNavigationOnUnsavedChanges` block through TanStack history while retaining the Next implementation for the legacy runtime.
 - Drop the `_splat` / `routeSlug` normalisation block from
   `pages/org/_/[[...routeSlug]].tsx` + `pages/project/_/[[...routeSlug]].tsx` (only there to keep both runtimes mounting the same body).
 - Remove `RouteValidationWrapper` + `next/router` compat shim usage from `__root.tsx`.

apps/studio/compat/next/_router-events.ts

@@ -14,11 +14,9 @@
 //   { fromLocation, toLocation, pathChanged, hrefChanged, hashChanged }
 // — we forward `toLocation.href` as the URL arg.
 //
-// Known gap: Next's `routeChangeStart` lets handlers throw to cancel the
-// navigation. TanStack's `subscribe` is fire-and-forget; cancellation
-// requires `useBlocker` instead. `usePreventNavigationOnUnsavedChanges`
-// relies on the throw-to-cancel pattern and will need migrating to
-// `useBlocker` separately.
+// Next's `routeChangeStart` lets handlers throw to cancel navigation. TanStack's
+// `subscribe` is fire-and-forget, so the shared unsaved-changes hook blocks through
+// TanStack history before navigation begins instead.
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 type AnyRouter = any

apps/studio/hooks/ui/usePreventNavigationOnUnsavedChanges.test.ts

@@ -1,32 +1,120 @@
-import { act, renderHook } from '@testing-library/react'
+import { act, renderHook, waitFor } from '@testing-library/react'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 import { usePreventNavigationOnUnsavedChanges } from './usePreventNavigationOnUnsavedChanges'
 
-const routerEvents = {
-  on: vi.fn(),
-  off: vi.fn(),
-}
+const mocks = vi.hoisted(() => ({
+  tanStackRouter: undefined as undefined | { history: { block: ReturnType<typeof vi.fn> } },
+  nextEvents: { on: vi.fn(), off: vi.fn() },
+  nextPush: vi.fn(),
+}))
+
+vi.mock('@tanstack/react-router', () => ({
+  useRouter: () => mocks.tanStackRouter,
+}))
 
 vi.mock('next/router', () => ({
-  useRouter: () => ({ events: routerEvents, push: vi.fn() }),
+  useRouter: () => ({ events: mocks.nextEvents, push: mocks.nextPush }),
 }))
 
 describe('usePreventNavigationOnUnsavedChanges', () => {
   beforeEach(() => {
-    routerEvents.on.mockClear()
-    routerEvents.off.mockClear()
+    vi.clearAllMocks()
+    mocks.tanStackRouter = undefined
   })
 
-  it('bypasses only the next intentional navigation', () => {
-    const { result } = renderHook(() => usePreventNavigationOnUnsavedChanges({ hasChanges: true }))
-    const routeChangeHandler = routerEvents.on.mock.calls.find(
+  it('retains the legacy Next navigation guard', () => {
+    renderHook(() => usePreventNavigationOnUnsavedChanges({ hasChanges: true }))
+    const routeChangeHandler = mocks.nextEvents.on.mock.calls.find(
       ([event]) => event === 'routeChangeStart'
     )?.[1]
 
-    act(() => result.current.bypassNavigationGuard())
-
-    expect(() => act(() => routeChangeHandler('/replication'))).not.toThrow()
     expect(() => act(() => routeChangeHandler('/settings'))).toThrow('Route change declined')
   })
+
+  it('blocks TanStack navigation before the route changes and can cancel it', async () => {
+    let blockerFn: (() => Promise<boolean>) | undefined
+    const unblock = vi.fn()
+    mocks.tanStackRouter = {
+      history: {
+        block: vi.fn(({ blockerFn: nextBlockerFn }) => {
+          blockerFn = nextBlockerFn
+          return unblock
+        }),
+      },
+    }
+
+    const { result } = renderHook(() => usePreventNavigationOnUnsavedChanges({ hasChanges: true }))
+    let navigationResult: Promise<boolean>
+    act(() => {
+      navigationResult = blockerFn!()
+    })
+
+    await waitFor(() => expect(result.current.shouldConfirmNavigation).toBe(true))
+    act(() => result.current.handleCancelNavigation())
+
+    let resolvedNavigation: boolean | undefined
+    await act(async () => {
+      resolvedNavigation = await navigationResult!
+    })
+    expect(resolvedNavigation).toBe(true)
+    await waitFor(() => expect(result.current.shouldConfirmNavigation).toBe(false))
+    expect(mocks.nextEvents.on).not.toHaveBeenCalled()
+  })
+
+  it('allows confirmed TanStack navigation to proceed', async () => {
+    let blockerFn: (() => Promise<boolean>) | undefined
+    mocks.tanStackRouter = {
+      history: {
+        block: vi.fn(({ blockerFn: nextBlockerFn }) => {
+          blockerFn = nextBlockerFn
+          return vi.fn()
+        }),
+      },
+    }
+
+    const { result } = renderHook(() => usePreventNavigationOnUnsavedChanges({ hasChanges: true }))
+    let navigationResult: Promise<boolean>
+    act(() => {
+      navigationResult = blockerFn!()
+    })
+
+    await waitFor(() => expect(result.current.shouldConfirmNavigation).toBe(true))
+    act(() => result.current.handleConfirmNavigation())
+
+    let resolvedNavigation: boolean | undefined
+    await act(async () => {
+      resolvedNavigation = await navigationResult!
+    })
+    expect(resolvedNavigation).toBe(false)
+  })
+
+  it('bypasses only the next intentional TanStack navigation', async () => {
+    let blockerFn: (() => Promise<boolean>) | undefined
+    mocks.tanStackRouter = {
+      history: {
+        block: vi.fn(({ blockerFn: nextBlockerFn }) => {
+          blockerFn = nextBlockerFn
+          return vi.fn()
+        }),
+      },
+    }
+
+    const { result } = renderHook(() => usePreventNavigationOnUnsavedChanges({ hasChanges: true }))
+    act(() => result.current.bypassNavigationGuard())
+
+    await expect(blockerFn!()).resolves.toBe(false)
+
+    let secondNavigation: Promise<boolean>
+    act(() => {
+      secondNavigation = blockerFn!()
+    })
+    await waitFor(() => expect(result.current.shouldConfirmNavigation).toBe(true))
+    act(() => result.current.handleCancelNavigation())
+    let resolvedNavigation: boolean | undefined
+    await act(async () => {
+      resolvedNavigation = await secondNavigation!
+    })
+    expect(resolvedNavigation).toBe(true)
+  })
 })

apps/studio/hooks/ui/usePreventNavigationOnUnsavedChanges.ts

@@ -1,53 +1,72 @@
-import { useRouter } from 'next/router'
+import { useRouter as useTanStackRouter } from '@tanstack/react-router'
+import { useRouter as useNextRouter } from 'next/router'
 import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
 
 import { BASE_PATH } from '@/lib/constants'
 
 interface UsePreventNavigationOnUnsavedChangesOptions {
-  /*
-   * Boolean indicating whether there are changes that would be lost if users navigate to another
-   * page or close the browser tab
-   */
   hasChanges: boolean
 }
 
 interface UsePreventNavigationOnUnsavedChangesReturn {
-  /*
-   * Cancel the navigation and keep the changes
-   */
   handleCancelNavigation: () => void
-  /*
-   * Confirm the navigation and lose the changes
-   */
   handleConfirmNavigation: () => void
-  /*
-   * Skip the guard for the next intentional navigation after confirmation was handled elsewhere.
-   */
   bypassNavigationGuard: () => void
-  /*
-   * Boolean indicating whether UI to request users confirmation for the navigation should be
-   * displayed
-   */
   shouldConfirmNavigation: boolean
 }
 
+interface PendingTanStackNavigation {
+  proceed: () => void
+  reset: () => void
+}
+
 /*
- * Hook that prevents navigation when users could lose their changes.
- * It prevents both NextJS and browser navigation (such as when closing the tab)
+ * Prevents in-app navigation and tab close when users could lose changes.
+ *
+ * Studio ships Next and TanStack Router side by side. TanStack navigation must be blocked through
+ * its history API before the route starts rendering; Next continues to use routeChangeStart.
  */
 export const usePreventNavigationOnUnsavedChanges = ({
   hasChanges,
 }: UsePreventNavigationOnUnsavedChangesOptions): UsePreventNavigationOnUnsavedChangesReturn => {
-  const router = useRouter()
+  const nextRouter = useNextRouter()
+  const tanStackRouter = useTanStackRouter({ warn: false })
   const [navigateUrl, setNavigateUrl] = useState<string>()
   const [confirmNavigate, setConfirmNavigate] = useState(false)
+  const [pendingTanStackNavigation, setPendingTanStackNavigation] =
+    useState<PendingTanStackNavigation>()
   const bypassNavigationGuardRef = useRef(false)
 
   useEffect(() => {
-    const handleBeforeUnload = (e: BeforeUnloadEvent) => {
+    if (!tanStackRouter || !hasChanges) return
+
+    return tanStackRouter.history.block({
+      enableBeforeUnload: true,
+      blockerFn: async () => {
+        if (bypassNavigationGuardRef.current) {
+          bypassNavigationGuardRef.current = false
+          return false
+        }
+
+        const shouldCancelNavigation = await new Promise<boolean>((resolve) => {
+          setPendingTanStackNavigation({
+            proceed: () => resolve(false),
+            reset: () => resolve(true),
+          })
+        })
+        setPendingTanStackNavigation(undefined)
+        return shouldCancelNavigation
+      },
+    })
+  }, [hasChanges, tanStackRouter])
+
+  useEffect(() => {
+    if (tanStackRouter) return
+
+    const handleBeforeUnload = (event: BeforeUnloadEvent) => {
       if (hasChanges) {
-        e.preventDefault()
-        e.returnValue = '' // deprecated, but older browsers still require this
+        event.preventDefault()
+        event.returnValue = ''
       }
     }
 
@@ -60,26 +79,31 @@ export const usePreventNavigationOnUnsavedChanges = ({
 
       if (hasChanges && !confirmNavigate) {
         setNavigateUrl(url)
-        throw 'Route change declined' // Just to prevent the route change
-        return
+        throw 'Route change declined'
       }
       setNavigateUrl(undefined)
     }
+
     window.addEventListener('beforeunload', handleBeforeUnload)
-    router.events.on('routeChangeStart', handleBrowseAway)
+    nextRouter.events.on('routeChangeStart', handleBrowseAway)
 
     return () => {
       window.removeEventListener('beforeunload', handleBeforeUnload)
-      router.events.off('routeChangeStart', handleBrowseAway)
+      nextRouter.events.off('routeChangeStart', handleBrowseAway)
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [confirmNavigate, hasChanges])
+  }, [confirmNavigate, hasChanges, nextRouter.events, tanStackRouter])
 
   const handleCancelNavigation = useCallback(() => {
+    pendingTanStackNavigation?.reset()
     setNavigateUrl(undefined)
-  }, [])
+  }, [pendingTanStackNavigation])
 
   const handleConfirmNavigation = useCallback(() => {
+    if (pendingTanStackNavigation) {
+      pendingTanStackNavigation.proceed()
+      return
+    }
+
     setConfirmNavigate(true)
     let urlToNavigate = navigateUrl ?? '/'
     if (BASE_PATH && urlToNavigate.startsWith(BASE_PATH)) {
@@ -87,8 +111,8 @@ export const usePreventNavigationOnUnsavedChanges = ({
     }
     if (!urlToNavigate.startsWith('/')) urlToNavigate = `/${urlToNavigate}`
     setNavigateUrl(undefined)
-    router.push(urlToNavigate)
-  }, [navigateUrl, router])
+    nextRouter.push(urlToNavigate)
+  }, [navigateUrl, nextRouter, pendingTanStackNavigation])
 
   const bypassNavigationGuard = useCallback(() => {
     bypassNavigationGuardRef.current = true
@@ -99,8 +123,14 @@ export const usePreventNavigationOnUnsavedChanges = ({
       handleCancelNavigation,
       handleConfirmNavigation,
       bypassNavigationGuard,
-      shouldConfirmNavigation: !!navigateUrl,
+      shouldConfirmNavigation: pendingTanStackNavigation !== undefined || navigateUrl !== undefined,
     }),
-    [navigateUrl, handleCancelNavigation, handleConfirmNavigation, bypassNavigationGuard]
+    [
+      handleCancelNavigation,
+      handleConfirmNavigation,
+      bypassNavigationGuard,
+      navigateUrl,
+      pendingTanStackNavigation,
+    ]
   )
 }