build: re-export FastResponse and srvx types - #4499
Conversation
|
@schplitt is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughNitro updates its Changessrvx type re-export
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
pi0
changed the title
| "./context": "./dist/runtime/context.mjs", | ||
| "./database": "./dist/runtime/database.mjs", | ||
| "./h3": "./lib/h3.mjs", | ||
| "./srvx": "./lib/srvx.mjs", |
There was a problem hiding this comment.
Exporting all can be risky against breaking change surface (srvx 0.12 to 0.13 could drop some for example), instead we can simply export FastResponse from main nitro subpath
There was a problem hiding this comment.
Alright. I know I needed FastURL and ServerRequest aswell as the types for those + ServerRequestContext, ServerRuntimeContext in the past
These can go under the nitro subpath aswell?
There was a problem hiding this comment.
we have nitro/types subpath for types. Not sure about ServerRequestContext i guess it needs explicit declaration for srvx to extend
pi0
changed the title
srvx.FastResponse
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/nitro.ts`:
- Around line 26-27: Update the srvx type export declaration in the runtime
entry point to also export ServerRequestContext and ServerRuntimeContext,
matching the complete type surface exposed by src/types/srvx.ts and the
ServerRequestContext usage in serverFetch.
🪄 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 Plus
Run ID: ae0ccc18-623a-42d7-b688-12cac4201b82
📒 Files selected for processing (2)
src/runtime/nitro.tssrc/types/srvx.ts
| // srvx | ||
| export type { FastResponse, FastURL, ServerRequest } from "srvx"; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep the srvx type exports consistent across Nitro entry points.
src/types/srvx.ts exposes ServerRequestContext and ServerRuntimeContext, but this runtime entry point omits both. Since serverFetch already accepts ServerRequestContext, consumers cannot import the complete advertised srvx type surface from the runtime module.
Proposed fix
-export type { FastResponse, FastURL, ServerRequest } from "srvx";
+export type {
+ FastResponse,
+ FastURL,
+ ServerRequest,
+ ServerRequestContext,
+ ServerRuntimeContext,
+} from "srvx";📝 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.
| // srvx | |
| export type { FastResponse, FastURL, ServerRequest } from "srvx"; | |
| // srvx | |
| export type { | |
| FastResponse, | |
| FastURL, | |
| ServerRequest, | |
| ServerRequestContext, | |
| ServerRuntimeContext, | |
| } from "srvx"; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/nitro.ts` around lines 26 - 27, Update the srvx type export
declaration in the runtime entry point to also export ServerRequestContext and
ServerRuntimeContext, matching the complete type surface exposed by
src/types/srvx.ts and the ServerRequestContext usage in serverFetch.
srvx.FastResponseFastResponse and srvx types
| FastResponse, | ||
| FastURL, |
There was a problem hiding this comment.
i don't think we need to export these types. FastURL type is URL same for FastResponse which is also already exported from main
| FastResponse, | |
| FastURL, |
There was a problem hiding this comment.
Yea right! Thanks for being patient
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/nitro.ts`:
- Line 27: Update the srvx type re-exports in the Nitro entry point to include
ServerRequestContext and ServerRuntimeContext alongside FastResponse and
FastURL. Keep ServerRequest removed, and ensure consumers importing types from
this module receive the complete supported srvx contract.
🪄 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 Plus
Run ID: 21274f75-5534-4238-934a-ee7e213f7ce2
📒 Files selected for processing (2)
src/runtime/nitro.tssrc/types/srvx.ts
💤 Files with no reviewable changes (1)
- src/types/srvx.ts
| export type { H3Event, EventHandlerRequest, EventHandlerWithFetch } from "h3"; | ||
|
|
||
| // srvx | ||
| export type { FastResponse, FastURL } from "srvx"; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep srvx context exports consistent across Nitro entry points.
Removing ServerRequest matches the PR objective, but this entry point still omits ServerRequestContext and ServerRuntimeContext, which are exposed by src/types/srvx.ts and used by Nitro’s request APIs. Consumers importing Nitro types from this module cannot access the complete srvx contract.
At minimum, re-export the context types here, or confirm that they are intentionally declaration-only and adjust the public typing contract accordingly.
#!/bin/bash
rg -n -C 3 '\b(ServerRequestContext|ServerRuntimeContext|serverFetch)\b' src🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/nitro.ts` at line 27, Update the srvx type re-exports in the
Nitro entry point to include ServerRequestContext and ServerRuntimeContext
alongside FastResponse and FastURL. Keep ServerRequest removed, and ensure
consumers importing types from this module receive the complete supported srvx
contract.
2 participants
closes #4498
I deemed the rest of the exports of srvx not necessary for usage in nitro so I only included the root package