fix: no invalidation for server functions in dev #5513
Conversation
github-actions
bot
added
documentation
|
Caution Review failedThe pull request is closed. WalkthroughRefactors server-functions and directive tooling to use function-ID-centric async resolution, propagates extractedFilename into function ID generation and replacer options, replaces a static manifest with an async getServerFnById virtual import, consolidates plugin APIs, and updates docs to focus on production-build ID behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Compiler
participant DirectivePlugin
participant ServerPlugin
participant Manifest as "#tanstack-start-server-fn-manifest"
Note over Compiler,DirectivePlugin: Build-time compilation
Compiler->>DirectivePlugin: provide extractedFilename + function metadata
DirectivePlugin->>ServerPlugin: generateFunctionId(opts including extractedFilename)
ServerPlugin-->>DirectivePlugin: return functionId
rect rgb(225,245,235)
Note over ServerPlugin,Manifest: Runtime resolution (async)
ServerPlugin->>Manifest: getServerFnById(functionId)
Manifest-->>ServerPlugin: async ServerFn (or throw)
ServerPlugin->>ServerPlugin: invoke ServerFn(...)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
|
View your CI Pipeline Execution ↗ for commit dbcde6c
☁️ 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-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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/directive-functions-plugin/src/compilers.ts (2)
482-491: Sanitize extractedFilename to avoid dev invalidation (strip volatile query params).Keeping existing QueryString entries propagates Vite/Dev HMR params (e.g., t=, v=), defeating “no invalidation in dev.” Build extractedFilename with only the directive param.
Apply:
- const [baseFilename, ..._searchParams] = opts.filename.split('?') as [ - string, - ...Array<string>, - ] - const searchParams = new URLSearchParams(_searchParams.join('&')) - searchParams.set(opts.directiveSplitParam, '') - const extractedFilename = `${baseFilename}?${searchParams.toString()}` + const [baseFilename] = opts.filename.split('?') as [string, ...Array<string>] + const searchParams = new URLSearchParams() + // Only include our stable directive split param to keep IDs/chunks stable in dev + searchParams.set(opts.directiveSplitParam, '') + const extractedFilename = `${baseFilename}?${searchParams.toString()}`
501-507: Bug: isSourceFn is always true.
!!opts.directiveSplitParamis a constant truthy string; replacers can’t distinguish source vs split files. Pass the actual context.Apply:
- const replacer = opts.replacer({ + const replacer = opts.replacer({ fn: '$$fn$$', extractedFilename, filename: opts.filename, functionId, - isSourceFn: !!opts.directiveSplitParam, + // Source file if we're NOT in the split-param file + isSourceFn: !compileDirectiveOpts.isDirectiveSplitParam, })packages/server-functions-plugin/src/index.ts (1)
154-160: Fix stale error message.Message references the old API name.
- `TanStackServerFnPluginEnv: environment "${opts.server.envName}" not found`, + `TanStackServerFnPlugin: environment "${opts.server.envName}" not found`,
🧹 Nitpick comments (7)
packages/directive-functions-plugin/src/compilers.ts (2)
529-530: Chunk name derivation: consider using relative path for cross‑platform stability.Optional: derive from relative path to avoid root substring quirks.
Apply:
- chunkName: fileNameToChunkName(opts.root, extractedFilename), + chunkName: fileNameToChunkName(opts.root, path.relative(opts.root, extractedFilename)),
413-421: Redundant add to functionNameSet.
functionNameSet.add(functionName)is called twice; keep one.Apply:
- functionNameSet.add(functionName) - while (programPath.scope.hasBinding(functionName)) { functionName = incrementFunctionNameVersion(functionName) programPath.scope.crawl() } functionNameSet.add(functionName)packages/server-functions-plugin/src/index.ts (3)
55-66: Clarify dev behavior: no invalidation after server env is ready.Early-returning when
serverDevEnvexists freezesdirectiveFnsByIdpost‑startup (dev uses on‑the‑fly import via encoded IDs). Add a brief comment so future edits don’t reintroduce dev invalidation.Apply:
- const onDirectiveFnsById = (d: Record<string, DirectiveFn>) => { - if (serverDevEnv) { - return - } + const onDirectiveFnsById = (d: Record<string, DirectiveFn>) => { + // In dev, once the server environment is ready we don't update this map anymore: + // dev resolution uses encoded file/export IDs and does not rely on this manifest. + if (serverDevEnv) return
161-166: Avoid regex from variable input; anchor/escape the pattern.Even if the IDs are internal, use an anchored/escaped regex to prevent accidental metacharacter matches and silence tooling warnings.
- resolveId: { - filter: { id: new RegExp(opts.manifestVirtualImportId) }, + resolveId: { + filter: { id: new RegExp(`^${escapeRegExp(opts.manifestVirtualImportId)}$`) }, handler() { return resolvedManifestVirtualImportId }, }, - load: { - filter: { id: new RegExp(resolvedManifestVirtualImportId) }, + load: { + filter: { id: new RegExp(`^${escapeRegExp(resolvedManifestVirtualImportId)}$`) }, handler() {Add helper near
resolveViteId:function resolveViteId(id: string) { return `\0${id}` } + +function escapeRegExp(s: string) { + return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +}Also applies to: 168-169
174-185: Harden dev decode path with try/catch.Guard against malformed IDs to surface actionable errors instead of uncaught exceptions.
if (this.environment.mode !== 'build') { const mod = ` export async function getServerFnById(id) { - const decoded = Buffer.from(id, 'base64url').toString('utf8') - const devServerFn = JSON.parse(decoded) + let devServerFn + try { + const decoded = Buffer.from(id, 'base64url').toString('utf8') + devServerFn = JSON.parse(decoded) + } catch (e) { + throw new Error('Invalid dev server function id: ' + id) + } const mod = await import(/* @vite-ignore */ devServerFn.file) return mod[devServerFn.export] } ` return mod }packages/start-server-core/vite.config.ts (1)
26-31: Remove duplicate external entry.
#tanstack-start-server-fn-manifestis already included viaObject.values(VIRTUAL_MODULES). Drop the extra literal.externalDeps: [ ...Object.values(VIRTUAL_MODULES), '#tanstack-start-entry', '#tanstack-router-entry', - '#tanstack-start-server-fn-manifest', ],docs/start/framework/react/guide/server-functions.md (1)
235-235: Consider clarifying hash algorithm specifics.The documentation mentions "SHA256 hashes" but should clarify whether this applies to dev and production or production only, especially given the PR's focus on dev mode behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
docs/start/framework/react/guide/server-functions.md(2 hunks)packages/directive-functions-plugin/src/compilers.ts(2 hunks)packages/directive-functions-plugin/src/index.ts(0 hunks)packages/server-functions-plugin/README.md(2 hunks)packages/server-functions-plugin/src/index.ts(4 hunks)packages/server-functions-plugin/vite.config.ts(1 hunks)packages/start-plugin-core/src/plugin.ts(2 hunks)packages/start-server-core/package.json(1 hunks)packages/start-server-core/src/fake-start-server-fn-manifest.ts(1 hunks)packages/start-server-core/src/getServerFnById.ts(1 hunks)packages/start-server-core/src/loadVirtualModule.ts(0 hunks)packages/start-server-core/src/tanstack-start.d.ts(1 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 (2)
- packages/start-server-core/src/loadVirtualModule.ts
- packages/directive-functions-plugin/src/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/start-server-core/vite.config.tspackages/start-server-core/src/fake-start-server-fn-manifest.tspackages/start-server-core/src/tanstack-start.d.tspackages/directive-functions-plugin/src/compilers.tspackages/start-server-core/src/getServerFnById.tspackages/start-plugin-core/src/plugin.tspackages/server-functions-plugin/vite.config.tspackages/start-server-core/src/virtual-modules.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/vite.config.tspackages/start-server-core/src/fake-start-server-fn-manifest.tspackages/start-server-core/src/tanstack-start.d.tspackages/start-server-core/src/getServerFnById.tspackages/start-plugin-core/src/plugin.tspackages/start-server-core/package.jsonpackages/start-server-core/src/virtual-modules.ts
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use internal docs links relative to the docs/ folder (e.g., ./guide/data-loading)
Files:
docs/start/framework/react/guide/server-functions.md
docs/{router,start}/**
📄 CodeRabbit inference engine (AGENTS.md)
Place router docs under docs/router/ and start framework docs under docs/start/
Files:
docs/start/framework/react/guide/server-functions.md
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
packages/start-server-core/package.json
🧬 Code graph analysis (4)
packages/start-server-core/vite.config.ts (1)
packages/start-server-core/src/virtual-modules.ts (1)
VIRTUAL_MODULES(3-7)
packages/start-server-core/src/tanstack-start.d.ts (1)
packages/start-server-core/src/fake-start-server-fn-manifest.ts (1)
getServerFnById(1-1)
packages/start-plugin-core/src/plugin.ts (3)
packages/start-plugin-core/src/start-compiler-plugin/plugin.ts (1)
startCompilerPlugin(39-116)packages/start-plugin-core/src/create-server-fn-plugin/plugin.ts (1)
createServerFnPlugin(42-175)packages/server-functions-plugin/src/index.ts (1)
TanStackServerFnPlugin(37-227)
packages/server-functions-plugin/src/index.ts (1)
packages/directive-functions-plugin/src/compilers.ts (2)
GenerateFunctionIdFn(26-30)DirectiveFn(12-19)
🪛 ast-grep (0.39.6)
packages/server-functions-plugin/src/index.ts
[warning] 167-167: 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(resolvedManifestVirtualImportId)
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)
🪛 Biome (2.1.2)
packages/start-server-core/src/tanstack-start.d.ts
[error] 15-15: 'async' modifier cannot be used in an ambient context.
(parse)
⏰ 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 (10)
packages/directive-functions-plugin/src/compilers.ts (1)
494-499: FunctionId inputs: prefer sanitized extractedFilename for stability.Now that extractedFilename is available, ensure your GenerateFunctionIdFn prefers it (sanitized as above) when differentiating source vs split files; keep filename for machine‑independent context. No action here if your implementation already does this.
If unsure, please confirm the generator logic.
packages/server-functions-plugin/src/index.ts (1)
71-116: Deterministic IDs in build, encoded file/export in dev — LGTM.Good split: base64url(file/export) in dev, SHA‑256 (or user override) in build with de‑dup suffixing.
If you expect
generateFunctionIdto run outside Node (e.g. non‑Node build adapters), confirmBuffer/cryptoavailability. Otherwise, all good.packages/start-plugin-core/src/plugin.ts (2)
330-330: Plugin order note — LGTM.Compiler before ServerFnPlugin ensures IDs/compilation are ready. Keep as‑is.
333-350: ServerFn plugin wiring confirmed — signature correctly excludes extractedFilename.Verification confirms the code at lines 333-350 properly passes
startPluginOpts?.serverFns?.generateFunctionIdconforming to the new signature. The internal implementation (packages/server-functions-plugin/src/index.ts) explicitly types the user callback asGenerateFunctionIdFnOptional, which omitsextractedFilename. User implementations only receive{filename, functionName}and the e2e test validates this usage pattern correctly.packages/start-server-core/vite.config.ts (1)
21-25: Include fake manifest entry — LGTM.Ensures the stub is built and published.
packages/start-server-core/src/virtual-modules.ts (1)
5-5: LGTM! Virtual module ID migration is consistent.The migration from
'tanstack-start-server-fn-manifest:v'to'#tanstack-start-server-fn-manifest'is correctly applied to both the constant definition and the type mapping.Also applies to: 11-11
packages/start-server-core/src/getServerFnById.ts (1)
1-1: LGTM! Delegation to virtual module aligns with the refactor.The re-export delegates server function resolution to the virtual module, consistent with the PR's unified, function-ID-centric approach.
packages/server-functions-plugin/README.md (2)
32-56: LGTM! Clear client RPC implementation example.The example demonstrates the expected pattern for implementing client-side runtime wrappers using fetch.
63-74: LGTM! Manifest usage updated to async function-based approach.The example correctly demonstrates the new
getServerFnByIdAPI, replacing the static manifest lookup pattern.docs/start/framework/react/guide/server-functions.md (1)
231-231: Verify production-only focus aligns with PR scope.The section title now specifies "for production build", but the PR title mentions dev mode ("no invalidation for server functions in dev"). Does function ID generation work differently in dev vs. production, or should this section cover both environments?
| generateFunctionId: ({ filename, functionName }) => { | ||
| // Return a custom ID string. If you return undefined, the default is used. | ||
| // For example, always hash (even in dev): | ||
| // return createHash('sha256').update(`${filename}--${functionName}`).digest('hex') | ||
| return crypto | ||
| .createHash('sha1') | ||
| .update(`${filename}--${functionName}`) | ||
| .digest('hex') | ||
| return undefined |
There was a problem hiding this comment.
Fix unreachable code and hash algorithm inconsistency.
Two issues:
- Unreachable code: Line 264 has
return undefinedafter line 263 already returns the hash, making it unreachable. - Hash algorithm mismatch: Line 235 states defaults use "SHA256 hashes", but the example uses
sha1(line 261).
Apply this diff:
generateFunctionId: ({ filename, functionName }) => {
// Return a custom ID string. If you return undefined, the default is used.
- return crypto
- .createHash('sha1')
+ return crypto
+ .createHash('sha256')
.update(`${filename}--${functionName}`)
.digest('hex')
- return undefined
},📝 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.
| generateFunctionId: ({ filename, functionName }) => { | |
| // Return a custom ID string. If you return undefined, the default is used. | |
| // For example, always hash (even in dev): | |
| // return createHash('sha256').update(`${filename}--${functionName}`).digest('hex') | |
| return crypto | |
| .createHash('sha1') | |
| .update(`${filename}--${functionName}`) | |
| .digest('hex') | |
| return undefined | |
| generateFunctionId: ({ filename, functionName }) => { | |
| // Return a custom ID string. If you return undefined, the default is used. | |
| return crypto | |
| .createHash('sha256') | |
| .update(`${filename}--${functionName}`) | |
| .digest('hex') | |
| }, |
🤖 Prompt for AI Agents
In docs/start/framework/react/guide/server-functions.md around lines 258 to 264,
the example for generateFunctionId contains an unreachable `return undefined`
after returning a hash and uses `sha1` while the surrounding text says SHA256;
remove the trailing `return undefined` and replace `sha1` with `sha256` so the
function consistently returns a SHA-256 hex digest (or if you intended to allow
undefined, make the return conditional instead).
| export type GenerateFunctionIdFn = (opts: { | ||
| filename: string | ||
| functionName: string | ||
| extractedFilename: string | ||
| }) => string | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Threading extractedFilename into ID generation looks good; verify call sites updated.
The new GenerateFunctionIdFn signature adds extractedFilename. Ensure all implementations and usages across the repo are updated to avoid type drift.
Run to locate and validate implementors:
🏁 Script executed:
#!/bin/bash
# Find typedef and implementors/usages
rg -nP --type=ts 'type\s+GenerateFunctionIdFn\b|interface\s+GenerateFunctionIdFn\b'
rg -nP --type=ts '\bgenerateFunctionId\s*:\s*\(' -C3
rg -nP --type=ts '\bgenerateFunctionId\(\s*\{[^}]*\}\s*\)' -C2Length of output: 712
🏁 Script executed:
#!/bin/bash
# Find all generateFunctionId call sites and implementations more broadly
echo "=== All generateFunctionId property definitions ==="
rg -nP --type=ts 'generateFunctionId\s*[:=]' -C5
echo ""
echo "=== All places where generateFunctionId is called ==="
rg -nP --type=ts 'generateFunctionId\s*\(' -C3
echo ""
echo "=== Files containing generateFunctionId ==="
fd --type f -e ts -e tsx | xargs rg -l 'generateFunctionId'Length of output: 9970
🏁 Script executed:
#!/bin/bash
# Check the schema definition for generateFunctionId
echo "=== Schema definition for generateFunctionId ==="
sed -n '162,175p' packages/start-plugin-core/src/schema.ts
echo ""
echo "=== Full call site in server-functions-plugin ==="
sed -n '90,102p' packages/server-functions-plugin/src/index.ts
echo ""
echo "=== Full call site in directive-functions-plugin ==="
sed -n '490,500p' packages/directive-functions-plugin/src/compilers.tsLength of output: 1387
Type signature mismatch: GenerateFunctionIdFn requires extractedFilename but not all call sites provide it.
The reviewed change adds extractedFilename as a required parameter to GenerateFunctionIdFn, but this creates a type safety violation:
- directive-functions-plugin (compilers.ts:494) passes
{filename, functionName, extractedFilename}✓ - server-functions-plugin (index.ts:95) passes only
{filename, functionName}✗ - Schema (start-plugin-core/src/schema.ts:162) defines args as
{filename, functionName}only—does not includeextractedFilename
In strict TypeScript mode, the server-functions-plugin call will fail type checking. Either:
- Make
extractedFilenameoptional in GenerateFunctionIdFn and update the schema accordingly, or - Update server-functions-plugin to pass
extractedFilenameand update the schema to include it
🤖 Prompt for AI Agents
In packages/directive-functions-plugin/src/compilers.ts around lines 26–31, the
new GenerateFunctionIdFn requires extractedFilename but other call sites and the
schema only provide filename and functionName; make extractedFilename optional
on the GenerateFunctionIdFn type (e.g., extractedFilename?: string) and update
any related schema/type definitions (start-plugin-core/src/schema.ts) to declare
extractedFilename as optional too, then adjust callers (server-functions-plugin
index.ts and any others) to either pass extractedFilename when available or rely
on the optional property so TypeScript strict mode typechecks successfully.
| server: { | ||
| getRuntimeCode: () => | ||
| `import { createServerRpc } from '@tanstack/react-start/server-runtime'`, | ||
| replacer: (opts) => | ||
| `createServerRpc(${JSON.stringify(opts.functionId)}, ${opts.fn})`, | ||
| `import { createServerRpc } from '@tanstack/${corePluginOpts.framework}-start/server-rpc'`, | ||
| replacer: (d) => `createServerRpc('${d.functionId}', ${d.fn})`, | ||
| envName: 'ssr', | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify envName value for server configuration.
The server configuration has envName: 'ssr' (line 25), which seems inconsistent. Should this be 'server' instead? The client has envName: 'client', so having the server marked as 'ssr' appears counterintuitive.
🏁 Script executed:
#!/bin/bash
# Search for envName usage patterns in server-functions-plugin and related packages
rg -nP --type=ts "envName:\s*['\"]" -A2 -B2Length of output: 665
I found that directive-functions-plugin uses envName: 'server' for the server configuration. Let me verify what the actual implementation in server-functions-plugin uses:
🏁 Script executed:
#!/bin/bash
# Search for envName in server-functions-plugin source code (not README)
rg -n "envName" packages/server-functions-plugin/src --type=tsLength of output: 816
🏁 Script executed:
#!/bin/bash
# Also check the actual server-functions-plugin README to see full context
head -30 packages/server-functions-plugin/README.mdLength of output: 990
Update README.md to use correct envName value for server configuration.
The README documentation incorrectly shows envName: 'ssr' on line 25, but the actual implementation defaults to envName: 'server' (source: src/index.ts:48). This inconsistency should be fixed in the README to match the implementation and maintain consistency with the client configuration pattern (envName: 'client').
Update line 25 from:
envName: 'ssr',
to:
envName: 'server',
🤖 Prompt for AI Agents
In packages/server-functions-plugin/README.md around lines 21 to 26, the README
shows envName: 'ssr' which is inconsistent with the implementation
(src/index.ts:48) that defaults to 'server'; update the README to use envName:
'server' so it matches the code and the client pattern.
| "imports": { | ||
| "#tanstack-start-server-fn-manifest": { | ||
| "default": "./dist/esm/fake-start-server-fn-manifest.js" | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add a types condition for the import-mapped module.
TypeScript under NodeNext resolves "imports" with conditions. Provide a "types" target so consumers get declarations for #tanstack-start-server-fn-manifest.
Apply:
"imports": {
"#tanstack-start-server-fn-manifest": {
- "default": "./dist/esm/fake-start-server-fn-manifest.js"
+ "types": "./dist/esm/fake-start-server-fn-manifest.d.ts",
+ "default": "./dist/esm/fake-start-server-fn-manifest.js"
}
},🤖 Prompt for AI Agents
In packages/start-server-core/package.json around lines 58-62, the "imports"
entry for "#tanstack-start-server-fn-manifest" lacks a "types" condition so
TypeScript consumers won't get declaration files; add a "types" condition
alongside "default" pointing to the package's .d.ts declaration for that module
(for example "./dist/esm/fake-start-server-fn-manifest.d.ts"), ensuring the
"types" entry is a sibling of "default" under the import mapping so
NodeNext/TypeScript can resolve types correctly.
| @@ -0,0 +1 @@ | |||
| export function getServerFnById() {} | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make the stub explicit and typed.
Throw with a clear message so accidental runtime usage is obvious; provide a typed async signature.
-export function getServerFnById() {}
+/**
+ * Stub implementation used during package build and as a fallback.
+ * The real implementation is provided by the Start plugin via a virtual module.
+ */
+export async function getServerFnById(_id: string): Promise<unknown> {
+ throw new Error(
+ '#tanstack-start-server-fn-manifest is a virtual module. Ensure the Start Vite plugin is active.',
+ )
+}📝 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.
| export function getServerFnById() {} | |
| /** | |
| * Stub implementation used during package build and as a fallback. | |
| * The real implementation is provided by the Start plugin via a virtual module. | |
| */ | |
| export async function getServerFnById(_id: string): Promise<unknown> { | |
| throw new Error( | |
| '#tanstack-start-server-fn-manifest is a virtual module. Ensure the Start Vite plugin is active.', | |
| ) | |
| } |
🤖 Prompt for AI Agents
packages/start-server-core/src/fake-start-server-fn-manifest.ts lines 1-1:
current export is an untyped empty stub; replace it with an explicit async typed
signature (for example async function getServerFnById<T = unknown>(id: string):
Promise<T>) and make the body immediately throw a clear runtime error (e.g.
throw new Error("getServerFnById is a stub and must not be called at runtime;
use the real implementation")). Ensure the function is exported and its
signature reflects asynchronous usage so accidental runtime invocation fails
fast with a clear message.
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 (3)
packages/server-functions-plugin/package.json(0 hunks)packages/start-server-core/src/index.tsx(0 hunks)packages/start-server-core/src/virtual-modules.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/start-server-core/src/index.tsx
- packages/server-functions-plugin/package.json
🧰 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/virtual-modules.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/virtual-modules.ts
⏰ 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 (1)
packages/start-server-core/src/virtual-modules.ts (1)
5-5: No action required. The migration is complete and correct.The change from the virtual module protocol
'tanstack-start-server-fn-manifest:v'to the Node.js subpath import'#tanstack-start-server-fn-manifest'has been fully implemented:
- All references to the old identifier have been updated (none found)
- The subpath import is properly configured in
package.json- The removal of
serverFnManifestfrom theVirtualModulestype is intentional: it's kept in theVIRTUAL_MODULESconstant as a configuration reference but no longer loaded throughloadVirtualModule()since it's now resolved as a subpath import- External packages like
start-plugin-corecorrectly use the constant value for build configuration
| export type VirtualModules = { | ||
| [VIRTUAL_MODULES.startManifest]: typeof import('tanstack-start-manifest:v') | ||
| [VIRTUAL_MODULES.serverFnManifest]: typeof import('tanstack-start-server-fn-manifest:v') | ||
| [VIRTUAL_MODULES.injectedHeadScripts]: typeof import('tanstack-start-injected-head-scripts:v') | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify intentional exclusion from VirtualModules type.
The serverFnManifest remains in the VIRTUAL_MODULES constant but was removed from the VirtualModules type. This creates an apparent inconsistency. Confirm this exclusion is intentional and that no code relies on VirtualModules[VIRTUAL_MODULES.serverFnManifest] type mapping.
🏁 Script executed:
#!/bin/bash
# Description: Check if any code expects the VirtualModules type to include serverFnManifest.
# Search for usage of VirtualModules type with serverFnManifest
echo "=== Checking for VirtualModules type usage with serverFnManifest ==="
rg -nP 'VirtualModules\[.*serverFnManifest.*\]' -A 2 -B 2
# Search for direct imports or type references that might expect this mapping
echo -e "\n=== Checking for type references to VirtualModules ==="
ast-grep --pattern 'type $_ = VirtualModules[$_]'Length of output: 124
🏁 Script executed:
#!/bin/bash
# Display VIRTUAL_MODULES constant definition
echo "=== VIRTUAL_MODULES constant definition ==="
rg -nP -A 10 'export const VIRTUAL_MODULES|const VIRTUAL_MODULES|VIRTUAL_MODULES\s*=' --type ts --type tsx packages/start-server-core/src/
# Check for any serverFnManifest usage in codebase
echo -e "\n=== serverFnManifest usage across codebase ==="
rg -nP 'serverFnManifest' -C 3 --type ts --type tsx packages/
# Check git history if available for this file
echo -e "\n=== Recent changes to virtual-modules.ts ==="
git log --oneline -10 packages/start-server-core/src/virtual-modules.ts 2>/dev/null || echo "Git history not available"Length of output: 345
🏁 Script executed:
#!/bin/bash
# Display the actual virtual-modules.ts file to see full context
echo "=== Full virtual-modules.ts file ==="
cat packages/start-server-core/src/virtual-modules.ts
# Check for serverFnManifest usage without type flags
echo -e "\n=== serverFnManifest usage across codebase ==="
rg -nP 'serverFnManifest' -C 2 -g 'packages/**/*.{ts,js}'
# Check git diff for the specific commit about server functions
echo -e "\n=== Git diff for 'fix: no invalidation for server functions in dev' ==="
git show a69f375c -- packages/start-server-core/src/virtual-modules.ts 2>/dev/null | head -50Length of output: 3381
🏁 Script executed:
#!/bin/bash
# Check what the "fix build" commit changed in virtual-modules.ts
echo "=== Git diff for 'fix build' commit (618d8580) ==="
git show 618d8580 -- packages/start-server-core/src/virtual-modules.ts 2>/dev/null
# Also check the full diff to see all changes in that commit
echo -e "\n=== Commit message and stats ==="
git show 618d8580 --stat 2>/dev/null | head -30Length of output: 1220
🏁 Script executed:
#!/bin/bash
# Check how VirtualModules type is actually used - is it indexed?
echo "=== VirtualModules type indexing usage ==="
rg -nP 'VirtualModules\s*\[' -C 3 -g 'packages/**/*.{ts,js}'
# Check if any code uses the type to access the manifest property
echo -e "\n=== Accessing VIRTUAL_MODULES.serverFnManifest at runtime ==="
rg -nP 'VIRTUAL_MODULES\.serverFnManifest' -C 2 -g 'packages/**/*.{ts,js}' | head -30
# Check if there are any type errors or comments about this
echo -e "\n=== Comments mentioning serverFnManifest or virtual modules type ==="
rg -nP '(TODO|FIXME|BUG|NOTE).*virtual|serverFn' -C 2 -g 'packages/**/*.{ts,js}' | head -30Length of output: 4694
Restore missing serverFnManifest entry to VirtualModules type.
The serverFnManifest remains in the VIRTUAL_MODULES constant but was removed from the VirtualModules type. This creates a type safety issue: the loadVirtualModule function (in packages/start-server-core/src/loadVirtualModule.ts) expects id: TId extends keyof VirtualModules, so passing VIRTUAL_MODULES.serverFnManifest to it would cause a TypeScript error.
The removal appears unintentional (commit 618d8580 "fix build" lacks explanation). Restore the type mapping to match the constant and maintain type safety for the generic loadVirtualModule function.
🤖 Prompt for AI Agents
In packages/start-server-core/src/virtual-modules.ts around lines 9 to 12, the
VirtualModules type is missing the mapping for VIRTUAL_MODULES.serverFnManifest
which was removed accidentally; restore a line mapping that key to the correct
module import type (e.g. add an entry like [VIRTUAL_MODULES.serverFnManifest]:
typeof import('tanstack-start-server-fn-manifest:v') so the VirtualModules type
matches the VIRTUAL_MODULES constant and keeps loadVirtualModule's id generic
typing correct).
Summary by CodeRabbit
Documentation
Refactor
Breaking Changes