[feat]: Add stable IDs for server functions#7693
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds optional ChangesManual server-function ID reservation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 148dfab
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview1 package(s) bumped directly, 13 bumped as dependents. 🟩 Patch bumps
|
Bundle Size Benchmarks
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 674-689: The reserveFunctionId logic currently only checks
reserved/generated IDs and manual owners, so a manual server function ID can
still collide with a known server function discovered by getKnownServerFns().
Update reserveFunctionId in compiler.ts to also consult the known
server-function set before accepting opts.functionId, and reject any manual ID
that matches an existing server function ID so handleCreateServerFn never reuses
an unrelated knownFns entry.
In `@packages/start-plugin-core/src/start-compiler/handleCreateServerFn.ts`:
- Around line 184-215: Resolve static template-literal IDs in the createServerFn
ID extraction path: handle no-expression template literals the same way as
string literals so `resolveStaticString`-compatible values like `id:
\`get-user\`` and `const id = \`get-user\`; createServerFn({ id })` don’t fall
through to the generic error. Update the ID resolution logic in
`handleCreateServerFn` (especially the branches that currently check
`t.isStringLiteral`, `t.isIdentifier`, and `bindingInit`) to accept the static
template-literal case and still validate it against
`MANUAL_SERVER_FN_ID_PATTERN` before returning the resolved value.
- Around line 227-250: The helper getCreateServerFnCallExpression only matches
an innermost callee named createServerFn, so aliased or namespace-resolved
factory calls are missed and manual ids are skipped. Update the resolution logic
in getCreateServerFnCallExpression (and any related createServerFn candidate
handling in handleCreateServerFn.ts) to unwrap aliased/namespace call chains and
recognize the actual factory call regardless of local import name, so { id:
"..." } is preserved instead of falling back to generated IDs.
🪄 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: 01e9cb30-43da-4380-bb1e-cd0ea1f81bcd
📒 Files selected for processing (6)
packages/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/tests/createServerFn/createServerFn.test.ts
Merging this PR will degrade performance by 28.87%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem server-fn-churn (solid) |
394.4 KB | 3,339.6 KB | -88.19% |
| ❌ | Memory | mem streaming-peak chunked (vue) |
11.9 MB | 12.3 MB | -3.19% |
| ⚡ | Memory | mem aborted-requests (solid) |
2.4 MB | 1.9 MB | +24.72% |
| ⚡ | Memory | mem peak-large-page (solid) |
3.9 MB | 3.4 MB | +14.54% |
| ⚡ | Memory | mem aborted-requests (vue) |
1,021 KB | 915.8 KB | +11.48% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing stable-serverfn-ids-2 (148dfab) with main (ba52d2b)1
Footnotes
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…tation on ID collisions
…hance tests for ID handling
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…for manual server function IDs
…outer into stable-serverfn-ids-2
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
We fixed the ESLint @typescript-eslint/no-unnecessary-condition error by correcting the type and loop condition in the newly added getCreateServerFnCallExpression function. The currentCall variable was typed as t.CallExpression | undefined despite never being assigned undefined, and the while (currentCall) guard was therefore always truthy. Changing the type to t.CallExpression and the loop to while (true) accurately reflects the logic, where all exit paths are via explicit return statements.
Tip
✅ We verified this fix by re-running @tanstack/start-plugin-core:test:eslint.
diff --git a/packages/start-plugin-core/src/start-compiler/handleCreateServerFn.ts b/packages/start-plugin-core/src/start-compiler/handleCreateServerFn.ts
index 0d0bbb8a..e587a47e 100644
--- a/packages/start-plugin-core/src/start-compiler/handleCreateServerFn.ts
+++ b/packages/start-plugin-core/src/start-compiler/handleCreateServerFn.ts
@@ -216,10 +216,10 @@ function extractManualServerFnId(
function getCreateServerFnCallExpression(
candidatePath: babel.NodePath<t.CallExpression>,
): t.CallExpression | undefined {
- let currentCall: t.CallExpression | undefined = candidatePath.node
+ let currentCall: t.CallExpression = candidatePath.node
let sawMethodChain = false
- while (currentCall) {
+ for (;;) {
const callee: t.CallExpression['callee'] = currentCall.callee
if (!t.isMemberExpression(callee)) {
return sawMethodChain ? currentCall : undefined
@@ -237,8 +237,6 @@ function getCreateServerFnCallExpression(
sawMethodChain = true
currentCall = innerCall
}
-
- return undefined
}
function resolveStaticString(
Or Apply changes locally with:
npx nx-cloud apply-locally TbLk-kNAH
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
…outer into stable-serverfn-ids-2
1 participant
Summary by CodeRabbit
idwhen creating server functions for stable identifier-based runtime lookup, including through chaining and locally named re-exports.idshapes, and enforces collision rules for manual/manual and manual/generated cases.