feat(vercel): allow overriding function config by route#4124
feat(vercel): allow overriding function config by route#4124RihanArfan wants to merge 6 commits intomainfrom
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:
📝 WalkthroughWalkthroughAdds a Vercel preset option Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/presets/vercel/utils.ts`:
- Around line 336-342: The route-function config emission currently iterates
Object.keys(nitro.options.vercel.routeFunctionConfig) which yields
nondeterministic order and can let wildcards shadow specific routes; update the
logic that maps over nitro.options.vercel.routeFunctionConfig to first sort the
route patterns by specificity (e.g., more static segments and
fewer/wildcards/params first) so specific patterns come before wildcards, and
ensure the generated src uses nitro.options.baseURL as a prefix (apply the same
baseURL handling used for observability routes) before calling
normalizeRouteSrc; keep dest generation via
withLeadingSlash(normalizeRouteDest(pattern)) unchanged.
- Around line 597-598: The current use of defu to create mergedConfig
(defu(overrides, baseFunctionConfig)) concatenates array fields like regions or
experimentalTriggers instead of letting per-route overrides replace them; change
the merge to treat arrays as replace-not-concat by implementing a custom merge
step: when combining overrides and baseFunctionConfig for mergedConfig, copy all
scalar/object fields normally but if a field exists in overrides and is an array
(e.g., overrides.regions, overrides.experimentalTriggers) then set
mergedConfig[field] = overrides[field] rather than concatenating; update the
logic around mergedConfig creation (the call site using defu, plus any
helper/merge function you add) so writeFile(resolve(funcDir, ".vc-config.json"),
JSON.stringify(mergedConfig, null, 2)) receives the corrected config.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 62396395-9a97-4ee6-8df2-4c9c7bbd66bf
📒 Files selected for processing (3)
src/presets/vercel/types.tssrc/presets/vercel/utils.tstest/presets/vercel.test.ts
| // Route function config routes | ||
| ...(nitro.options.vercel?.routeFunctionConfig | ||
| ? Object.keys(nitro.options.vercel.routeFunctionConfig).map((pattern) => ({ | ||
| src: normalizeRouteSrc(pattern), | ||
| dest: withLeadingSlash(normalizeRouteDest(pattern)), | ||
| })) | ||
| : []), |
There was a problem hiding this comment.
Route-function routes should be specificity-ordered and baseURL-aware.
Current emission order depends on object key insertion, so wildcard patterns can shadow specific ones. Also, these src routes are not prefixed with nitro.options.baseURL, unlike observability routes.
💡 Proposed fix
+ const routeFunctionPatterns = nitro.options.vercel?.routeFunctionConfig
+ ? Object.keys(nitro.options.vercel.routeFunctionConfig).sort(
+ (a, b) => b.split(/\/(?!\*)/).length - a.split(/\/(?!\*)/).length
+ )
+ : [];
+
config.routes!.push(
@@
- ...(nitro.options.vercel?.routeFunctionConfig
- ? Object.keys(nitro.options.vercel.routeFunctionConfig).map((pattern) => ({
- src: normalizeRouteSrc(pattern),
+ ...routeFunctionPatterns.map((pattern) => ({
+ src: joinURL(nitro.options.baseURL, normalizeRouteSrc(pattern)),
dest: withLeadingSlash(normalizeRouteDest(pattern)),
- }))
- : []),
+ })),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/presets/vercel/utils.ts` around lines 336 - 342, The route-function
config emission currently iterates
Object.keys(nitro.options.vercel.routeFunctionConfig) which yields
nondeterministic order and can let wildcards shadow specific routes; update the
logic that maps over nitro.options.vercel.routeFunctionConfig to first sort the
route patterns by specificity (e.g., more static segments and
fewer/wildcards/params first) so specific patterns come before wildcards, and
ensure the generated src uses nitro.options.baseURL as a prefix (apply the same
baseURL handling used for observability routes) before calling
normalizeRouteSrc; keep dest generation via
withLeadingSlash(normalizeRouteDest(pattern)) unchanged.
| * } | ||
| * ``` | ||
| */ | ||
| routeFunctionConfig?: Record<string, VercelServerlessFunctionConfig>; |
There was a problem hiding this comment.
@pi0 what should we call the key? I'm not satisfied with this, just used it as a placeholder for now. Also as it accepts a list of routes perhaps the key should be plural with routeFunctionsConfig if we decide not to change it to something else altogether?
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/2.deploy/20.providers/vercel.md (1)
64-83: Consider adding a wildcard pattern example.The example only shows exact path patterns (e.g.,
/api/heavy-computation), but line 85 mentions wildcard support via rou3 matching. Including at least one wildcard pattern example in the main code block would better demonstrate this capability.💡 Example with wildcard pattern
export default defineNitroConfig({ vercel: { routeFunctionConfig: { "/api/heavy-computation": { maxDuration: 800, memory: 4096, }, + "/api/slow/**": { + maxDuration: 300, + }, "/api/regional": { regions: ["lhr1", "cdg1"], },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/2.deploy/20.providers/vercel.md` around lines 64 - 83, Add a wildcard route example to the existing vercel.routeFunctionConfig in the defineNitroConfig snippet so readers see rou3 matching in practice: update the routeFunctionConfig block (the vercel object used with defineNitroConfig) to include at least one pattern using a wildcard (e.g., a prefix or glob-style path) alongside the existing exact paths, and show any relevant keys (maxDuration, memory, regions, experimentalTriggers) applied to that wildcard entry so the example demonstrates how to configure wildcard routes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/2.deploy/20.providers/vercel.md`:
- Line 60: The docs for vercel.routeFunctionConfig are ambiguous about array
property merging; update the sentence describing merge behavior to explicitly
state that array properties (e.g., regions) in a route's partial config
completely replace the corresponding arrays from the base vercel.functions
config rather than being concatenated/merged, and add a short example or note
referencing vercel.routeFunctionConfig and vercel.functions to make this
replacement behavior clear (this follows the behavior implemented in the
route-merge logic around the array handling in the vercel utils).
---
Nitpick comments:
In `@docs/2.deploy/20.providers/vercel.md`:
- Around line 64-83: Add a wildcard route example to the existing
vercel.routeFunctionConfig in the defineNitroConfig snippet so readers see rou3
matching in practice: update the routeFunctionConfig block (the vercel object
used with defineNitroConfig) to include at least one pattern using a wildcard
(e.g., a prefix or glob-style path) alongside the existing exact paths, and show
any relevant keys (maxDuration, memory, regions, experimentalTriggers) applied
to that wildcard entry so the example demonstrates how to configure wildcard
routes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a493dd7-ebeb-46f3-be6c-5f3d7310a649
📒 Files selected for processing (1)
docs/2.deploy/20.providers/vercel.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/presets/vercel.test.ts (1)
170-177: Avoid locking duplicate route entries into the snapshot.
/api/helloand/api/echoare expected here and again at Line 315-Line 325. Keeping both in expected output can normalize redundant route generation instead of surfacing it.♻️ Suggested guard for duplicates
expect(config).toMatchInlineSnapshot(`...`); + + const apiRoutePairs = config.routes + .filter((r: any) => r?.src === "/api/hello" || r?.src === "/api/echo") + .map((r: any) => `${r.src}->${r.dest}`); + expect(new Set(apiRoutePairs).size).toBe(apiRoutePairs.length);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/presets/vercel.test.ts` around lines 170 - 177, The snapshot contains duplicate route entries (e.g., "/api/hello" and "/api/echo") which mask redundant generation; update the test to deduplicate before asserting by filtering the expected routes (the array used for the snapshot, e.g., expectedRoutes) to keep only unique entries (compare on dest and src) — for example, apply a filter or build a Set/Map keyed by `${dest}|${src}` so the snapshot only contains the first occurrence and remove the repeated literal entries from the expected list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/presets/vercel.test.ts`:
- Around line 170-177: The snapshot contains duplicate route entries (e.g.,
"/api/hello" and "/api/echo") which mask redundant generation; update the test
to deduplicate before asserting by filtering the expected routes (the array used
for the snapshot, e.g., expectedRoutes) to keep only unique entries (compare on
dest and src) — for example, apply a filter or build a Set/Map keyed by
`${dest}|${src}` so the snapshot only contains the first occurrence and remove
the repeated literal entries from the expected list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13c5d6de-4764-4ade-ad45-251b4d2ed4ce
📒 Files selected for processing (2)
test/fixture/nitro.config.tstest/presets/vercel.test.ts
🔗 Linked issue
Closes #3815
❓ Type of change
📚 Description
Allows setting Vercel function configuration (https://vercel.com/docs/build-output-api/primitives#serverless-function-configuration) on a per-function basis. It continues to symlink the existing to __server.func besides .vc-config.json for functions which match the routes which has custom config.
This unblocks future work for supporting Vercel Queues within Nitro as we'll be able to add triggers to a new queue handler route.
📝 Checklist