router icon indicating copy to clipboard operation
router copied to clipboard

fix: queue pushpop actions

Open tomrehnstrom opened this issue 1 month ago β€’ 3 comments

When running push/pop actions like back, forward or go in quick succession the ignoreBlocker functionality does not work since the event which flips back the ignore flag does not run right away and the second call to back for example runs before the event meaning the flag is not reset correctly when the second pushpop event is called. This PR fixes this by implementing a promise based queue system

It also ensure that ignoreBlocker also works with go

Summary by CodeRabbit

  • Improvements

    • Enhanced scheduling of back/forward/go navigation to make transitions more reliable and to avoid races.
    • Improved coordination with navigation blockers and state synchronization for consistent history behavior.
    • Added an option to bypass blockers for certain navigations when needed.
  • Breaking Changes

    • Public navigation API signature updated; create-memory-history options signature adjusted.

✏️ Tip: You can customize this high-level summary in your review settings.

tomrehnstrom avatar Dec 04 '25 11:12 tomrehnstrom

View your CI Pipeline Execution β†— for commit d8e1933750cad5c0d52d78e343d2bad183659407

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... βœ… Succeeded 8m 31s View β†—
nx run-many --target=build --exclude=examples/*... βœ… Succeeded 1m 27s View β†—

☁️ Nx Cloud last updated this comment at 2025-12-04 11:50:39 UTC

nx-cloud[bot] avatar Dec 04 '25 11:12 nx-cloud[bot]

Walkthrough

Adds an optional ignoreBlocker boolean to the public go method, introduces a scheduler to serialize/backpressure BACK/FORWARD/GO browser actions, reworks pop handling to coordinate blockers and synthetic recovery navigations, and inlines the createMemoryHistory opts type.

Changes

Cohort / File(s) Summary
History core β€” scheduling & blockers
packages/history/src/index.ts
Public go(n, ignoreBlocker) signature added. Introduces schedulePushPopAction to serialize/coalesce back/forward/go browser actions; adds state: scheduledPushPopAction, resolveScheduledPushPopAction, nextOnPushPop. Reworks onPushPopEvent to consult scheduled state, evaluate blockers, perform synthetic corrective history.go(...) when blocked, and ensure scheduling state is reset in a finally block.
API surface / types
packages/history/src/index.ts
createMemoryHistory opts type changed to an inline { initialEntries: Array<string>; initialIndex?: number }. Public history input type for createHistory updates go signature to accept ignoreBlocker. Minor refactor/formatting adjustments and local type tightening.

Sequence Diagram

sequenceDiagram
    participant App as Application
    participant History as History API
    participant Scheduler as Scheduler
    participant Browser as Browser History
    participant Blockers as Blocker Chain
    participant Subscribers as Subscribers

    App->>History: call go/back/forward (maybe ignoreBlocker)
    History->>Scheduler: schedulePushPopAction(action, delta, ignoreBlocker)
    Scheduler->>Browser: perform browser.history call (push/pop/go) [serialized]
    Browser->>History: popstate -> onPushPopEvent()
    History->>Scheduler: consult nextOnPushPop / scheduled state

    alt skip blocker (ignoreBlocker / skip flag)
        History->>Subscribers: notify subscribers (no blocker evaluation)
    else
        History->>Blockers: evaluate blocker chain
        alt blockers allow
            History->>Subscribers: notify subscribers
        else blockers block
            History->>Browser: synthetic history.go(recoveryDelta) to revert
            History->>History: set ignoreNextPop and update state
            History->>Subscribers: notify of blocked outcome
        end
    end

    Scheduler->>Scheduler: finally: reset scheduledPushPopAction / resolveScheduledPushPopAction / nextOnPushPop

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Review scheduling/state machine correctness: scheduledPushPopAction, resolveScheduledPushPopAction, nextOnPushPop, ignoreNextPop.
  • Verify blocker evaluation logic and synthetic recovery history.go(...) avoid event loops and maintain consistent indices.
  • Confirm public API/type change (go signature, createMemoryHistory opts) is compatible with external callers and documentation.

Poem

🐰 I hopped a step and timed the tide,

I queued my moves and let blockers hide.
A gentle nudge when paths were stuck,
Schedules hum and subscribers luck.
Hop on β€” history's neat and spry.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
βœ… Passed checks (2 passed)
Check name Status Explanation
Description Check βœ… Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check βœ… Passed The title 'fix: queue pushpop actions' directly addresses the main change: implementing a queue system for serializing push/pop navigation actions to fix the ignoreBlocker bypass bug.
✨ Finishing touches
  • [ ] πŸ“ Generate docstrings
πŸ§ͺ Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

πŸ“œ Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between f1723d15b2075fccfc8c09c2bfef587485e468b8 and d8e1933750cad5c0d52d78e343d2bad183659407.

πŸ“’ Files selected for processing (1)
  • packages/history/src/index.ts (5 hunks)
🧰 Additional context used
πŸ““ Path-based instructions (1)
**/*.{ts,tsx}

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Use TypeScript strict mode with extensive type safety throughout the codebase

