Skip to content

http: deprecate writeHeader #59060

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

http: deprecate writeHeader #59060

wants to merge 1 commit into from

Conversation

bjohansebas
Copy link
Member

It’s been over 7 years since it was deprecated in documentation only. I think it’s time to deprecate it at the runtime level.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jul 14, 2025
@bjohansebas bjohansebas force-pushed the deprecate-writeHeader branch from f633c2e to 25189c5 Compare July 14, 2025 01:03
@bjohansebas bjohansebas added semver-major PRs that contain breaking changes and should be released in the next major version. deprecations Issues and PRs related to deprecations. labels Jul 14, 2025
@bjohansebas bjohansebas force-pushed the deprecate-writeHeader branch 2 times, most recently from 2deb1bb to 04a2638 Compare July 14, 2025 01:21
@bjohansebas bjohansebas force-pushed the deprecate-writeHeader branch from 04a2638 to d00318d Compare July 14, 2025 01:49
@codecov Codecov
Copy link

codecov bot commented Jul 14, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.06%. Comparing base (049664b) to head (d00318d).
Report is 41 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #59060      +/-   ##
==========================================
- Coverage   90.06%   90.06%   -0.01%     
==========================================
  Files         645      645              
  Lines      189130   189132       +2     
  Branches    37094    37090       -4     
==========================================
+ Hits       170339   170340       +1     
+ Misses      11511    11508       -3     
- Partials     7280     7284       +4
Files with missing lines Coverage Δ
lib/_http_server.js 96.83% <100.00%> (+<0.01%) ⬆️

... and 22 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bjohansebas
Copy link
Member Author

Ping to @nodejs/tsc since this is a breaking change.

@RaisinTen RaisinTen added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jul 14, 2025
mcollina
mcollina approved these changes Jul 14, 2025
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@bjohansebas bjohansebas added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 16, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 16, 2025
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/67960/

@bjohansebas bjohansebas added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 22, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 22, 2025
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/68115/

@bjohansebas
Copy link
Member Author

Hmm, it looks like the failing tests aren’t related to my changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@ShogunPanda ShogunPanda ShogunPanda approved these changes

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@pimterry pimterry pimterry approved these changes

@anonrig anonrig anonrig approved these changes

@Ethan-Arrowood Ethan-Arrowood Ethan-Arrowood approved these changes

@RafaelGSS RafaelGSS RafaelGSS approved these changes

@marco-ippolito marco-ippolito marco-ippolito approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

Assignees
No one assigned
Labels
deprecations Issues and PRs related to deprecations. http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.