Skip to content

Conversation

@birkskyum
Copy link
Member

@birkskyum birkskyum commented Nov 9, 2025

Closes #5749

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of chained synchronous navigation calls to prevent state inconsistencies.
    • Fixed an issue where parallel navigate calls with search parameters could cause unexpected behavior.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 9, 2025

Walkthrough

RouterCore now tracks a pending location state during navigation builds via the new pendingBuiltLocation field. The buildLocation method prioritizes this pending location over the latest committed location when determining the current location context. Microtask-based cleanup logic resets the pending state after commit initiation, enabling chained synchronous navigate calls. A previously skipped test validating parallel navigate calls with search parameters has been re-enabled with initialization synchronization.

Changes

Cohort / File(s) Change Summary
Pending Location State Management
packages/router-core/src/router.ts
Adds public pendingBuiltLocation field to RouterCore; updates buildLocation to check pending state before latest location; modifies build-and-commit flow to set pending location, then clear via microtask after commit begins.
Test Re-enablement
packages/solid-router/tests/useNavigate.test.tsx
Converts skipped test case for parallel navigate calls to active test; adds initialization wait (screen.findByTestId('param1')) to ensure router readiness before assertions.

Sequence Diagram

sequenceDiagram
    participant App
    participant Router as RouterCore
    participant Task as Microtask Queue

    App->>Router: buildAndCommitLocation() called
    Router->>Router: set pendingBuiltLocation = newLocation
    Router->>Router: call commitLocation()
    Note over Router: Commit begins synchronously
    Router-->>Task: schedule microtask: clear pendingBuiltLocation
    
    rect rgb(200, 220, 255)
    Note over Router: buildLocation called during<br/>pendingBuiltLocation window
    Router->>Router: return pendingBuiltLocation<br/>(not latestLocation yet)
    end
    
    Task->>Router: execute: pendingBuiltLocation = undefined
    Note over Router: Pending state cleared,<br/>latestLocation now current
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify microtask cleanup timing ensures no stale pending state persists across navigation calls
  • Confirm buildLocation priority logic (pending → latest) is consistent with intended semantics
  • Validate that the test re-enablement with initialization wait properly addresses the parallel navigate scenario

Possibly related PRs

Suggested reviewers

  • schiller-manuel

Poem

🐰 A pending state hops into view,
Before the commit makes dreams come true,
Microtasks clean with gentle grace,
Parallel paths find their place! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly reflects the main problem being fixed: enabling a test for parallel navigate calls with search params and implementing the underlying fix in RouterCore with pendingBuiltLocation logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix(solid-router)--setting-search-params-with-2-parallel-navigate-calls

📜 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 cfe7ee9 and c29874d.

📒 Files selected for processing (2)
  • packages/router-core/src/router.ts (3 hunks)
  • packages/solid-router/tests/useNavigate.test.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript in strict mode with extensive type safety across the codebase

Files:

  • packages/solid-router/tests/useNavigate.test.tsx
  • packages/router-core/src/router.ts
packages/{react-router,solid-router}/**

📄 CodeRabbit inference engine (AGENTS.md)

Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/

Files:

  • packages/solid-router/tests/useNavigate.test.tsx
packages/router-core/**

📄 CodeRabbit inference engine (AGENTS.md)

Keep framework-agnostic core router logic in packages/router-core/

Files:

  • packages/router-core/src/router.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.

Applied to files:

  • packages/solid-router/tests/useNavigate.test.tsx
  • packages/router-core/src/router.ts
📚 Learning: 2025-10-14T18:59:33.990Z
Learnt from: FatahChan
Repo: TanStack/router PR: 5475
File: e2e/react-start/basic-prerendering/src/routes/redirect/$target/via-beforeLoad.tsx:8-0
Timestamp: 2025-10-14T18:59:33.990Z
Learning: In TanStack Router e2e test files, when a route parameter is validated at the route level (e.g., using zod in validateSearch or param validation), switch statements on that parameter do not require a default case, as the validation ensures only expected values will reach the switch.

Applied to files:

  • packages/solid-router/tests/useNavigate.test.tsx
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/

Applied to files:

  • packages/solid-router/tests/useNavigate.test.tsx
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.

Applied to files:

  • packages/router-core/src/router.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-core/** : Keep framework-agnostic core router logic in packages/router-core/

Applied to files:

  • packages/router-core/src/router.ts
🧬 Code graph analysis (1)
packages/router-core/src/router.ts (1)
packages/router-core/src/routeInfo.ts (1)
  • FullSearchSchema (212-215)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Test
  • GitHub Check: Preview
🔇 Additional comments (4)
packages/solid-router/tests/useNavigate.test.tsx (1)

1256-1325: LGTM! Test re-enabled with proper initialization synchronization.

The test correctly validates the parallel navigate scenario described in the PR objectives. The added initialization wait (lines 1304-1305) ensures that router defaults from validateSearch are applied before assertions, which is necessary given the new pendingBuiltLocation mechanism that tracks in-flight location builds.

The test structure properly exercises the fix:

  • Two synchronous navigate calls update different search params (lines 1270-1278)
  • Assertions verify both updates are preserved in the final location (lines 1319-1324)
packages/router-core/src/router.ts (3)

899-899: LGTM! Clear field addition for tracking pending location state.

The pendingBuiltLocation field appropriately tracks locations being built during navigation but not yet committed, enabling synchronous navigate calls to chain correctly.


1597-1598: LGTM! Correct precedence for chaining parallel navigations.

The precedence order correctly enables synchronous navigate calls to build on pending state:

  1. dest._fromLocation - explicit override
  2. this.pendingBuiltLocation - in-flight location from a previous synchronous navigate call
  3. this.latestLocation - fallback to committed state

This ensures that when two navigate calls execute synchronously (e.g., lines 1270-1278 in the test), the second call builds on the first's pending location rather than both starting from the same committed location.


1961-1982: LGTM! Elegant microtask-based cleanup with defensive checks.

The implementation correctly enables chaining while preventing stale state:

  1. Set pending state before commit (lines 1961-1963): Stores the built location so subsequent synchronous navigate calls can chain
  2. Microtask-based cleanup (lines 1974-1980): Clears pending state after synchronous execution completes, using Promise.resolve().then()
  3. Defensive equality check (line 1977): Only clears if pendingBuiltLocation still points to the same location, preventing earlier navigations from clearing state set by later ones

Timing guarantee: Synchronous navigate calls execute before microtasks run, ensuring all calls in a synchronous batch can read and chain off pendingBuiltLocation before it's cleared.


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.

@nx-cloud
Copy link

nx-cloud bot commented Nov 9, 2025

View your CI Pipeline Execution ↗ for commit c29874d

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

☁️ Nx Cloud last updated this comment at 2025-11-09 14:53:21 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 9, 2025

More templates

@tanstack/arktype-adapter

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

@tanstack/directive-functions-plugin

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/server-functions-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/zod-adapter

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

commit: c29874d

@birkskyum birkskyum merged commit da77078 into main Nov 9, 2025
6 checks passed
@birkskyum birkskyum deleted the fix(solid-router)--setting-search-params-with-2-parallel-navigate-calls branch November 9, 2025 15:09
roduyemi pushed a commit to roduyemi/oss-router that referenced this pull request Nov 19, 2025
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.

solid-router - parallel navigation override search params

2 participants