Files:

  • packages/history/src/index.ts
πŸ”‡ Additional comments (6)
packages/history/src/index.ts (6)

99-115: LGTM - Signature updates for ignoreBlocker propagation.

The ignoreBlocker parameter is now consistently required across go, back, and forward in the internal createHistory opts type, enabling proper blocker bypass semantics throughout navigation paths.


204-213: LGTM - ignoreBlocker correctly propagated to opts.go.

The ignoreBlocker option is properly extracted and passed through to the underlying history implementation.


396-428: Queue serialization looks correct after the race condition fix.

The chained action path (lines 416-426) now properly updates scheduledPushPopAction with a new promise, ensuring subsequent callers see the correct queued state. The finally block cleanup at lines 484-491 will correctly reset state after each action completes.

Minor observation: ignoreNextBeforeUnload is set immediately (line 406) even when an action is queued, which means a later-queued action's ignoreBlocker value will overwrite an earlier one. This seems acceptable since beforeUnload events are edge cases and the final state reflects the latest user intent.


535-549: LGTM - Consistent integration with the scheduling layer.

The back, forward, and go methods correctly integrate with schedulePushPopAction, properly distinguishing isGo for accurate action type detection in onPushPopEvent.


484-491: LGTM - Finally block safely resets scheduling state.

The conditional check on resolveScheduledPushPopAction ensures the cleanup only runs when there's an active scheduled action. This correctly handles the case where the corrective navigation's popstate is ignored via ignoreNextPop.


472-476: Verify the notification behavior when navigation is blocked.

After blocking navigation with go(-delta), the code calls history.notify(notify) with the original action type. Subscribers need to verify this notification is interpreted correctlyβ€”if subscribers treat this as "navigation completed," it could cause state inconsistencies. Clarify whether this notification is intentional for blocker UI updates or if it should indicate a blocked/failed navigation instead.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❀️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Dec 04 '25 11:12 coderabbitai[bot]

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@6019
@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@6019
@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@6019
@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@6019
@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/nitro-v2-vite-plugin@6019
@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@6019
@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@6019
@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@6019
@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@6019
@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@6019
@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@6019
@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@6019
@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@6019
@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@6019
@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@6019
@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@6019
@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@6019
@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@6019
@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@6019
@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@6019
@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@6019
@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@6019
@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@6019
@tanstack/solid-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-ssr-query@6019
@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@6019
@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@6019
@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@6019
@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@6019
@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@6019
@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@6019
@tanstack/start-static-server-functions

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-static-server-functions@6019
@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@6019
@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@6019
@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@6019
@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@6019

commit: d8e1933

pkg-pr-new[bot] avatar Dec 04 '25 11:12 pkg-pr-new[bot]