Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthroughThis PR adds comprehensive JSDoc documentation to Nitro's exported type definitions across src/types/, including configuration, handler, module, routing, OpenAPI, and runtime types. One structural change removes an index signature from NitroMeta; all other modifications are documentation-only. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
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/types/config.ts`:
- Line 455: Update the cron expression in the scheduledTasks example to the
correct syntax by replacing the invalid string "**/5 * * * *" with "*/5 * * *
*" in the scheduledTasks documentation or example (search for the scheduledTasks
constant/type or the doc block containing that cron example in
src/types/config.ts and update the example string).
In `@src/types/nitro.ts`:
- Around line 18-21: The removal of the index signature from the NitroMeta
interface breaks assignability for consumers that add arbitrary keys; restore
backward compatibility by reintroducing the index signature (e.g., add [key:
string]: any to NitroMeta) or, if you intend this as a breaking change,
explicitly mark it in the changelog/release notes and add migration guidance
explaining how callers should migrate off storing extra keys; locate NitroMeta
in the src/types/nitro.ts file to apply one of these two remedies.
🪄 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: fc91ea1a-75db-4cc9-9f60-37ac3fa50912
📒 Files selected for processing (8)
src/types/config.tssrc/types/handler.tssrc/types/module.tssrc/types/nitro.tssrc/types/openapi.tssrc/types/route-rules.tssrc/types/runtime/cache.tssrc/types/runtime/nitro.ts
commit: |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
docs/1.docs/50.websocket.md (1)
113-147: Consider varying repeated “Called …” hook descriptions.The lifecycle hook section reads a bit repetitive; rephrasing one or two lead sentences would improve flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/1.docs/50.websocket.md` around lines 113 - 147, The lifecycle hook descriptions for open, message, close, and error are repetitive — change one or two lead sentences to vary wording while keeping meaning; for example, rephrase the `open(peer)` intro to "Invoked when a connection is established" and the `message(peer, message)` intro to "Triggered on incoming messages", leaving `close(peer, details)` and `error` as-is (or similarly vary them) so the hooks `open`, `message`, `close`, and `error` are clearer and the section reads less repetitive.src/presets/vercel/runtime/queue-handler.ts (1)
10-11: Avoidconsole.errorhere to match source logging rules.On Line 10,
console.erroris used insrc/presets/...; this violates the repo logging guideline forsrc/**/*.As per coding guidelines: `src/**/*.{ts,js}`: Use `console` only in `src/runtime/` code; use `nitro.logger` in build/dev code or `consola` as fallback.Proposed patch
export default defineHandler((event) => { return handleCallback(async (message, metadata) => { try { await useNitroHooks().callHook("vercel:queue", { message, metadata, send }); } catch (error) { - console.error("[vercel:queue]", error); useNitroApp().captureError?.(error as Error, { event, tags: ["vercel:queue"] }); throw error; } })(event.req as Request); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/presets/vercel/runtime/queue-handler.ts` around lines 10 - 11, Replace the direct console.error call with the repo-approved logger: get the Nitro logger (via useNitroApp()/nitro?.logger) or fall back to consola and call logger.error with the same context; specifically, where console.error("[vercel:queue]", error) currently appears in queue-handler.ts, change it to use the Nitro/logger error method and preserve passing the error and contextual tags (the surrounding code uses useNitroApp().captureError?.(error as Error, { event, tags: ["vercel:queue"] }) so ensure the logger message includes the "[vercel:queue]" tag and the error object and only use consola if nitro.logger is not available).
🤖 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`:
- Around line 151-169: The example uses defineNitroConfig but should use
defineConfig for consistency with the rest of the docs; update the example in
the nitro.config.ts snippet to replace defineNitroConfig with defineConfig and
ensure the snippet imports defineConfig from "nitro" (or uses the same import
style as other examples on the page) so the config block uses defineConfig({
vercel: { queues: { ... } } });.
In `@package.json`:
- Around line 175-185: Update the zephyr-agent version alignment in package.json
so peerDependencies matches what we test in devDependencies: either widen the
peer range from "zephyr-agent": "^0.2.0" to include 1.x (e.g., "^1.0.0" or
"^1.0.0 || ^0.2.0") inside the peerDependencies block, or instead pin
devDependencies' zephyr-agent back to the advertised "^0.2.0"; adjust the
"zephyr-agent" entry in peerDependencies (and devDependencies if you choose to
change that) so both entries are consistent.
In `@src/runtime/internal/routes/dev-tasks.ts`:
- Around line 9-10: The payload construction uses
Object.fromEntries(event.url.searchParams.entries()) which drops duplicate query
keys; change the logic that builds payload (the const payload in this handler)
to iterate over event.url.searchParams.keys() or entries and use
event.url.searchParams.getAll(key) so that keys with multiple values become
arrays (and single values remain scalars), ensuring ?array=1&array=2 yields
["1","2"] instead of "2"; update the payload assembly to map each key to either
a single value or the array returned by getAll().
- Around line 8-12: The current parsing uses event.req.json().catch(() => ({}))
which hides malformed JSON and allows POST tasks to run; change this in the
handler that builds body/payload so JSON syntax errors return a 400 response
instead of falling back to {}: explicitly try to parse event.req.json(), on a
SyntaxError (or JSON parse failure) return a 400 with an explanatory message,
but preserve the empty-object fallback only when the request has no body (e.g.,
Content-Length === 0 or missing body), and then continue building payload from
the validated body and event.url.searchParams as before.
---
Nitpick comments:
In `@docs/1.docs/50.websocket.md`:
- Around line 113-147: The lifecycle hook descriptions for open, message, close,
and error are repetitive — change one or two lead sentences to vary wording
while keeping meaning; for example, rephrase the `open(peer)` intro to "Invoked
when a connection is established" and the `message(peer, message)` intro to
"Triggered on incoming messages", leaving `close(peer, details)` and `error`
as-is (or similarly vary them) so the hooks `open`, `message`, `close`, and
`error` are clearer and the section reads less repetitive.
In `@src/presets/vercel/runtime/queue-handler.ts`:
- Around line 10-11: Replace the direct console.error call with the
repo-approved logger: get the Nitro logger (via useNitroApp()/nitro?.logger) or
fall back to consola and call logger.error with the same context; specifically,
where console.error("[vercel:queue]", error) currently appears in
queue-handler.ts, change it to use the Nitro/logger error method and preserve
passing the error and contextual tags (the surrounding code uses
useNitroApp().captureError?.(error as Error, { event, tags: ["vercel:queue"] })
so ensure the logger message includes the "[vercel:queue]" tag and the error
object and only use consola if nitro.logger is not available).
🪄 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: 7574b6ff-9ab4-454c-9874-b8adfac24e2b
⛔ Files ignored due to path filters (2)
docs/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (46)
.github/workflows/ci.yml.github/workflows/npm-publish.ymldocs/.config/docs.yamldocs/1.docs/2.quick-start.mddocs/1.docs/4.renderer.mddocs/1.docs/5.routing.mddocs/1.docs/50.assets.mddocs/1.docs/50.configuration.mddocs/1.docs/50.database.mddocs/1.docs/50.lifecycle.mddocs/1.docs/50.openapi.mddocs/1.docs/50.plugins.mddocs/1.docs/50.tasks.mddocs/1.docs/50.websocket.mddocs/1.docs/6.server-entry.mddocs/1.docs/7.cache.mddocs/1.docs/8.storage.mddocs/1.docs/99.migration.mddocs/2.deploy/0.index.mddocs/2.deploy/20.providers/aws-amplify.mddocs/2.deploy/20.providers/aws.mddocs/2.deploy/20.providers/cloudflare.mddocs/2.deploy/20.providers/iis.mddocs/2.deploy/20.providers/vercel.mddocs/2.deploy/20.providers/zephyr.mddocs/3.config/0.index.mddocs/4.examples/plugins.mddocs/package.jsonexamples/plugins/README.mdexamples/plugins/server.tspackage.jsonscripts/update-deps.tssrc/presets/deno/runtime/deno-deploy.tssrc/presets/vercel/preset.tssrc/presets/vercel/runtime/queue-handler.tssrc/presets/vercel/types.tssrc/presets/vercel/utils.tssrc/runtime/internal/routes/dev-tasks.tssrc/runtime/internal/routes/swagger.tssrc/types/config.tssrc/types/nitro.tssrc/types/openapi.tssrc/types/route-rules.tssrc/types/runtime/nitro.tstest/presets/nitro-dev.test.tstest/presets/vercel.test.ts
✅ Files skipped from review due to trivial changes (25)
- docs/2.deploy/20.providers/zephyr.md
- docs/2.deploy/0.index.md
- .github/workflows/npm-publish.yml
- docs/1.docs/50.plugins.md
- docs/2.deploy/20.providers/iis.md
- docs/1.docs/6.server-entry.md
- docs/2.deploy/20.providers/aws-amplify.md
- docs/1.docs/7.cache.md
- docs/1.docs/50.assets.md
- examples/plugins/server.ts
- docs/1.docs/8.storage.md
- docs/1.docs/4.renderer.md
- docs/1.docs/50.configuration.md
- docs/2.deploy/20.providers/aws.md
- src/presets/vercel/utils.ts
- docs/package.json
- docs/1.docs/50.database.md
- src/types/runtime/nitro.ts
- docs/4.examples/plugins.md
- docs/2.deploy/20.providers/cloudflare.md
- docs/1.docs/50.openapi.md
- src/types/nitro.ts
- examples/plugins/README.md
- docs/.config/docs.yaml
- src/types/route-rules.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/config.ts
| "zephyr-agent": "^1.0.1" | ||
| }, | ||
| "peerDependencies": { | ||
| "@vercel/queue": "^0.1.4", | ||
| "dotenv": "*", | ||
| "giget": "*", | ||
| "jiti": "^2.6.1", | ||
| "rollup": "^4.60.1", | ||
| "vite": "^7 || ^8", | ||
| "xml2js": "^0.6.2", | ||
| "zephyr-agent": "^0.1.15" | ||
| "zephyr-agent": "^0.2.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the package.json file
find . -name "package.json" -type f | head -5Repository: nitrojs/nitro
Length of output: 232
🏁 Script executed:
# Read the relevant sections of package.json around lines 175-185
head -200 package.json | tail -50Repository: nitrojs/nitro
Length of output: 1161
🏁 Script executed:
# Get a better view of the entire devDependencies and peerDependencies sections
rg '"(devDependencies|peerDependencies)"' -A 30 package.jsonRepository: nitrojs/nitro
Length of output: 1734
Align the zephyr-agent peer range with the version this repo tests against.
devDependencies specifies ^1.0.1, but peerDependencies advertises only ^0.2.0. This excludes the version being tested locally and will produce unnecessary peer-version conflicts for consumers on 1.x. Widen the peer range to ^1.0.0 or pin devDependencies back to the advertised 0.2.0 range.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 175 - 185, Update the zephyr-agent version
alignment in package.json so peerDependencies matches what we test in
devDependencies: either widen the peer range from "zephyr-agent": "^0.2.0" to
include 1.x (e.g., "^1.0.0" or "^1.0.0 || ^0.2.0") inside the peerDependencies
block, or instead pin devDependencies' zephyr-agent back to the advertised
"^0.2.0"; adjust the "zephyr-agent" entry in peerDependencies (and
devDependencies if you choose to change that) so both entries are consistent.
Co-authored-by: Pooya Parsa <pyapar@gmail.com>
❓ Type of change
📚 Description
Adds additional jsdocs and links to docs pages.
Also corrects some old jsdocs @see links to point to new docs routes.
📝 Checklist