Skip to content

Conversation

@Sheraff
Copy link
Contributor

@Sheraff Sheraff commented Jan 22, 2026

Same as #6442 but for solid-router

Summary by CodeRabbit

Bug Fixes

  • Fixed handling of internal navigation links to correctly distinguish them from external links, ensuring reliable routing behavior for safe internal routes.

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

@nx-cloud
Copy link

nx-cloud bot commented Jan 22, 2026

View your CI Pipeline Execution ↗ for commit 6b28d99

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

☁️ Nx Cloud last updated this comment at 2026-01-22 12:47:00 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

The change refines external link detection in solid-router's link handling by introducing an early exit for internal "safe" links (those starting with "/" but not "//"). URL parsing and protocol validation now operate on the refined to variable, preventing safe internal navigations from undergoing external link checks.

Changes

Cohort / File(s) Summary
Link External Detection
packages/solid-router/src/link.tsx
Added early return for internal links starting with "/" (excluding "//"). Adjusted URL construction and dangerous protocol checks to use local to variable. Updated warning message and return statement references from options object to to variable.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~35 minutes

Possibly related PRs

Suggested labels

package: solid-router

Suggested reviewers

  • schiller-manuel
  • nlynzaad

Poem

🐰 A path that starts with just one slash,
No need to parse—we skip the hash!
Internal routes now safe and true,
The router hops to routes anew! 🌿

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 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: skipping URL parsing for safe 'to' props in solid-router, which aligns with the actual modifications in the link.tsx file.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 22, 2026

More templates

@tanstack/arktype-adapter

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-fn-stubs

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-fn-stubs@6453

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/vue-router

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

@tanstack/vue-router-devtools

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

@tanstack/vue-router-ssr-query

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

@tanstack/vue-start

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

@tanstack/vue-start-client

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

@tanstack/vue-start-server

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

@tanstack/zod-adapter

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

commit: 6b28d99

@Sheraff Sheraff merged commit ef3486c into main Jan 22, 2026
6 checks passed
@Sheraff Sheraff deleted the refactor-solid-router-skip-url-parsing-for-safe-internal-to branch January 22, 2026 17:04
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.

3 participants