fix: writeEarlyHints resolves immediately instead of hanging (#1383)#1415
fix: writeEarlyHints resolves immediately instead of hanging (#1383)#1415rafaumeu wants to merge 1 commit into
Conversation
Fixes h3js#1383 The callback for Node.js writeEarlyHints is optional and may not be invoked in all cases. By resolving the promise immediately after calling writeEarlyHints, we avoid hanging requests while still sending early hints to the client.
📝 WalkthroughWalkthroughThe Node.js fast path in ChangeswriteEarlyHints Node.js fix and manual test
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
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: 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 `@test-write-early-hints.mjs`:
- Around line 1-6: The import path in the H3 module import statement is
incorrect and the test script is not using the patched writeEarlyHints helper
function. Fix the import statement to use the correct path "./src/index.ts"
instead of "../src/index.ts", and replace the direct call to
event.node?.writeEarlyHints? with a call to the patched writeEarlyHints(event,
hints) helper function to properly validate the PR's fix.
🪄 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: c4b1f9aa-8c35-4745-8d55-8a60953a1c42
📒 Files selected for processing (2)
src/utils/response.tstest-write-early-hints.mjs
| import { H3 } from "../src/index.ts"; | ||
|
|
||
| const app = new H3().get("/", async (event) => { | ||
| const start = Date.now(); | ||
| await event.node?.writeEarlyHints?.({ | ||
| link: "</style.css>; rel=preload; as=style", |
There was a problem hiding this comment.
Manual reproduction script misses the fixed API path (and likely has a broken import).
Line 1 should import from the repo root path (./src/index.ts), and Line 5 should call the patched writeEarlyHints(event, hints) helper to actually validate this PR’s fix.
Proposed patch
-import { H3 } from "../src/index.ts";
+import { H3, writeEarlyHints } from "./src/index.ts";
@@
- await event.node?.writeEarlyHints?.({
- link: "</style.css>; rel=preload; as=style",
- });
+ await writeEarlyHints(event, {
+ Link: "</style.css>; rel=preload; as=style",
+ });🤖 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 `@test-write-early-hints.mjs` around lines 1 - 6, The import path in the H3
module import statement is incorrect and the test script is not using the
patched writeEarlyHints helper function. Fix the import statement to use the
correct path "./src/index.ts" instead of "../src/index.ts", and replace the
direct call to event.node?.writeEarlyHints? with a call to the patched
writeEarlyHints(event, hints) helper function to properly validate the PR's fix.
1 participant
Fixes #1383
The callback for Node.js writeEarlyHints is optional and may not
be invoked in all cases. By resolving the promise immediately after
calling writeEarlyHints, we avoid hanging requests while still
sending early hints to the client.
Changes:
Testing:
Summary by CodeRabbit
Tests
Refactor