[feat]: Add stable IDs for server functions#7682
Conversation
…nction ID resolution
📝 WalkthroughWalkthroughAdds ChangesManual server function ID support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
View your CI Pipeline Execution ↗ for commit b8bdb89
☁️ Nx Cloud last updated this comment at |
Merging this PR will improve performance by 3.18%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem serialization-payload (solid) |
6.8 MB | 7.1 MB | -3.59% |
| ❌ | Memory | mem request-churn (solid) |
1.1 MB | 1.2 MB | -3.29% |
| ❌ | Memory | mem streaming-peak chunked (vue) |
11.9 MB | 12.3 MB | -3.26% |
| ⚡ | Memory | mem peak-large-page (solid) |
3.9 MB | 3.4 MB | +14.16% |
| ⚡ | Memory | mem aborted-requests (solid) |
2.4 MB | 2.1 MB | +12.53% |
| ⚡ | Memory | mem aborted-requests (vue) |
1,021 KB | 980.3 KB | +4.15% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing LadyBluenotes:stable-serverfn-ids (599f31b) with main (ba52d2b)1
Footnotes
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/start-plugin-core/src/vite/start-compiler-plugin/module-specifier.ts (1)
67-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTighten typing of the decoded payload.
JSON.parse(...)returnsany, sodecoded.file/decoded.exportaccesses bypass strict checks. Type it asunknownand narrow explicitly to keep the validation type-safe.♻️ Optional: narrow from
unknown- const decoded = JSON.parse(Buffer.from(id, 'base64url').toString('utf8')) - if ( - typeof decoded.file === 'string' && - typeof decoded.export === 'string' - ) { + const decoded: unknown = JSON.parse( + Buffer.from(id, 'base64url').toString('utf8'), + ) + if ( + decoded !== null && + typeof decoded === 'object' && + typeof (decoded as Record<string, unknown>).file === 'string' && + typeof (decoded as Record<string, unknown>).export === 'string' + ) { return { - file: decoded.file, - export: decoded.export, + file: (decoded as { file: string }).file, + export: (decoded as { export: string }).export, } }As per coding guidelines: "Use TypeScript strict mode with extensive type safety".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-plugin-core/src/vite/start-compiler-plugin/module-specifier.ts` around lines 67 - 77, The variable `decoded` from `JSON.parse()` currently has type `any`, which bypasses TypeScript's strict mode checks. Change the code to explicitly type `decoded` as `unknown` instead of relying on the implicit `any` type from JSON.parse, then use the existing typeof checks to narrow the type safely. This ensures the property accesses to `decoded.file` and `decoded.export` are properly validated before use and maintain type safety throughout the validation block.Source: Coding guidelines
packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts (1)
552-559: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
devServerFnmodule construction.Lines 553-559 and 600-606 are identical. Extract a small local helper to avoid drift since both build the same virtual module source.
♻️ Optional helper
const buildDevServerFnModule = (fnId: string) => `export const devServerFn = ${JSON.stringify( getViteDevServerFnImport( fnId, serverFnsById, createViteDevServerFnModuleSpecifierEncoder(root), ), )}`Then
return buildDevServerFnModule(fnId)in both branches.Also applies to: 599-607
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts` around lines 552 - 559, The code that constructs the devServerFn export string is duplicated in two separate locations. Extract this duplicate logic into a local helper function named buildDevServerFnModule that accepts fnId as a parameter. The helper should contain the logic that calls getViteDevServerFnImport with fnId, serverFnsById, and createViteDevServerFnModuleSpecifierEncoder(root), wraps it in JSON.stringify, and returns the template literal string. Replace both occurrences of this duplicate code with calls to the new helper function to eliminate code drift and improve maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/start-plugin-core/src/start-compiler/compiler.ts`:
- Around line 675-681: The issue is that reserveFunctionId only checks this
compiler instance's functionIds set, but generateFunctionId returns without
consulting this set before generating an ID, allowing a manually reserved ID to
later collide with an auto-generated ID. Additionally, known IDs from
getKnownServerFns() are not included in the collision check. Update
generateFunctionId to consult both this.functionIds and the known server
function IDs from getKnownServerFns() before returning a generated ID, ensuring
auto-generated IDs follow the same collision detection path as manually reserved
IDs.
---
Nitpick comments:
In
`@packages/start-plugin-core/src/vite/start-compiler-plugin/module-specifier.ts`:
- Around line 67-77: The variable `decoded` from `JSON.parse()` currently has
type `any`, which bypasses TypeScript's strict mode checks. Change the code to
explicitly type `decoded` as `unknown` instead of relying on the implicit `any`
type from JSON.parse, then use the existing typeof checks to narrow the type
safely. This ensures the property accesses to `decoded.file` and
`decoded.export` are properly validated before use and maintain type safety
throughout the validation block.
In `@packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts`:
- Around line 552-559: The code that constructs the devServerFn export string is
duplicated in two separate locations. Extract this duplicate logic into a local
helper function named buildDevServerFnModule that accepts fnId as a parameter.
The helper should contain the logic that calls getViteDevServerFnImport with
fnId, serverFnsById, and createViteDevServerFnModuleSpecifierEncoder(root),
wraps it in JSON.stringify, and returns the template literal string. Replace
both occurrences of this duplicate code with calls to the new helper function to
eliminate code drift and improve maintainability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d34eb9e0-a53d-48c9-ab74-8f67b9b7ad49
📒 Files selected for processing (10)
docs/start/framework/react/guide/server-functions.mdpackages/start-client-core/src/createServerFn.tspackages/start-client-core/src/tests/createServerFn.test-d.tspackages/start-plugin-core/src/start-compiler/compiler.tspackages/start-plugin-core/src/start-compiler/handleCreateServerFn.tspackages/start-plugin-core/src/start-compiler/types.tspackages/start-plugin-core/src/vite/start-compiler-plugin/module-specifier.tspackages/start-plugin-core/src/vite/start-compiler-plugin/plugin.tspackages/start-plugin-core/tests/createServerFn/createServerFn.test.tspackages/start-plugin-core/tests/vite/start-compiler-utils.test.ts
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud has identified a possible root cause for your failed CI:
We determined this failure is an environment issue unrelated to the PR changes. The error occurs inside Playwright's own FullConfigInternal constructor, where the runtime environment has a frozen/sealed object preventing Playwright from initializing its config — a problem no code change in this PR could cause or fix. We recommend re-running the CI pipeline to recover from this transient environment state.
No code changes were suggested for this issue.
You can trigger a rerun by pushing an empty commit:
git commit --allow-empty -m "chore: trigger rerun"
git push
🎓 Learn more about Self-Healing CI on nx.dev
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/start-plugin-core/src/start-compiler/compiler.ts`:
- Around line 713-721: The reserveFunctionId method adds function IDs to the
functionIds set, but the invalidateModules method does not clear these reserved
IDs when modules are invalidated, causing stale reservations to persist in
long-lived dev compilers. Modify the reserveFunctionId method to track which
module each function ID belongs to (e.g., store both the ID and its owning
module), and then update invalidateModules to iterate through and remove
function ID reservations that belonged to the invalidated modules from the
functionIds set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f2d45d44-ba44-4ee9-a0ad-0c3fb0d57ddc
📒 Files selected for processing (2)
packages/start-plugin-core/src/start-compiler/compiler.tspackages/start-plugin-core/tests/createServerFn/createServerFn.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/start-plugin-core/tests/createServerFn/createServerFn.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts (1)
574-580: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the duplicated
devServerFnmodule construction.The two branches build the exact same
export const devServerFn = ...string and each re-creates the module-specifier encoder viacreateViteDevServerFnModuleSpecifierEncoder(root). Consider a small helper to keep both registration paths in sync and avoid rebuilding the encoder twice per resolution.♻️ Suggested helper
+ const buildDevServerFnModule = (id: string) => + `export const devServerFn = ${JSON.stringify( + getViteDevServerFnImport( + id, + serverFnsById, + createViteDevServerFnModuleSpecifierEncoder(root), + ), + )}`Then both branches become
return buildDevServerFnModule(fnId).Also applies to: 621-627
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts` around lines 574 - 580, The code contains duplicate construction of the `export const devServerFn = ...` module string in two separate branches (around lines 574-580 and 621-627), and each branch independently creates the module-specifier encoder via createViteDevServerFnModuleSpecifierEncoder(root). Extract this shared logic into a helper function (such as buildDevServerFnModule) that takes fnId as a parameter and handles the getViteDevServerFnImport and JSON.stringify logic, while moving the encoder creation to a single location at a higher scope so it is created only once and can be reused by both branches. Replace both duplicated return statements with calls to this new helper function to keep the registration paths in sync and avoid rebuilding the encoder twice.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts`:
- Around line 574-580: The code contains duplicate construction of the `export
const devServerFn = ...` module string in two separate branches (around lines
574-580 and 621-627), and each branch independently creates the module-specifier
encoder via createViteDevServerFnModuleSpecifierEncoder(root). Extract this
shared logic into a helper function (such as buildDevServerFnModule) that takes
fnId as a parameter and handles the getViteDevServerFnImport and JSON.stringify
logic, while moving the encoder creation to a single location at a higher scope
so it is created only once and can be reused by both branches. Replace both
duplicated return statements with calls to this new helper function to keep the
registration paths in sync and avoid rebuilding the encoder twice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a30191b8-8cda-4449-9772-6dce6e86de76
📒 Files selected for processing (4)
packages/start-plugin-core/src/start-compiler/compiler.tspackages/start-plugin-core/src/start-compiler/handleCreateServerFn.tspackages/start-plugin-core/src/vite/start-compiler-plugin/plugin.tspackages/start-plugin-core/tests/createServerFn/createServerFn.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/start-plugin-core/src/start-compiler/compiler.ts
- packages/start-plugin-core/src/start-compiler/handleCreateServerFn.ts
- packages/start-plugin-core/tests/createServerFn/createServerFn.test.ts
|
@schiller-manuel, one dev-mode nuance I wanted to get your 2c on: Manual IDs are intentionally opaque, e.g. Generated dev IDs also have a fallback path because they already encode For this PR, I kept manual IDs on the registration-based path rather than adding a new discovery mechanism for opaque IDs. I think that adding pre-registration discovery for manual IDs would be a separate dev-server enhancement, not required for the core stable-ID behavior. Does that seem right, or is there an existing dev path you’d expect this to hook into? |
PR summary
This PR adds stable/manual server function IDs for TanStack Start.
It adds
.id('...')tocreateServerFn()so users can preserve a server function’s identity across file moves or variable renames. Manual IDs are statically extracted by the compiler, validated as URL-safe path segments, stripped from emitted runtime code, and used as the server function RPC ID.It also updates collision behavior so manual IDs take precedence over generated/custom IDs. If a generated ID collides with a manual ID, the generated ID is deduped with a suffix like
_1, regardless of declaration order.Implementation details
.id()builder API and type coverage..id()from emitted caller/provider code.serverFnsByIdentries during HMR invalidation.Coverage
.id()builder ordering and factory behavior.generateFunctionIdprecedence.Summary by CodeRabbit
Release Notes
New Features
id()support for server functions (including factory-based usage) to keep function identity stable across file moves and renames.Documentation
Tests
id()chaining, invalid/dynamic IDs, duplicate detection, deduping, and expected compiled RPC output.