fix: decouple server functions caller and provider #5712
Conversation
- remove directiveLabel - make directive configurable - make envName required
this let's us avoid `tanstack-start-injected-head-scripts:v` being present in prod build
WalkthroughRefactors directive and server-function compilation and plugin wiring: threads a single Changes
Sequence Diagram(s)sequenceDiagram
actor File as File (source)
participant Transform as Transform Handler
participant EnvSel as Env Selector
participant Compiler as compileDirectives
participant Finder as findDirectives
participant Caller as onDirectiveFnsById
rect rgb(230,240,255)
File->>Transform: transform(code, id)
Transform->>Transform: normalize id
Transform->>EnvSel: id contains directiveSplitParam?
EnvSel-->>Transform: isDirectiveSplitParam (true/false)
end
rect rgb(235,255,230)
alt isDirectiveSplitParam
Transform->>Transform: select provider options
else
Transform->>Transform: select caller matching env
end
end
rect rgb(255,245,220)
Note over Transform,Compiler: Compile with explicit directive + split params
Transform->>Compiler: compileDirectives({ directive, directiveSplitParam, isDirectiveSplitParam, ... })
Compiler->>Finder: findDirectives({ directive, directiveSplitParam, isDirectiveSplitParam, ... })
Finder-->>Compiler: directives map
Compiler-->>Transform: { compiledResult, directiveFnsById }
end
rect rgb(250,235,245)
alt not isDirectiveSplitParam
Transform->>Caller: onDirectiveFnsById(directiveFnsById)
else
Note right of Transform: skip onDirectiveFnsById for split-param path
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
Comment |
|
View your CI Pipeline Execution ↗ for commit f549563
☁️ 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-router-ssr-query
@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-adapter
commit: |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/start-plugin-core/src/create-server-fn-plugin/handleCreateServerFn.ts (1)
149-150: Use the provided directive stringWe now take
opts.directive, but we still emit a hard-coded'use server'directive. Any consumer configuring a different directive will silently fail because the transformed AST never reflects their value. Please wire the new option through.- [t.directive(t.directiveLiteral('use server'))], + [t.directive(t.directiveLiteral(opts.directive))],packages/directive-functions-plugin/src/compilers.ts (1)
492-499: Fix isSourceFn derivation for replacers
isSourceFnis meant to distinguish source modules from provider split chunks, but!!opts.directiveSplitParamis alwaystruebecause the param string is never empty. Provider-side compilations now masquerade as source calls, so replacer implementations that branch onisSourceFnwill mis-handle split files. Please flip the flag to use!opts.isDirectiveSplitParaminstead.- isSourceFn: !!opts.directiveSplitParam, + isSourceFn: !opts.isDirectiveSplitParam,
🧹 Nitpick comments (2)
packages/start-server-core/src/router-manifest.ts (2)
11-12: Add type annotations for the dynamic import.The dynamic import lacks explicit type information, which reduces type safety and could lead to runtime errors if the virtual module doesn't export
tsrStartManifestas expected.Consider adding a type annotation:
- const { tsrStartManifest } = await import('tanstack-start-manifest:v') + const { tsrStartManifest } = await import('tanstack-start-manifest:v') as { tsrStartManifest: () => typeof startManifest }Alternatively, declare module types in a
.d.tsfile:declare module 'tanstack-start-manifest:v' { export const tsrStartManifest: () => StartManifest }As per coding guidelines (TypeScript in strict mode with extensive type safety).
21-23: Add type annotations for the dynamic import.Similar to the import on line 11, this dynamic import lacks type information.
Add appropriate type annotations:
- const { injectedHeadScripts } = await import( - 'tanstack-start-injected-head-scripts:v' - ) + const { injectedHeadScripts } = await import( + 'tanstack-start-injected-head-scripts:v' + ) as { injectedHeadScripts?: string }Or declare the module type in a
.d.tsfile as mentioned in the previous comment.As per coding guidelines (TypeScript in strict mode with extensive type safety).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
packages/directive-functions-plugin/src/compilers.ts(7 hunks)packages/directive-functions-plugin/src/index.ts(1 hunks)packages/directive-functions-plugin/tests/compiler.test.ts(0 hunks)packages/react-start/package.json(1 hunks)packages/react-start/src/ssr-rpc.ts(1 hunks)packages/react-start/vite.config.ts(1 hunks)packages/server-functions-plugin/src/index.ts(4 hunks)packages/solid-start/package.json(1 hunks)packages/solid-start/src/ssr-rpc.ts(1 hunks)packages/solid-start/vite.config.ts(1 hunks)packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts(2 hunks)packages/start-plugin-core/src/create-server-fn-plugin/handleCreateServerFn.ts(1 hunks)packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts(2 hunks)packages/start-plugin-core/src/plugin.ts(3 hunks)packages/start-server-core/package.json(1 hunks)packages/start-server-core/src/createServerRpc.ts(0 hunks)packages/start-server-core/src/createSsrRpc.ts(1 hunks)packages/start-server-core/src/loadVirtualModule.ts(0 hunks)packages/start-server-core/src/router-manifest.ts(2 hunks)packages/start-server-core/src/virtual-modules.ts(1 hunks)packages/start-server-core/vite.config.ts(1 hunks)
💤 Files with no reviewable changes (3)
- packages/start-server-core/src/createServerRpc.ts
- packages/directive-functions-plugin/tests/compiler.test.ts
- packages/start-server-core/src/loadVirtualModule.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/solid-start/src/ssr-rpc.tspackages/react-start/vite.config.tspackages/start-plugin-core/src/create-server-fn-plugin/plugin.tspackages/start-server-core/src/virtual-modules.tspackages/start-plugin-core/src/create-server-fn-plugin/handleCreateServerFn.tspackages/start-plugin-core/src/create-server-fn-plugin/compiler.tspackages/server-functions-plugin/src/index.tspackages/start-server-core/src/createSsrRpc.tspackages/solid-start/vite.config.tspackages/react-start/src/ssr-rpc.tspackages/start-server-core/src/router-manifest.tspackages/directive-functions-plugin/src/compilers.tspackages/start-server-core/vite.config.tspackages/start-plugin-core/src/plugin.tspackages/directive-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/solid-start/src/ssr-rpc.tspackages/react-start/vite.config.tspackages/start-plugin-core/src/create-server-fn-plugin/plugin.tspackages/start-server-core/src/virtual-modules.tspackages/start-server-core/package.jsonpackages/start-plugin-core/src/create-server-fn-plugin/handleCreateServerFn.tspackages/start-plugin-core/src/create-server-fn-plugin/compiler.tspackages/react-start/package.jsonpackages/start-server-core/src/createSsrRpc.tspackages/solid-start/package.jsonpackages/solid-start/vite.config.tspackages/react-start/src/ssr-rpc.tspackages/start-server-core/src/router-manifest.tspackages/start-server-core/vite.config.tspackages/start-plugin-core/src/plugin.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
packages/start-server-core/package.jsonpackages/react-start/package.jsonpackages/solid-start/package.json
🧠 Learnings (6)
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Applied to files:
packages/solid-start/src/ssr-rpc.tspackages/start-server-core/src/router-manifest.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-core/** : Keep framework-agnostic core router logic in packages/router-core/
Applied to files:
packages/start-server-core/src/router-manifest.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to docs/{router,start}/** : Place router docs under docs/router/ and start framework docs under docs/start/
Applied to files:
packages/start-server-core/src/router-manifest.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{router-cli,router-generator,router-plugin,virtual-file-routes}/** : Keep CLI, generators, bundler plugins, and virtual file routing utilities in their dedicated tooling package directories
Applied to files:
packages/start-server-core/src/router-manifest.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-plugin/** : Use unplugin for universal bundler plugins in the router-plugin package
Applied to files:
packages/start-server-core/src/router-manifest.ts
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
packages/start-server-core/src/router-manifest.ts
🧬 Code graph analysis (5)
packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts (1)
packages/start-plugin-core/src/start-compiler-plugin/compilers.ts (1)
CompileStartFrameworkOptions(18-18)
packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (1)
packages/start-plugin-core/src/create-server-fn-plugin/handleCreateServerFn.ts (1)
handleCreateServerFn(8-157)
packages/server-functions-plugin/src/index.ts (3)
packages/directive-functions-plugin/src/compilers.ts (2)
ReplacerFn(32-38)GenerateFunctionIdFn(26-30)packages/directive-functions-plugin/src/index.ts (3)
ReplacerFn(19-19)GenerateFunctionIdFn(20-20)TanStackDirectiveFunctionsPluginEnv(48-126)packages/start-server-core/src/fake-start-server-fn-manifest.ts (1)
getServerFnById(1-1)
packages/start-plugin-core/src/plugin.ts (4)
packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts (1)
createServerFnPlugin(42-178)packages/server-functions-plugin/src/index.ts (1)
TanStackServerFnPlugin(42-244)packages/start-server-core/src/virtual-modules.ts (1)
VIRTUAL_MODULES(1-5)packages/start-plugin-core/src/constants.ts (1)
VITE_ENVIRONMENT_NAMES(1-6)
packages/directive-functions-plugin/src/index.ts (2)
packages/directive-functions-plugin/src/compilers.ts (3)
DirectiveFn(12-19)GenerateFunctionIdFn(26-30)compileDirectives(55-110)packages/router-plugin/src/core/utils.ts (1)
debug(1-3)
🪛 ast-grep (0.39.6)
packages/directive-functions-plugin/src/index.ts
[warning] 33-33: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp("${directive}"|'${directive}', 'gm')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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 (10)
packages/start-server-core/src/router-manifest.ts (1)
11-11: The suggestion in the review comment would break bundler support and is not viable.The hardcoded strings in
router-manifest.ts(lines 11 and 21-22) are not problematic duplication—they are a technical necessity. Bundlers like Vite and Webpack require statically analyzable string literals inawait import()calls at build time. Usingawait import(VIRTUAL_MODULES.startManifest)would prevent the bundler from resolving the virtual module, breaking the build.The VIRTUAL_MODULES constant serves a different purpose (likely for reference and type safety elsewhere) but cannot be used in these dynamic import statements without losing bundler support.
Likely an incorrect or invalid review comment.
packages/react-start/vite.config.ts (1)
32-32: LGTM! SSR RPC entry added correctly.The new entry point for SSR RPC follows the existing pattern and aligns with the corresponding source file and package.json export additions.
packages/solid-start/vite.config.ts (1)
30-30: LGTM! SSR RPC entry added correctly.The entry addition is consistent with the React Start implementation and properly wired with the source file and package.json exports.
packages/start-server-core/vite.config.ts (1)
24-24: LGTM! Core SSR RPC entry added correctly.The createSsrRpc entry point is properly configured and will be re-exported by downstream packages (react-start and solid-start).
packages/react-start/src/ssr-rpc.ts (1)
1-1: LGTM! Consistent re-export pattern.The re-export follows the same pattern as the Solid Start implementation. The verification script in the solid-start/src/ssr-rpc.ts review will confirm the export path exists.
packages/solid-start/package.json (1)
59-64: LGTM! SSR RPC export properly configured.The new export follows the established pattern and correctly maps to the built artifacts in the dist directory.
packages/react-start/package.json (1)
65-70: LGTM! SSR RPC export properly configured.The export configuration is consistent with the Solid Start implementation and follows the existing export patterns in this package.
packages/start-plugin-core/src/create-server-fn-plugin/compiler.ts (2)
63-63: LGTM! Directive parameter added to options.The directive field is properly added to the constructor options, enabling explicit directive handling as described in the PR objectives.
256-263: Code changes are correct; directive propagation is appropriate.The verification confirms that
handleCreateMiddlewareis correctly designed without directive context. The function signature only acceptsenvparameter, and all transformations (removing validators and server calls on the client) are purely environment-based, not directive-dependent. The directive is correctly passed only tohandleCreateServerFn, where it's needed for server function creation logic.packages/solid-start/src/ssr-rpc.ts (1)
1-1: Export path verified and confirmed.The
@tanstack/start-server-corepackage properly exports./createSsrRpcwith both TypeScript types and default exports configured. The source file exists atpackages/start-server-core/src/createSsrRpc.ts. No issues found.
| const fn = async (opts: any, signal: any): Promise<any> => { | ||
| const serverFn = await getServerFnById(functionId) | ||
| return serverFn(opts ?? {}, signal) | ||
| } |
There was a problem hiding this comment.
Make SSR RPC callable with a single argument
The generated declaration right now requires two positional parameters, so TypeScript callers can’t do await rpc(payload) without supplying a second argument. The client/server RPC helpers keep signal optional; this one should too to avoid an API regression.
- const fn = async (opts: any, signal: any): Promise<any> => {
+ const fn = async (
+ opts?: any,
+ signal?: any,
+ ): Promise<any> => {
const serverFn = await getServerFnById(functionId)
return serverFn(opts ?? {}, signal)
}📝 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 fn = async (opts: any, signal: any): Promise<any> => { | |
| const serverFn = await getServerFnById(functionId) | |
| return serverFn(opts ?? {}, signal) | |
| } | |
| const fn = async ( | |
| opts?: any, | |
| signal?: any, | |
| ): Promise<any> => { | |
| const serverFn = await getServerFnById(functionId) | |
| return serverFn(opts ?? {}, signal) | |
| } |
🤖 Prompt for AI Agents
In packages/start-server-core/src/createSsrRpc.ts around lines 6 to 9, the async
wrapper requires two positional params which prevents callers from invoking
rpc(payload) — make the second parameter optional (e.g. signal?: AbortSignal or
signal?: any) in the function signature and keep the current fallback behavior
when calling serverFn(opts ?? {}, signal) so callers can omit the signal without
a type error.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/directive-functions-plugin/tests/compiler.test.ts (1)
48-55: Consider updating or clarifying comments.The comments mention
splitImportFnand conditional behavior for split functions vs referenced functions, but thereplacercode only showscreateServerRpc(${JSON.stringify(opts.functionId)}, ${opts.fn}). If this conditional logic is handled elsewhere in the compiler (driven by the newisDirectiveSplitParamflag), consider updating these comments to clarify that, or remove them if they're outdated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/directive-functions-plugin/tests/compiler.test.ts(2 hunks)packages/start-plugin-core/tests/createMiddleware-create-server-fn-plugin/createMiddleware.test.ts(1 hunks)packages/start-plugin-core/tests/createServerFn/createServerFn.test.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-plugin-core/tests/createMiddleware-create-server-fn-plugin/createMiddleware.test.tspackages/start-plugin-core/tests/createServerFn/createServerFn.test.tspackages/directive-functions-plugin/tests/compiler.test.ts
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/start-plugin-core/tests/createMiddleware-create-server-fn-plugin/createMiddleware.test.tspackages/start-plugin-core/tests/createServerFn/createServerFn.test.ts
🧬 Code graph analysis (1)
packages/directive-functions-plugin/tests/compiler.test.ts (1)
packages/directive-functions-plugin/src/compilers.ts (1)
CompileDirectivesOpts(42-53)
⏰ 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: Test
- GitHub Check: Preview
🔇 Additional comments (5)
packages/start-plugin-core/tests/createMiddleware-create-server-fn-plugin/createMiddleware.test.ts (1)
34-34: LGTM! Directive option added correctly.The addition of the
directive: 'use server'option aligns with the PR's objective to thread directives through the compiler pipeline. The test setup correctly passes this directive to theServerFnCompilerconstructor.packages/start-plugin-core/tests/createServerFn/createServerFn.test.ts (1)
30-30: LGTM! Directive option added with comprehensive test coverage.The addition of the
directive: 'use server'option is correct and aligns with the PR's refactoring objectives. The inline snapshots throughout this file (lines 77-96, 118-130, 139-157) provide excellent verification that the directive is properly emitted in the generated code, confirming the feature works as expected.packages/directive-functions-plugin/tests/compiler.test.ts (3)
20-29: LGTM! Configuration correctly updated for new API.The addition of
directiveSplitParamandisDirectiveSplitParam: falsealigns with the refactoredCompileDirectivesOptsinterface. Thefalsevalue is appropriate for client-side compilation.
31-40: LGTM! SSR configuration correctly updated.The configuration changes mirror the client config appropriately, with
isDirectiveSplitParam: falsefor SSR compilation context.
60-978: Excellent test coverage maintained.The comprehensive test suite remains unchanged and continues to validate all directive compilation scenarios. The fact that only configuration objects needed updating demonstrates good API design and backward compatibility.
Summary by CodeRabbit
New Features
Refactor