Skip to content

Conversation

@Sheraff
Copy link
Contributor

@Sheraff Sheraff commented Nov 15, 2025

Summary by CodeRabbit

  • New Features

    • Added optional parameter encoding control to path interpolation, allowing path parameters to be used as raw values without encoding when configured, providing greater flexibility in route handling.
  • Improvements

    • Enhanced hash extraction logic in route location parsing.
    • Refined path parameter handling during route construction for improved consistency.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 15, 2025

Walkthrough

The changes introduce an optional encode flag to InterpolatePathOptions to control parameter encoding during path interpolation. Additionally, hash extraction logic is simplified in parseLocation, and buildLocation refactors its encoding strategy to use unencoded interpolation instead of post-processing with decodePath.

Changes

Cohort / File(s) Summary
Path interpolation encoding control
packages/router-core/src/path.ts
Added optional encode?: boolean property to InterpolatePathOptions interface. Modified interpolatePath to conditionally apply encodeParam based on the flag, affecting _splat wildcards, named parameters, and optional parameters.
Router location building updates
packages/router-core/src/router.ts
Simplified hash extraction in parseLocation from hash.split('#').reverse()[0] to last(hash.split('#')). Updated buildLocation to call interpolatePath with encode: false instead of applying decodePath post-interpolation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

  • Review the conditional encoding logic in interpolatePath to ensure it correctly applies/skips encoding for all parameter types
  • Verify that switching from post-interpolation decodePath to encode: false flag produces equivalent results
  • Confirm the simplified hash extraction logic maintains parity with the previous implementation

Possibly related PRs

Suggested labels

package: router-core

Suggested reviewers

  • nlynzaad
  • schiller-manuel

Poem

🐰 With paths to encode or leave bare,
The router hops with careful care,
A flag to guide the encoding way,
Hash extraction's cleaner today,
Through encoded lands, the router'll dare! ✨

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 change: refactoring buildLocation to avoid decoding when encoding is skipped, which is the core optimization in the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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 refactor-router-core-build-location-skip-encode-decode-pathname

📜 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 cdd2de3 and 2c7c08c.

📒 Files selected for processing (2)
  • packages/router-core/src/path.ts (5 hunks)
  • packages/router-core/src/router.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
📚 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-22T00:56:53.426Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.

Applied to files:

  • packages/router-core/src/router.ts
📚 Learning: 2025-09-22T00:56:49.237Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.

Applied to files:

  • packages/router-core/src/router.ts
🧬 Code graph analysis (1)
packages/router-core/src/router.ts (2)
packages/router-core/src/utils.ts (1)
  • last (191-193)
packages/router-core/src/path.ts (1)
  • interpolatePath (249-357)
⏰ 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: Preview
  • GitHub Check: Test
🔇 Additional comments (7)
packages/router-core/src/path.ts (5)

208-215: LGTM! Clear documentation for the new encode option.

The optional encode parameter is well-documented and maintains backward compatibility.


249-254: LGTM! Backward-compatible parameter addition.

The encode = true default ensures existing behavior is preserved while allowing callers to opt into skipping encoding.


304-306: LGTM! Conditional encoding for wildcard parameters.

The logic correctly applies encoding based on the encode flag.


320-322: LGTM! Consistent conditional encoding for named parameters.

The encoding logic is consistent with the wildcard implementation and properly handles the fallback value.


345-346: LGTM! Consistent conditional encoding for optional parameters.

The encoding logic follows the same pattern as other parameter types.

packages/router-core/src/router.ts (2)

1187-1187: LGTM! Cleaner hash extraction.

Using last(hash.split('#')) is more efficient than reverse()[0] and produces identical results.


1680-1688: Verify parameter safety with unencoded interpolation.

The refactor skips encoding during interpolation to avoid an unnecessary encode/decode round-trip. However, please verify that parameters containing special URL characters (#, ?, &, /) are properly handled or prevented upstream.

For example, if a param value contains #, the unencoded pathname /route/foo#bar would break when constructing fullPath at line 1748, as the # would be interpreted as a hash delimiter rather than part of the path.

Run this script to check how params are validated and whether special characters are handled:


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

@nx-cloud
Copy link

nx-cloud bot commented Nov 15, 2025

🤖 Nx Cloud AI Fix Eligible

An automatically generated fix could have helped fix failing tasks for this run, but Self-healing CI is disabled for this workspace. Visit workspace settings to enable it and get automatic fixes in future runs.

To disable these notifications, a workspace admin can disable them in workspace settings.


View your CI Pipeline Execution ↗ for commit 2c7c08c

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

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 15, 2025

More templates

@tanstack/arktype-adapter

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

@tanstack/directive-functions-plugin

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/server-functions-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/zod-adapter

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

commit: 2c7c08c

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.

2 participants