-
-
Couldn't load subscription status.
- Fork 1.3k
fix: basepath rewrite to match with or without trailing slash #5302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdjusted the basepath rewrite regex from ^/{{trimmedBasepath}}/ to ^/{{trimmedBasepath}}(?:/|$) to match when the basepath is followed by a slash or ends at the basepath. No public API or declaration changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant Rewriter
participant Handler
Client->>Router: Request path
Router->>Rewriter: Apply basepath regex
alt Path starts with basepath + "/" or ends at basepath
Note right of Rewriter: Uses ^/{{trimmedBasepath}}(?:/|$)
Rewriter-->>Router: Match + rewritten path
Router->>Handler: Forward rewritten path
Handler-->>Client: Response
else No match
Rewriter-->>Router: No rewrite
Router->>Handler: Forward original path
Handler-->>Client: Response
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/router-core/src/rewrite.ts (1)
21-40: Regex change is correct; add output tests
The pattern^/${trimmedBasepath}(?:/|$)now gracefully handles paths with or without trailing slash. All existing tests validate the input rewrite behavior, but none cover theoutputfunction. Please add tests to ensureoutputrestores the basepath viajoinPathscorrectly across cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/router-core/src/rewrite.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/rewrite.ts
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/rewrite.ts
|
we already have this PR open for that: #5244 maybe help out there to figure out why the tests work without the fix applied? |
Summary
This PR fixes the basepath rewrite logic to correctly match routes with or without a trailing slash.
Problem
The rewrite logic didn’t consider basepaths without a trailing slash. For example, if you define a basepath like
/myappand try to access the root page at/myapp, it would not work unless you added a trailing slash (i.e.,/myapp/). This caused the root page to be inaccessible without the slash, breaking prerendering in Tanstack StartRelated Issue
See #5261 for more details
Summary by CodeRabbit