Skip to content

Conversation

@TeChn4K
Copy link
Contributor

@TeChn4K TeChn4K commented Oct 15, 2025

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
Copy link
Contributor

coderabbitai bot commented Oct 15, 2025

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 name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring 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 name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The 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 ✅ Passed The 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 ✅ Passed All 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.

Copy link
Member

@SeanCassiere SeanCassiere left a comment

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?

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

nx-cloud bot commented Oct 16, 2025

View your CI Pipeline Execution ↗ for commit decb4dd

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 7m 51s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1m 24s View ↗

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 16, 2025

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
Contributor Author

TeChn4K commented Nov 1, 2025

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:main Nov 2, 2025
6 checks passed
Sign up for free to 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