feat(network-config): add Node response streaming config#300
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: f0fbcc5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR introduces explicit response streaming configuration for Node functions and Vercel Build Output functions. New ChangesResponse Streaming Configuration & Detection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.changeset/streaming-node-config.md (1)
5-5: 💤 Low valueConsider expanding the changeset description to include default behavior.
The current description is concise but could be more informative for users reading the changelog. Consider mentioning that Node functions stream by default and can opt out.
📝 Suggested enhancement
-Add explicit response streaming configuration for Node functions and Vercel Build Output functions. +Add explicit response streaming configuration for Node functions and Vercel Build Output functions. Node functions now stream by default with optional opt-out via the `streaming` property.🤖 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 @.changeset/streaming-node-config.md at line 5, Update the changeset description to state the default behavior: mention that Node functions (and Vercel Build Output functions) stream responses by default and that projects can opt out (describe the opt-out configuration or flag name if applicable). Edit the sentence "Add explicit response streaming configuration for Node functions and Vercel Build Output functions." to include the default (streaming on) and a short note on how to disable it, referencing "Node functions" and "Vercel Build Output functions" so readers immediately know the default and opt-out path.packages/network-config/src/services/v4-config-parser.ts (1)
161-166: ⚡ Quick winDerive
streamingfrom a single resolved runtime value.
runtimeandstreamingdefaults are computed separately; consolidating them avoids future drift if runtime defaults change.♻️ Proposed refactor
- entrypoints.push({ + const runtime = settings.runtime ?? 'node-20'; + const streaming = settings.streaming ?? runtimeStreamsByDefault(runtime); + + entrypoints.push({ path: file, - runtime: settings.runtime ?? 'node-20', + runtime, memory: settings.memory ?? 128, maxDuration: settings.max_duration ?? 30, schedule: func.schedule, symlinks: func.symlinks, - streaming: settings.streaming ?? runtimeStreamsByDefault(settings.runtime), + streaming, package: settings.includeFiles != null || settings.excludeFiles != null ? {🤖 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 `@packages/network-config/src/services/v4-config-parser.ts` around lines 161 - 166, Compute a single resolved runtime value and use it for both the runtime field and to derive streaming so the defaults stay consistent: create a local constant (e.g., resolvedRuntime = settings.runtime ?? 'node-20') and replace occurrences that use settings.runtime when calling runtimeStreamsByDefault with that constant; keep settings.streaming override semantics (streaming: settings.streaming ?? runtimeStreamsByDefault(resolvedRuntime)) and set runtime: resolvedRuntime (or runtime: settings.runtime ?? 'node-20') so both fields rely on the same resolved value.
🤖 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 `@packages/network-config/tsconfig.json`:
- Line 5: The tsconfig.json contains a rootDir set to "../.." which causes
TypeScript to emit a nested repository structure; to fix it, open the package's
tsconfig.json and either remove the "rootDir" property entirely or change it to
"rootDir": "." so the compiler uses the package root (matching other packages)
and outputs files to the expected dist/... layout.
In `@tsconfig.base.json`:
- Line 7: Remove the obsolete "ignoreDeprecations": "5.0" setting from
tsconfig.base.json; locate the JSON property named ignoreDeprecations and delete
that key/value pair so the file no longer contains the deprecated compiler
option.
---
Nitpick comments:
In @.changeset/streaming-node-config.md:
- Line 5: Update the changeset description to state the default behavior:
mention that Node functions (and Vercel Build Output functions) stream responses
by default and that projects can opt out (describe the opt-out configuration or
flag name if applicable). Edit the sentence "Add explicit response streaming
configuration for Node functions and Vercel Build Output functions." to include
the default (streaming on) and a short note on how to disable it, referencing
"Node functions" and "Vercel Build Output functions" so readers immediately know
the default and opt-out path.
In `@packages/network-config/src/services/v4-config-parser.ts`:
- Around line 161-166: Compute a single resolved runtime value and use it for
both the runtime field and to derive streaming so the defaults stay consistent:
create a local constant (e.g., resolvedRuntime = settings.runtime ?? 'node-20')
and replace occurrences that use settings.runtime when calling
runtimeStreamsByDefault with that constant; keep settings.streaming override
semantics (streaming: settings.streaming ??
runtimeStreamsByDefault(resolvedRuntime)) and set runtime: resolvedRuntime (or
runtime: settings.runtime ?? 'node-20') so both fields rely on the same resolved
value.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a00e0a1b-5de1-49e4-896b-c270bdf7363c
📒 Files selected for processing (13)
.changeset/streaming-node-config.mdpackages/network-config/src/detection/frameworks/nestjs.test.tspackages/network-config/src/services/schema-validator.test.tspackages/network-config/src/services/v4-config-parser.test.tspackages/network-config/src/services/v4-config-parser.tspackages/network-config/src/services/vercel-build-output-parser.test.tspackages/network-config/src/services/vercel-build-output-parser.tspackages/network-config/src/v4/index.tspackages/network-config/src/v4/parse.test.tspackages/network-config/src/v4/schema.jsonpackages/network-config/src/v4/schema.tspackages/network-config/tsconfig.jsontsconfig.base.json
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4ac36f6f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
💡 Codex Review Allow capture substitutions to span nested paths
The new matcher treats every sdk/packages/network-config/src/services/v4-config-parser.ts Lines 40 to 41 in 5902df2 Preserve Bun functions' default streaming
When a v4 function uses sdk/packages/network-config/src/services/v4-config-parser.ts Lines 109 to 110 in 5902df2 Avoid using one matching entrypoint for dynamic route type
For substituted destinations that can resolve to more than one entrypoint, Match named capture substitutions in destinations
The v4 schema documents that named capture groups from route requirements can be used in sdk/packages/network-config/src/services/v4-config-parser.ts Lines 174 to 175 in 5902df2 Apply later streaming overrides to matched files
This computes the new ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Testing
Summary by CodeRabbit
New Features
Improvements