Add support for customizable functionId generation.
#5373
Conversation
If not provided, the default (basename_functionName) will be used in development and sha256 of the default for production
WalkthroughAdds a customizable generateFunctionId hook and threads it through directive and server function plugins, Start plugin schema/options, and E2E configs; normalizes filenames with path.relative, centralizes deduplicated URL-safe functionId generation, and adds the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Dev Config
participant DF as Directive Plugin
participant SF as Server Fn Plugin
participant Gen as buildGenerateFunctionId
participant MF as Manifest
Dev->>DF: supply generateFunctionId? (filename, functionName)
Dev->>SF: supply generateFunctionId? / envName?
DF->>Gen: request id (relativeFilename, functionName)
Gen-->>DF: candidate id (user or seeded)
DF->>Gen: normalize URL-safe & dedupe
DF-->>MF: register handler with final id
SF->>Gen: same generation & dedupe flow
SF-->>MF: include ids in manifest
MF-->>Dev: emitted manifest with unified ids
sequenceDiagram
autonumber
participant Client as Client
participant Server as Server Runtime
participant MF as Manifest
Client->>Server: POST /_serverFn/<id>/...
Server->>MF: lookup <id>
MF-->>Server: handler reference
Server-->>Client: invoke handler -> response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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)
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 ad7bd9d
☁️ 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: |
| serverFns: z | ||
| .object({ | ||
| base: z.string().optional().default('/_serverFn'), | ||
| functionId: z |
There was a problem hiding this comment.
we need a better, more descriptive name here.
functionId makes it seem this IS the function id, but it is actually a function that generated functionIds
how about:
generateFunctionIdcreateFunctionId
?
i think we should call it with the raw file name and raw function name as props instead of that single string argument
| functionId = deduplicatedId | ||
| } | ||
| } | ||
| functionId = makeFunctionIdUrlSafe(functionId) |
There was a problem hiding this comment.
i wouldnt postprocess the function id anymore.
the function id that is returned by the function id generator function must be url safe. we need to document this requirement.
then the invocation of makeFunctionIdUrlSafe can simply go into the default function id generator function for dev
There was a problem hiding this comment.
Done. ID generation and deduplicatoin is now the responsibility of server-functions-plugin
| let deduplicatedId = generatedId | ||
| let iteration = 0 | ||
| do { | ||
| deduplicatedId = `${deduplicatedId}_${++iteration}` |
There was a problem hiding this comment.
i am wondering whether we should call the function id generator function a single time at the end of the build with all of the functions?
then the function id generator function is responsible to provide unique function ids.
this ensures that we dont modfiy any of the function ids supplied by the user.
There was a problem hiding this comment.
We need the functionId at generation time for the replacers. We cant defer this up to the end
…on. Add support for an optional `undefined` return and streamline generation process.
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
e2e/react-start/custom-basepath/tests/navigation.spec.ts(1 hunks)e2e/react-start/server-functions/tests/server-functions.spec.ts(1 hunks)e2e/react-start/server-functions/vite.config.ts(1 hunks)packages/directive-functions-plugin/src/compilers.ts(6 hunks)packages/server-functions-plugin/src/index.ts(10 hunks)packages/start-plugin-core/src/schema.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/start-plugin-core/src/schema.ts
🧰 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:
e2e/react-start/server-functions/tests/server-functions.spec.tspackages/server-functions-plugin/src/index.tspackages/directive-functions-plugin/src/compilers.tse2e/react-start/custom-basepath/tests/navigation.spec.tse2e/react-start/server-functions/vite.config.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/server-functions/tests/server-functions.spec.tse2e/react-start/custom-basepath/tests/navigation.spec.tse2e/react-start/server-functions/vite.config.ts
🧬 Code graph analysis (1)
packages/server-functions-plugin/src/index.ts (2)
packages/directive-functions-plugin/src/compilers.ts (1)
FunctionIdFn(26-26)packages/directive-functions-plugin/src/index.ts (1)
FunctionIdFn(20-20)
⏰ 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
…cy across packages. Adjust associated logic and type definitions. Only the internal implementation converts to url safe. If a consumer wants to use their own generator, the responsibility of generating safe urls is on them
…y. Update related types, remove legacy `currentId`, and streamline deduplication logic. directive-functions-plugin does not generate functionIds anymore. This is the responsibility of the server-functions-plugin
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/directive-functions-plugin/src/compilers.ts (1)
463-466: Consider simplifying the type assertion.The type assertion is technically correct but verbose. Since
split('?')always returns a non-empty array, you could simplify this to:const [baseFilename = '', ..._searchParams] = opts.filename.split('?')Or if you prefer the assertion for explicitness, the current code is fine.
packages/server-functions-plugin/src/index.ts (1)
283-322: Document GenerateFunctionIdFnOptional behavior
Add JSDoc aboveGenerateFunctionIdFnOptionalinpackages/server-functions-plugin/src/index.tsexplaining that the returned ID must be URL-safe (-,_only), returningundefinedtriggers the built-in default, and duplicate IDs are automatically deduplicated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
e2e/react-start/server-functions/vite.config.ts(1 hunks)packages/directive-functions-plugin/src/compilers.ts(5 hunks)packages/directive-functions-plugin/src/index.ts(6 hunks)packages/server-functions-plugin/src/index.ts(9 hunks)packages/start-plugin-core/src/plugin.ts(1 hunks)packages/start-plugin-core/src/schema.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/start-plugin-core/src/plugin.ts
- packages/directive-functions-plugin/src/index.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:
e2e/react-start/server-functions/vite.config.tspackages/start-plugin-core/src/schema.tspackages/server-functions-plugin/src/index.tspackages/directive-functions-plugin/src/compilers.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/server-functions/vite.config.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/src/schema.ts
🧬 Code graph analysis (3)
e2e/react-start/server-functions/vite.config.ts (1)
packages/router-core/src/route.ts (1)
id(1547-1549)
packages/server-functions-plugin/src/index.ts (1)
packages/directive-functions-plugin/src/compilers.ts (1)
GenerateFunctionIdFn(26-29)
packages/directive-functions-plugin/src/compilers.ts (1)
packages/directive-functions-plugin/src/index.ts (1)
GenerateFunctionIdFn(20-20)
⏰ 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 (6)
e2e/react-start/server-functions/vite.config.ts (1)
6-24: LGTM! Clean test setup for constant ID behavior.The E2E test configuration correctly demonstrates the customizable
generateFunctionIdAPI. The approach of returning'constant_id'for specific functions andundefinedfor others (to preserve default behavior) is clear and appropriate for testing.packages/start-plugin-core/src/schema.ts (1)
162-171: LGTM! Schema correctly defines the customizable function ID API.The
generateFunctionIdfield is properly defined with the correct signature: takes{ filename, functionName }and returns an optional string. This matches theGenerateFunctionIdFnOptionaltype and allows users to customize ID generation while falling back to defaults whenundefinedis returned.packages/directive-functions-plugin/src/compilers.ts (3)
8-8: Good choice usingpathefor cross-platform path normalization.Using
patheensures consistent forward-slash separators across Windows, macOS, and Linux, which is essential for generating stable function IDs regardless of the development platform.Based on learnings.
26-29: LGTM! Clear type definition for required function ID generator.The
GenerateFunctionIdFntype correctly defines the required variant (non-optional return) that's used internally after the user's optional function is wrapped with defaults and deduplication.
472-478: Excellent normalization strategy for consistent function IDs.Using
path.relativeto compute paths relative to the project root ensures that function IDs remain consistent across different machines and environments, which is critical for matching client and server function calls.packages/server-functions-plugin/src/index.ts (1)
275-281: LGTM! URL-safety sanitization is correct.The
makeFunctionIdUrlSafefunction properly sanitizes IDs by:
- Replacing unsafe characters with underscores
- Collapsing multiple underscores
- Trimming leading/trailing underscores
- Preserving the
--joiner
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/directive-functions-plugin/tests/compiler.test.ts (1)
6-12: Add JSDoc to clarify the transformation logic.The
makeFunctionIdUrlSafefunction performs several sequential transformations that work together to produce clean, URL-safe identifiers. The cleanup on line 11 (replace(/_--/g, '--')) preserves the--separator even when the character immediately before it was sanitized to_. This logic is subtle and would benefit from a brief JSDoc comment explaining the transformation sequence and its purpose.Consider adding documentation like:
+/** + * Sanitizes a location string to be URL-safe for function IDs. + * Transformations applied in sequence: + * 1. Replace non-alphanumeric characters (except `-` and `_`) with `_` + * 2. Collapse multiple consecutive underscores into one + * 3. Trim leading/trailing underscores + * 4. Clean up `_--` patterns to preserve the `--` separator + */ function makeFunctionIdUrlSafe(location: string): string { return location .replace(/[^a-zA-Z0-9-_]/g, '_') // Replace unsafe chars with underscore .replace(/_{2,}/g, '_') // Collapse multiple underscores .replace(/^_|_$/g, '') // Trim leading/trailing underscores .replace(/_--/g, '--') // Clean up the joiner }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/directive-functions-plugin/tests/compiler.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/directive-functions-plugin/tests/compiler.test.ts
🧬 Code graph analysis (1)
packages/directive-functions-plugin/tests/compiler.test.ts (1)
packages/directive-functions-plugin/src/compilers.ts (1)
CompileDirectivesOpts(41-52)
⏰ 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). (1)
- GitHub Check: Test
🔇 Additional comments (2)
packages/directive-functions-plugin/tests/compiler.test.ts (2)
20-55: LGTM! Config updates correctly demonstrate the feature.The addition of the
generateFunctionIdoption across all three config objects (client, SSR, and server) properly demonstrates the new customization capability. The filename update to./test-files/test.tsmaintains consistency with the root path, and the test snapshots confirm that the custom function ID generation is working as expected throughout all test cases.
14-18: Path normalization is already handled by core code. The default implementation callspath.relative(opts.root, …)before invokinggenerateFunctionId, so the test helper correctly receives normalized filenames.
… server functions.
If not provided, the default (basename_functionName) will be used in development and sha256 of the default for production
Summary by CodeRabbit