feat: add static function middleware support of basepath #5970
Conversation
WalkthroughThe static function middleware's fetch call now constructs a prefixed URL by joining an environment variable base path ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/start-static-server-functions/src/staticFunctionMiddleware.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety throughout the codebase
Files:
packages/start-static-server-functions/src/staticFunctionMiddleware.ts
🔇 Additional comments (1)
packages/start-static-server-functions/src/staticFunctionMiddleware.ts (1)
119-119: Verify thatTSS_ROUTER_BASEPATHis properly documented and injected — it is not part of the official TanStack Router API.Web search confirms that TanStack Router's official approach for basepath configuration is via
RouterOptions.basepath(set in code when creating the router), not environment variables.TSS_ROUTER_BASEPATHappears to be a project-specific variable that is not documented in TanStack's official API.Before merging this change:
- Document where and how
TSS_ROUTER_BASEPATHis injected — confirm it's set in build config, deployment scripts, or.envfiles and document that requirement- Remove the non-null assertion — replace
process.env.TSS_ROUTER_BASEPATH!with a check that handles the case where the variable is undefined, e.g.,process.env.TSS_ROUTER_BASEPATH ?? ''or similar fallback- Consider the official approach — evaluate whether using
RouterOptions.basepath(configured at router creation time) would be more maintainable than relying on an undocumented environment variable
| const basePath = process.env.TSS_ROUTER_BASEPATH! | ||
| const prefixedUrl = path.join(basePath, url) | ||
| result = await fetch(prefixedUrl, { |
There was a problem hiding this comment.
Use proper URL construction instead of path.join(), and validate the environment variable.
There are several critical issues with this implementation:
-
Incorrect API:
path.join()is for filesystem paths, not URLs. It has OS-specific behavior (e.g., Windows uses backslashes) and doesn't handle URL semantics correctly. -
Missing validation: The non-null assertion on
process.env.TSS_ROUTER_BASEPATHwill cause a runtime error if the variable is undefined, andpath.join(undefined, url)will fail. -
Type safety: This violates TypeScript strict mode guidelines by using non-null assertion without validation.
Apply this diff to fix the URL construction and add proper validation:
- const basePath = process.env.TSS_ROUTER_BASEPATH!
- const prefixedUrl = path.join(basePath, url)
- result = await fetch(prefixedUrl, {
+ const basePath = process.env.TSS_ROUTER_BASEPATH || ''
+ const prefixedUrl = basePath
+ ? `${basePath.replace(/\/$/, '')}${url}`
+ : url
+ result = await fetch(prefixedUrl, {Alternatively, if you need more robust URL handling:
- const basePath = process.env.TSS_ROUTER_BASEPATH!
- const prefixedUrl = path.join(basePath, url)
- result = await fetch(prefixedUrl, {
+ const basePath = process.env.TSS_ROUTER_BASEPATH || ''
+ const prefixedUrl = basePath
+ ? new URL(url, new URL(basePath, 'http://dummy')).pathname
+ : url
+ result = await fetch(prefixedUrl, {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const basePath = process.env.TSS_ROUTER_BASEPATH! | |
| const prefixedUrl = path.join(basePath, url) | |
| result = await fetch(prefixedUrl, { | |
| const basePath = process.env.TSS_ROUTER_BASEPATH || '' | |
| const prefixedUrl = basePath | |
| ? `${basePath.replace(/\/$/, '')}${url}` | |
| : url | |
| result = await fetch(prefixedUrl, { |
🤖 Prompt for AI Agents
In packages/start-static-server-functions/src/staticFunctionMiddleware.ts around
lines 119 to 121, the code wrongly uses path.join and non-null assertion for
building the fetch URL; validate that process.env.TSS_ROUTER_BASEPATH is defined
(throw or return a clear error if not), construct the URL using the WHATWG URL
API (e.g., new URL(url, base) or url.resolve equivalent) to correctly combine
base and relative paths, and remove the non-null assertion so TypeScript
enforces the runtime check; ensure the resulting string passed to fetch is a
fully-qualified URL.
|
View your CI Pipeline Execution ↗ for commit 8f4d04e
☁️ Nx Cloud last updated this comment at |
1 participant
Hello 👋
I tried the new
staticFunctionMiddleware, but I've found an issue as my application is hosted on a subpath (/docs). I propose to fix this, but I'm not sure the router basepath is the right variable to use. Ideally, I would prefer to use theimport.meta.env. BASE_URLof Vite, as I think TSR files are quite similar to bundling assets, but I'm not sure it will work.Let me know what you think and I'll change this straightaway. Thanks for this package!
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.