Skip to content

Conversation

@schiller-manuel
Copy link
Contributor

@schiller-manuel schiller-manuel commented Dec 11, 2025

fixes #6068

Summary by CodeRabbit

  • Refactor
    • Consolidated internal context handling in the routing system to improve code consistency and reduce redundancy across multiple operations.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 11, 2025

Walkthrough

This PR refactors context aggregation in the router load pipeline by introducing buildMatchContext, a centralized utility that merges __routeContext and __beforeLoadContext from parent matches. This replaces scattered context construction logic across executeBeforeLoad, getLoaderContext, and the loadRouteMatch commit path, addressing issues with async beforeLoad context propagation in nested routes.

Changes

Cohort / File(s) Summary
Context aggregation refactoring
packages/router-core/src/load-matches.ts
Introduces buildMatchContext helper to centralize context merging logic. Replaces ad-hoc context construction in executeBeforeLoad (excludes current match's beforeLoadContext when building parent context), getLoaderContext (replaces manual accumulation loop), and loadRouteMatch commit path (uses buildMatchContext instead of merge loop). Updates beforeLoad pipeline to use merged context and clarifies when parent contexts are included or excluded.

Sequence Diagram

sequenceDiagram
    participant Router as Router
    participant BuildCtx as buildMatchContext
    participant Execute as executeBeforeLoad
    participant Loader as getLoaderContext
    participant Commit as loadRouteMatch Commit

    Router->>BuildCtx: buildMatchContext(matches, index)
    Note over BuildCtx: Merge parent matches<br/>__routeContext +<br/>__beforeLoadContext
    BuildCtx-->>Router: Aggregated context

    Router->>Execute: executeBeforeLoad with parent context
    Execute->>BuildCtx: buildMatchContext(exclude current beforeLoad)
    BuildCtx-->>Execute: Parent context only
    Execute->>Execute: beforeLoad pipeline
    Note over Execute: Context merged after<br/>beforeLoad resolution
    Execute-->>Router: Updated context

    Router->>Loader: getLoaderContext(index)
    Loader->>BuildCtx: buildMatchContext(all parent + current)
    BuildCtx-->>Loader: Full accumulated context
    Loader-->>Router: Loader context

    Router->>Commit: Commit route match
    Commit->>BuildCtx: buildMatchContext(inner, index)
    BuildCtx-->>Commit: Merged context for commit
    Note over Commit: Context consistency<br/>across nested routes
    Commit-->>Router: Committed
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Context merging logic: Verify buildMatchContext correctly accumulates __routeContext and __beforeLoadContext from parent matches without duplication
  • Async beforeLoad context propagation: Ensure child routes receive properly resolved parent context even when parent's beforeLoad is async (addresses #6068)
  • All call sites: Confirm all replacements (executeBeforeLoad, getLoaderContext, commit path) pass correct match indices and exclusion logic
  • Edge cases: Test scenarios where beforeLoad is skipped, resolves to undefined, or where context chains are deeply nested

Possibly related PRs

Suggested labels

package: router-core

Poem

🐰 A rabbit hops through nested routes with glee,
Building contexts cleanly, consistently!
No more undefined in the async beforeLoad chain—
Merged and stable, context flows like rain.
One helper function, no more tangled thread,
Parent contexts flow right to routes ahead! 🌿

Pre-merge checks and finishing touches

✅ 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 and concisely describes the main change: fixing context accumulation for all parent routes, which directly addresses the core issue of context becoming undefined in nested async beforeLoad scenarios.
Linked Issues check ✅ Passed The PR directly addresses issue #6068 by introducing buildMatchContext to consistently merge parent context from all router matches, ensuring context remains stable through the route chain including async beforeLoad scenarios.
Out of Scope Changes check ✅ Passed All changes are scoped to the load-matches.ts file and directly target context accumulation and merging logic, with no unrelated modifications detected.
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-6068

📜 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 7593e30 and c5e478b.

📒 Files selected for processing (1)
  • packages/router-core/src/load-matches.ts (5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TypeScript strict mode with extensive type safety for all code

Files:

  • packages/router-core/src/load-matches.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Implement ESLint rules for router best practices using the ESLint plugin router

Files:

  • packages/router-core/src/load-matches.ts
⏰ 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 (5)
packages/router-core/src/load-matches.ts (5)

53-74: Well-designed context aggregation utility.

The buildMatchContext function centralizes context accumulation logic, properly handling edge cases:

  • When index=0 and includeCurrentMatch=false, correctly returns only router options context
  • Gracefully handles missing matches with early continue statements
  • Uses Object.assign which naturally handles undefined values

This effectively addresses the async beforeLoad context propagation issue by ensuring parent contexts are always properly accumulated.


433-438: Correct context building for beforeLoad execution.

Using buildMatchContext(inner, index, false) properly accumulates all parent contexts (both __routeContext and __beforeLoadContext) while excluding the current match's __beforeLoadContext that hasn't been computed yet. Merging with match.__routeContext ensures the current match's route-level context is available.

This is the key fix for the async beforeLoad context propagation issue.


596-596: Correct use of buildMatchContext for loader context.

Using the default includeCurrentMatch=true is appropriate here since the loader executes after beforeLoad completes, meaning the current match's __beforeLoadContext should be included in the accumulated context.


761-766: Consistent context commit using centralized builder.

The commitContext function now uses buildMatchContext to ensure the match's exposed context property reflects the fully accumulated context from all ancestors. This is correctly called after loader completion (or skip) in both sync and async paths (lines 842 and 884).


468-491: Minor: Consider handling when beforeLoadContext is exactly undefined vs missing.

The updateContext function returns early when beforeLoadContext === undefined, which means no __beforeLoadContext is stored for this match. This is intentional (as per the comment), but worth noting that this differs from when beforeLoad returns an empty object {}.

The current behavior is correct for the fix, but ensure downstream consumers don't rely on __beforeLoadContext always being defined after beforeLoad runs.


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

@nx-cloud
Copy link

nx-cloud bot commented Dec 11, 2025

View your CI Pipeline Execution ↗ for commit c5e478b

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

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 11, 2025

More templates

@tanstack/arktype-adapter

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

@tanstack/directive-functions-plugin

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/server-functions-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/vue-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-router@6070

@tanstack/vue-router-devtools

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

@tanstack/vue-router-ssr-query

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

@tanstack/vue-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-start@6070

@tanstack/vue-start-client

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

@tanstack/vue-start-server

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

@tanstack/zod-adapter

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

commit: c5e478b

@schiller-manuel schiller-manuel merged commit 636a71c into main Dec 11, 2025
6 checks passed
@schiller-manuel schiller-manuel deleted the fix-6068 branch December 11, 2025 19:40
includeCurrentMatch: boolean = true,
): Record<string, unknown> => {
const context: Record<string, unknown> = {
...(inner.router.options.context ?? {}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undefined is spreadable, we can skip creating an object here

Suggested change
...(inner.router.options.context ?? {}),
...(inner.router.options.context),

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.

Nested context from async beforeLoad can be unexpected undefined. causing errors in child routes

3 participants