Skip to content

fix(router-core): fire onBeforeNavigate after beforeLoad redirect#7808

Open
ChihebBENCHEIKH1 wants to merge 1 commit into
TanStack:mainfrom
ChihebBENCHEIKH1:fix/on-before-navigate-after-redirect
Open

fix(router-core): fire onBeforeNavigate after beforeLoad redirect#7808
ChihebBENCHEIKH1 wants to merge 1 commit into
TanStack:mainfrom
ChihebBENCHEIKH1:fix/on-before-navigate-after-redirect

Conversation

@ChihebBENCHEIKH1

@ChihebBENCHEIKH1 ChihebBENCHEIKH1 commented Jul 13, 2026

Copy link
Copy Markdown

Problem

onBeforeNavigate was gated on !this.stores.redirect.get() in router-core's load(). stores.redirect is populated whenever beforeLoad throws redirect(...) (line 2629) and never cleared, so every navigation after the first in-beforeLoad redirect silently skipped the onBeforeNavigate emit. The neighbouring onBeforeLoad emit has no such gate and kept firing correctly.

Real-world impact: Sentry's TanStack Router tracing integration stops producing navigation spans once any route ever redirects in beforeLoad (getsentry/sentry-javascript#21966), and userland patterns that hook onBeforeNavigate (e.g. form-state persistence) go silent after the first redirect.

Fix

Remove the state.redirect gate around the onBeforeNavigate emit so it matches onBeforeLoad and fires on every navigation. stores.redirect is still populated for createRequestHandler (SSR redirect headers) — nothing else reads it.

Tests

Added fires on subsequent navigations after a redirect in beforeLoad (#3920) in packages/router-core/tests/callbacks.test.ts: sets up /foo whose beforeLoad throws redirect({ to: '/bar' }), then navigates /foo/baz and asserts onBeforeNavigate still fires on /baz. Fails on main, passes with this change. Full @tanstack/router-core and @tanstack/react-router unit + type + eslint suites are green.

Fixes #3920.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed navigation lifecycle events not firing after a route redirect.
    • onBeforeNavigate now consistently runs for every navigation, including navigations following a beforeLoad redirect.
    • Improved compatibility with integrations that rely on navigation events, such as tracing and monitoring tools.

The `onBeforeNavigate` emission was gated on `stores.redirect` being
unset, but that store was populated whenever `beforeLoad` threw
`redirect(...)` and never cleared. Every navigation after the first
in-`beforeLoad` redirect silently skipped `onBeforeNavigate` — the
neighboring `onBeforeLoad` emission has no such gate.

Remove the gate so `onBeforeNavigate` matches `onBeforeLoad` and fires
on every navigation, including navigations that follow a redirect.

Fixes TanStack#3920.
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0f600f92-a846-4bc0-84ce-377747771f46

📥 Commits

Reviewing files that changed from the base of the PR and between 41f7bf3 and d0afeb1.

📒 Files selected for processing (3)
  • .changeset/fix-on-before-navigate-after-redirect.md
  • packages/router-core/src/router.ts
  • packages/router-core/tests/callbacks.test.ts

📝 Walkthrough

Walkthrough

RouterCore now emits onBeforeNavigate for every navigation, including navigations after a beforeLoad redirect. A regression test verifies continued emissions on later navigation, and a changeset documents the patch release.

Changes

Router core navigation lifecycle

Layer / File(s) Summary
Unconditional navigation event emission
packages/router-core/src/router.ts, .changeset/fix-on-before-navigate-after-redirect.md
onBeforeNavigate is emitted without gating on the router’s redirect store, and the changeset records the behavioral fix.
Redirect navigation regression coverage
packages/router-core/tests/callbacks.test.ts
Tests verify onBeforeNavigate during a beforeLoad redirect and during later unrelated navigation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: package: router-core

Suggested reviewers: sheraff, sleitor, schiller-manuel, nlynzaad, birkskyum

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the core fix: firing onBeforeNavigate after a beforeLoad redirect.
Linked Issues check ✅ Passed The router change removes the redirect gate and the regression test covers the #3920 behavior fix.
Out of Scope Changes check ✅ Passed The changeset and test updates are directly related to the redirect bug fix and do not appear out of scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

onBeforeNavigate not invoked after redirect within beforeLoad method was invoked

1 participant