Repository navigation
perf(start-plugin-core): skip resolving member calls on non-namespace receivers - #8758
Conversation
… receivers
A top-level `a.b()` initializer was a direct-call candidate for any `a`, so
`const Input = z.object({...})` resolved `z` and, in dev, loaded and parsed all
of zod through `?server-fn-module-lookup` while every compiled file waited.
For a receiver that is not a namespace import, resolution gives the call the
kind of the receiver itself, which only yields a candidate when the receiver
is an env-only factory and the code calls one of its properties. Skip those
calls; namespace calls, identifier calls and method chains resolve as before.
Refs TanStack#8757
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 655264c The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main compiler optimization: skipping resolution of member calls on non-namespace receivers. Description check The description includes the required Changes, Checklist, and Release Impact sections. It explains the motivation, implementation scope, performance results, tests, changeset, and AI assistance disclo… Linked Issues check Issue #8757requires the compiler to avoid resolving top-level member calls when the receiver is not a namespace import.isMemberCallOnNonNamespaceReceiverexcludes those calls from both direct-call…Out of Scope Changes check The compiler change, compiler tests, and changeset directly support issue #8757. The namespace-import regression coverage preserves required behavior. No unrelated implementation or test changes are s…Full details: Description check
-
Explanation
The description includes the required Changes, Checklist, and Release Impact sections. It explains the motivation, implementation scope, performance results, tests, changeset, and AI assistance disclosure. One checklist item remains unchecked, but the description is otherwise complete.
Full details: Linked Issues check
-
Explanation
Issue
#8757requires the compiler to avoid resolving top-level member calls when the receiver is not a namespace import.isMemberCallOnNonNamespaceReceiverexcludes those calls from both direct-call candidate paths. The tests cover named-import receivers across client, SSR, and provider output. The tests also preserve renamed env-only factory calls through namespace imports for client and server builds.
Full details: Out of Scope Changes check
-
Explanation
The compiler change, compiler tests, and changeset directly support issue
#8757. The namespace-import regression coverage preserves required behavior. No unrelated implementation or test changes are shown in the supplied pull request evidence.
-
✨ Finishing Touches: 💡 1
-
🧪 Generate unit tests (beta)
-
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
|
View your CI Pipeline Execution ↗ for commit 655264c
☁️ Nx Cloud last updated this comment at |
Merging this PR will degrade performance by 1.9%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Memory | mem client interrupted-navigations (vue) |
97.5 KB | 92.5 KB | +5.42% |
| 👁 | Memory | mem client navigation-churn (react) |
81.9 KB | 84.5 KB | -3.14% |
| 👁 | Memory | mem server error-paths redirect (solid) |
11.6 KB | 12.3 KB | -5.22% |
| 👁 | Memory | mem client unique-location-churn (vue) |
80.1 KB | 83.7 KB | -4.31% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing shkumbinhasani:perf/start-compiler-member-call-receivers (655264c) with main (336fdfb)
Footnotes
-
3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
🎯 Changes
Fixes #8757.
A top-level call in a compiled file is a direct-call candidate, so that a renamed
createServerOnlyFn/createClientOnlyFnis still found.isTopLevelDirectCallCandidateaccepts anya.b(), soconst Input = z.object({...})resolvesz, and in dev that loads zod through?server-fn-module-lookupand parses it with Babel, while every compiled file waits.For
a.b()whereais not a namespace import,resolveCalleeKindreturns the kind ofaitself. That passes the candidate filter only whenais an env-only factory and the code calls one of its properties (createServerOnlyFn.bind(...)): aRoot/Builderreceiver resolves toBuilder, and a method-chain kind failsisDirectCallCandidateForKind. This PR skips those calls. Namespace calls (Start.createServerOnlyFn(), orfactory.serverOnly()through a renamed re-export), identifier calls and method chains resolve as before. In the full traversal, a skipped call still goes through the nested factory-name check.Reproducer from the issue (
start-basic-cloudflare, 60 server function files withimport { z } from 'zod'schemas and a top-levelSentry.getClient()), baseline and patched builds alternated, medians:?server-fn-module-lookuptransforms, client?server-fn-module-lookuptransforms, ssrthis.resolvecalls, client / ssr/perfThe gain only applies when the receiver is not a namespace import:
import * as z from 'zod'still loads zod.Tests: two new tests in
tests/compiler.test.ts. One checks that a named-import receiver (z.object(),api.users()) triggers no resolve and no load; it fails onmain. The other checks that a renamed env-only factory called through a namespace import still compiles on client and server. Rantest:unit,test:typesandtest:eslintfor@tanstack/start-plugin-core, and thehmr(vite, vite bundled dev, rsbuild) andserver-functions(vite, rsbuild) e2e suites; all pass.Written with an AI assistant (Claude Code).
✅ Checklist
🚀 Release Impact
🤖 Generated with Claude Code
Summary by CodeRabbit