Skip to content

fix(router-core): onStay callback should use 'current' matches not 'previous' - #5485

Merged
schiller-manuel merged 2 commits into
TanStack:mainfrom
TeChn4K:patch-1
Nov 2, 2025
Merged

fix(router-core): onStay callback should use 'current' matches not 'previous'#5485
schiller-manuel merged 2 commits into
TanStack:mainfrom
TeChn4K:patch-1

Conversation

@TeChn4K

@TeChn4KTeChn4K commented Oct 15, 2025

Copy link
Copy Markdown
Contributor

This PR fixes#1901

onStay route hook currently gives the previous match instead of the current one.
Now, same as onEnter hook, you get the current match

Summary by CodeRabbit

  • Bug Fixes

    • More accurate detection of which views remain active during navigation, reducing unnecessary unmounts/reloads and preserving component state for smoother transitions.
    • Fewer edge cases when navigating between similar paths or parameter-only updates, improving perceived performance.
  • Tests

    • Added lifecycle callback tests to ensure onEnter/onLeave/onStay behaviors are consistent across navigations.

@coderabbitai

coderabbitaiBot commented Oct 15, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The stayingMatches computation in packages/router-core/src/router.ts was changed to derive stayingMatches by filtering newMatches for ids present in previousMatches (previously filtered previousMatches by ids in newMatches). A new test file for lifecycle callbacks was added. No other exports or error handling changes.

Changes

Cohort / File(s)Summary
Load transition matching logic
packages/router-core/src/router.ts
Rebased stayingMatches calculation to filter from newMatches against previousMatches by id (previously the inverse). Entering/exiting logic remains computed relative to the new stayingMatches.
Lifecycle callback tests
packages/router-core/tests/callbacks.test.ts
Added tests that verify RouterCore lifecycle callbacks (onEnter, onLeave, onStay) across navigations and search/query updates.

Sequence Diagram(s)

sequenceDiagram
autonumber
participant App
participant Router
participant Matches as Match Engine
App->>Router: navigate() / onReady()
Router->>Matches: compute previousMatches
Router->>Matches: compute newMatches
note right of Router: Old flow
Router->>Router: stayingMatches = previousMatches ∩ newMatches (by id)
Router->>Router: enteringMatches = newMatches - stayingMatches
Router->>Router: exitingMatches = previousMatches - stayingMatches
Router-->>App: invoke onEnter/onStay/onLeave
Loading
sequenceDiagram
autonumber
participant App
participant Router
participant Matches as Match Engine
App->>Router: navigate() / onReady()
Router->>Matches: compute previousMatches
Router->>Matches: compute newMatches
note right of Router: New flow (this PR)
Router->>Router: stayingMatches = newMatches ∩ previousMatches (by id)
Router->>Router: enteringMatches = newMatches - stayingMatches
Router->>Router: exitingMatches = previousMatches - stayingMatches
Router-->>App: invoke onEnter/onStay/onLeave
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Areas to review closely:
    • packages/router-core/src/router.ts — ensure the new filter direction preserves semantics for onStay and related callbacks.
    • packages/router-core/tests/callbacks.test.ts — validate test coverage for parameter/search updates and callback argument shapes.

Possibly related PRs

Suggested reviewers

  • schiller-manuel

Poem

I hop between the routes so spry,
Now "stay" reads what is nigh.
New matches meet old by ID,
No stale params will bother me.
Paws on paths, I cheer the fix—hooray! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title Check✅ PassedThe pull request title "fix(router-core): onStay callback should use 'current' matches not 'previous'" is directly and specifically related to the main change in the changeset. The title concisely describes the fix being applied—changing how stayingMatches are derived to use newMatches instead of previousMatches so that the onStay callback receives current match values rather than previous ones. The title is clear, avoids vague terminology, and directly reflects the primary objective of the PR without unnecessary noise.
Linked Issues Check✅ PassedThe changes directly address the requirements specified in issue #1901. The code modification in router.ts implements the suspected fix by changing stayingMatches to be derived from newMatches instead of previousMatches, which ensures the onStay callback receives current match values including updated search parameters. The new test file (callbacks.test.ts) validates this behavior by verifying that onStay correctly receives updated search parameters when navigating to the same route, confirming that the fix aligns with the expected behavior outlined in the linked issue.
Out of Scope Changes Check✅ PassedAll changes in this pull request are directly scoped to the objective of fixing the onStay callback behavior. The modification to router.ts targets the specific calculation of stayingMatches in the onReady load path, directly addressing the root cause identified in issue #1901. The new test file validates the fix by testing the callback behavior with updated search parameters. No unrelated refactoring, broad changes, or out-of-scope functionality appear to have been introduced.
✨ 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 2b27ca2 and decb4dd.

📒 Files selected for processing (1)
  • packages/router-core/tests/callbacks.test.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/router-core/tests/callbacks.test.ts

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.

@SeanCassiereSeanCassiere left a comment

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.

Could you please add a test to make sure this behaviour is being enforced?

@SeanCassiereSeanCassiere changed the title fix: onStay hook use current matches not previousfix(router-core): onStay callback should use 'current' matches not 'previous'Oct 16, 2025
@nx-cloud

nx-cloudBot commented Oct 16, 2025

Copy link
Copy Markdown
Contributor

View your CI Pipeline Execution ↗ for commit decb4dd

CommandStatusDurationResult
nx affected --targets=test:eslint,test:unit,tes...✅ Succeeded7m 51sView ↗
nx run-many --target=build --exclude=examples/*...✅ Succeeded1m 24sView ↗

☁️ Nx Cloud last updated this comment at 2025-11-02 01:04:05 UTC

@pkg-pr-new

pkg-pr-newBot commented Oct 16, 2025

Copy link
Copy Markdown
More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@5485

@tanstack/directive-functions-plugin

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

@tanstack/eslint-plugin-router

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

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@5485

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@5485

@tanstack/router-vite-plugin

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

@tanstack/server-functions-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@5485

@tanstack/virtual-file-routes

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

@tanstack/zod-adapter

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

commit: decb4dd

@TeChn4K

Copy link
Copy Markdown
ContributorAuthor

Could you please add a test to make sure this behaviour is being enforced?

Added! @SeanCassiere
I was not sure where to write them, so I created a new callbacks.test.ts. Feel free to update to match your styles and conventions

@schiller-manuel
schiller-manuel merged commit b865b42 into TanStack:mainNov 2, 2025
6 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

onStay property should use current matches, not previous

3 participants

@TeChn4K@SeanCassiere@schiller-manuel