Skip to content

Conversation

schiller-manuel
Copy link
Contributor

@schiller-manuel schiller-manuel commented Oct 17, 2025

fixes #5509

Summary by CodeRabbit

  • Bug Fixes
    • Fixed router origin derivation to properly handle cases where the browser origin is null or unavailable, providing a fallback to localhost for improved compatibility across different environments.

Copy link
Contributor

coderabbitai bot commented Oct 17, 2025

Walkthrough

A guard condition is added when deriving origin on the client-side: origin from window.origin is only used when it exists and is not the string 'null', otherwise fallback to 'http://localhost' to prevent URL parsing errors.

Changes

Cohort / File(s) Change Summary
Client-side origin derivation guard
packages/router-core/src/router.ts
Add conditional check when setting origin from window.origin to ensure it exists and is not the literal string 'null', with fallback to 'http://localhost' for edge cases like file: protocol deployments

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Poem

🐰 A null string caused quite the fright,
When file protocol came into sight,
But now we check before we dare,
Localhost's fallback waits with care!
URLs parse safe, left and right! 🎉

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 pull request title "fix: fallback if origin is 'null'" is clearly related to the main change in the changeset. The modification adds a guard to check whether window.origin exists and is not the literal string 'null' before using it, with a fallback to 'http://localhost' otherwise. The title accurately reflects this primary objective, is concise and specific, and would allow teammates scanning the commit history to understand the fix at a glance without ambiguity or vague terminology.
Linked Issues Check ✅ Passed The code changes directly address the requirements specified in linked issue #5509. The issue reports a regression where window.origin returns the literal string "null" when using file: protocol in environments like Veeva CRM, causing "Cannot be parsed as a URL against 'null'" errors. The fix implements exactly what was needed: a guard that checks if window.origin exists and is not the string 'null', then falls back to 'http://localhost', which restores backward compatibility with 1.131.x behavior as expected by the issue.
Out of Scope Changes Check ✅ Passed All changes in this pull request are in scope and directly related to fixing issue #5509. Only one file is modified (packages/router-core/src/router.ts) with a targeted fix that adds guards for origin derivation on the client. No new exported entities are introduced, and there are no unrelated refactorings, documentation changes, or extraneous modifications present in the changeset.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ 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-5509

📜 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 a9c92ff and 622d704.

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

📄 CodeRabbit inference engine (AGENTS.md)

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

Files:

  • packages/router-core/src/router.ts
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 (1)
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
PR: TanStack/router#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
⏰ 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 (1)
packages/router-core/src/router.ts (1)

970-978: LGTM! Correctly handles file: protocol edge case.

The guard condition properly prevents using the literal string 'null' as a URL base, which resolves the regression in file: protocol environments (e.g., Veeva CRM). The optional chaining and explicit null-string check ensure robustness, and the fallback to 'http://localhost' maintains consistency with the server-side behavior.


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

Copy link

nx-cloud bot commented Oct 17, 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 622d704

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

☁️ Nx Cloud last updated this comment at 2025-10-17 17:35:33 UTC

Copy link

pkg-pr-new bot commented Oct 17, 2025

More templates

@tanstack/arktype-adapter

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

@tanstack/directive-functions-plugin

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/server-functions-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/zod-adapter

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

commit: 622d704

@schiller-manuel schiller-manuel merged commit 9a0a321 into main Oct 17, 2025
5 of 6 checks passed
@schiller-manuel schiller-manuel deleted the fix-5509 branch October 17, 2025 17:39
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.

Cannot be parsed as a URL against "null"

1 participant