From 2dd7b102049a64d072411e7a03cae2adbac732c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 04:34:33 +0000 Subject: [PATCH] fix: skip blockers on non-cancelable navigate events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the Navigation API spec, some navigate events are not cancelable (notably certain back/forward traversals, depending on browser and user activation). Calling preventDefault() on them is ignored, so blockers would run shouldBlock() — possibly showing a confirm() dialog — while the navigation proceeded regardless of the user's answer. Check event.cancelable before running blockers so shouldBlock() is skipped entirely when the navigation cannot be prevented, and document the caveat in the useBlocker docs. Fixes #211 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JBczYkJZ5sdLWD4ue9zy37 --- packages/docs/src/pages/ApiHooksPage.tsx | 6 ++++ packages/router/docs/useBlocker-design.md | 14 +++++++-- packages/router/src/__tests__/setup.ts | 17 ++++++++-- .../router/src/__tests__/useBlocker.test.tsx | 31 +++++++++++++++++++ .../router/src/core/NavigationAPIAdapter.ts | 8 +++-- packages/router/src/hooks/useBlocker.ts | 5 +++ 6 files changed, 73 insertions(+), 8 deletions(-) diff --git a/packages/docs/src/pages/ApiHooksPage.tsx b/packages/docs/src/pages/ApiHooksPage.tsx index 0fa14f9..544c188 100644 --- a/packages/docs/src/pages/ApiHooksPage.tsx +++ b/packages/docs/src/pages/ApiHooksPage.tsx @@ -113,6 +113,12 @@ function EditForm() { This hook only handles SPA navigations (links, programmatic navigation). For hard navigations (tab close, refresh), handle beforeunload separately. +
  • + Some navigate events are not cancelable — notably certain browser back/forward + traversals, depending on the browser and user activation. For those,{" "} + shouldBlock is not called and the navigation proceeds, since the Navigation + API provides no way to cancel it. +
    • diff --git a/packages/router/docs/useBlocker-design.md b/packages/router/docs/useBlocker-design.md index 768e7b8..cea3454 100644 --- a/packages/router/docs/useBlocker-design.md +++ b/packages/router/docs/useBlocker-design.md @@ -67,11 +67,15 @@ When navigation occurs: For navigations within the app (links, programmatic navigation): 1. Navigation event fires -2. All registered blocker functions are called synchronously -3. If any returns `true`: +2. If the event is not cancelable (`event.cancelable === false`), blockers are + skipped entirely and navigation proceeds — `preventDefault()` would be + ignored, so running blockers (and possibly showing a `confirm()` dialog) + would be misleading +3. All registered blocker functions are called synchronously +4. If any returns `true`: - Navigation is prevented (`event.preventDefault()`) - User remains on current page -4. If all return `false`: +5. If all return `false`: - Navigation proceeds normally ### Hard Navigations @@ -226,6 +230,10 @@ Some navigations cannot or should not be blocked: - Same-page hash changes (anchor links) - `replace` navigations that don't change the path (state-only updates) - Hard navigations (tab close, external URL) - use `beforeunload` separately +- Non-cancelable navigate events — per the Navigation API spec, some events + (notably certain back/forward traversals, depending on browser and user + activation) have `cancelable: false`. Blockers are skipped for these since + the navigation cannot be prevented ## Comparison with `onNavigate` diff --git a/packages/router/src/__tests__/setup.ts b/packages/router/src/__tests__/setup.ts index 21e89f8..71d9570 100644 --- a/packages/router/src/__tests__/setup.ts +++ b/packages/router/src/__tests__/setup.ts @@ -76,11 +76,13 @@ export function createMockNavigation(initialUrl = "http://localhost/") { eventInfo?: unknown, eventFormData?: FormData | null, eventNavigationType?: "push" | "replace" | "reload" | "traverse", + eventCancelable = true, ): NavigateEvent & { defaultPrevented: boolean } => { let defaultPrevented = false; return { type: "navigate", canIntercept: true, + cancelable: eventCancelable, hashChange: false, destination: { url: destinationUrl, @@ -103,7 +105,10 @@ export function createMockNavigation(initialUrl = "http://localhost/") { return defaultPrevented; }, preventDefault: vi.fn(() => { - defaultPrevented = true; + // Mimic DOM behavior: preventDefault is ignored on non-cancelable events + if (eventCancelable) { + defaultPrevented = true; + } }), } as unknown as NavigateEvent & { defaultPrevented: boolean }; }; @@ -194,13 +199,19 @@ export function createMockNavigation(initialUrl = "http://localhost/") { // This allows testing of onNavigate callback behavior __simulateNavigationWithEvent( url: string, - options?: { info?: unknown; formData?: FormData }, + options?: { info?: unknown; formData?: FormData; cancelable?: boolean }, ): { event: NavigateEvent & { defaultPrevented: boolean }; proceed: () => void; } { const newUrl = new URL(url, currentEntry.url).href; - const event = createMockNavigateEvent(newUrl, options?.info, options?.formData); + const event = createMockNavigateEvent( + newUrl, + options?.info, + options?.formData, + undefined, + options?.cancelable, + ); // Dispatch navigate event first (allows onNavigate to be called) dispatchEvent("navigate", event); diff --git a/packages/router/src/__tests__/useBlocker.test.tsx b/packages/router/src/__tests__/useBlocker.test.tsx index 0a982bb..8516700 100644 --- a/packages/router/src/__tests__/useBlocker.test.tsx +++ b/packages/router/src/__tests__/useBlocker.test.tsx @@ -72,6 +72,37 @@ describe("useBlocker", () => { expect(event.preventDefault).toHaveBeenCalled(); }); + it("skips shouldBlock when the navigate event is not cancelable", () => { + const shouldBlock = vi.fn(() => true); + + function TestComponent() { + useBlocker({ shouldBlock }); + return
    Home
    ; + } + + const routes: RouteDefinition[] = [ + { path: "/", component: TestComponent }, + { path: "/about", component: () =>
    About
    }, + ]; + + render(); + expect(screen.getByText("Home")).toBeInTheDocument(); + + // Simulate a non-cancelable navigation (e.g. a browser traversal that + // cannot be canceled). preventDefault() would be ignored, so the blocker + // must not run — otherwise a confirm() dialog would be shown whose + // answer cannot be honored. + const { event } = mockNavigation.__simulateNavigationWithEvent("/about", { + cancelable: false, + }); + + expect(shouldBlock).not.toHaveBeenCalled(); + expect(event.preventDefault).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(false); + // Navigation is still handled normally (route matched, so intercepted) + expect(event.intercept).toHaveBeenCalled(); + }); + it("blocks navigation based on dynamic condition", async () => { function TestComponent() { const [isDirty, setIsDirty] = useState(false); diff --git a/packages/router/src/core/NavigationAPIAdapter.ts b/packages/router/src/core/NavigationAPIAdapter.ts index 5bffb7f..0ae2da1 100644 --- a/packages/router/src/core/NavigationAPIAdapter.ts +++ b/packages/router/src/core/NavigationAPIAdapter.ts @@ -288,8 +288,12 @@ export class NavigationAPIAdapter implements RouterAdapter { // Invalidate cached snapshot to pick up new info this.#cachedSnapshot = null; - // Check blockers first - if any blocker returns true, prevent navigation - if (checkBlockers?.()) { + // Check blockers first - if any blocker returns true, prevent navigation. + // Some navigate events are not cancelable (e.g. certain traversals, + // depending on browser and user activation); preventDefault() would be + // ignored on them, so skip shouldBlock() entirely rather than show a + // confirmation dialog whose answer cannot be honored. + if (event.cancelable && checkBlockers?.()) { event.preventDefault(); return; } diff --git a/packages/router/src/hooks/useBlocker.ts b/packages/router/src/hooks/useBlocker.ts index 4f5b4e9..54b8a1b 100644 --- a/packages/router/src/hooks/useBlocker.ts +++ b/packages/router/src/hooks/useBlocker.ts @@ -35,6 +35,11 @@ export type UseBlockerOptions = { * * Note: This hook only handles SPA navigations (links, programmatic navigation). * For hard navigations (tab close, refresh), handle `beforeunload` separately. + * + * Note: Some navigate events are not cancelable — notably certain browser + * back/forward traversals, depending on the browser and user activation. + * For those, `shouldBlock` is not called and the navigation proceeds, + * since the Navigation API provides no way to cancel it. */ export function useBlocker(options: UseBlockerOptions): void { const context = useContext(BlockerContext);