Uh oh!
There was an error while loading. Please reload this page.
fix: dev serverfn resolution on windows - #5520
Conversation
WalkthroughServer function ID generation in dev environments now normalizes trailing slashes via a helper function and adjusts file path computation to construct routes with leading slash prefixes. Concurrently, the fake manifest's Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
View your CI Pipeline Execution ↗ for commit f535282
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter@tanstack/directive-functions-plugin@tanstack/eslint-plugin-router@tanstack/history@tanstack/nitro-v2-vite-plugin@tanstack/react-router@tanstack/react-router-devtools@tanstack/react-router-ssr-query@tanstack/react-start@tanstack/react-start-client@tanstack/react-start-server@tanstack/router-cli@tanstack/router-core@tanstack/router-devtools@tanstack/router-devtools-core@tanstack/router-generator@tanstack/router-plugin@tanstack/router-ssr-query-core@tanstack/router-utils@tanstack/router-vite-plugin@tanstack/server-functions-plugin@tanstack/solid-router@tanstack/solid-router-devtools@tanstack/solid-start@tanstack/solid-start-client@tanstack/solid-start-server@tanstack/start-client-core@tanstack/start-plugin-core@tanstack/start-server-core@tanstack/start-static-server-functions@tanstack/start-storage-context@tanstack/valibot-adapter@tanstack/virtual-file-routes@tanstack/zod-adaptercommit: |
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 (2)
packages/server-functions-plugin/src/index.ts(1 hunks)packages/start-server-core/src/fake-start-server-fn-manifest.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/start-server-core/src/fake-start-server-fn-manifest.tspackages/server-functions-plugin/src/index.ts
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/start-server-core/src/fake-start-server-fn-manifest.ts
🧬 Code graph analysis (2)
packages/start-server-core/src/fake-start-server-fn-manifest.ts (1)
packages/start-server-core/src/getServerFnById.ts (1)
getServerFnById(1-1)
packages/server-functions-plugin/src/index.ts (1)
packages/directive-functions-plugin/src/compilers.ts (1)
GenerateFunctionIdFn(26-30)
⏰ 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 (3)
packages/server-functions-plugin/src/index.ts (3)
71-76: LGTM! Clean helper for path normalization.The
withTrailingSlashhelper correctly ensures a trailing slash on paths, which is essential for consistent path prefix matching on different platforms.
96-96: Correct application of normalized path.Using the computed
filevalue instead ofextractedFilenameensures the server function receives the properly normalized path with the/@idprefix for module resolution.
83-91: Verify path normalization handles Windows edge cases.The path normalization logic looks sound—using
withTrailingSlash(root)for the prefix check while slicing byroot.length(without trailing slash) correctly preserves the leading slash. However, please verify:
- Edge case: If
extractedFilenameequalsrootexactly,filebecomes empty and line 90 produces"/@id/". Confirm this is handled appropriately.- Windows paths: Ensure that both
serverDevEnv.config.rootandextractedFilenameare normalized to forward slashes by Vite before this code runs, as mixing backslashes and forward slashes would break thestartsWithcheck.
| @@ -1 +1 @@ | |||
| export function getServerFnById() {} | |||
| export async function getServerFnById(): Promise<any> {} | |||
There was a problem hiding this comment.
Missing required parameter in function signature.
The fake implementation is missing the id parameter that the real implementation includes (see packages/server-functions-plugin/src/index.ts lines 191 and 211). Even though this is a stub, the signature should match to maintain type consistency and prevent runtime errors if this fake is accidentally invoked.
Apply this diff to fix the signature:
-export async function getServerFnById(): Promise<any> {}+export async function getServerFnById(id: string): Promise<any> {}Additionally, consider tightening the return type from Promise<any> to be more specific if the actual return type is known.
📝 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.
| exportasyncfunctiongetServerFnById(): Promise<any>{} | |
| exportasyncfunctiongetServerFnById(id: string): Promise<any>{} |
🤖 Prompt for AI Agents
In packages/start-server-core/src/fake-start-server-fn-manifest.ts around line
1, the exported async function is missing the required id parameter present in
the real implementation; update the function signature to accept the same id
parameter (and any other parameters that the real implementation defines at
lines 191 and 211 in packages/server-functions-plugin/src/index.ts) so the stub
matches the real API, and tighten the return type from Promise<any> to the
actual return type if known (or to a more specific union/interface) to maintain
type consistency and avoid runtime/typing surprises.
Summary by CodeRabbit