stream: deprecate legacy prependListener fallback#62435
stream: deprecate legacy prependListener fallback#62435karan-lrn wants to merge 1 commit intonodejs:mainfrom
Conversation
|
Review requested:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #62435 +/- ##
==========================================
- Coverage 89.71% 89.70% -0.02%
==========================================
Files 695 695
Lines 214154 214224 +70
Branches 41009 41014 +5
==========================================
+ Hits 192132 192167 +35
- Misses 14075 14112 +37
+ Partials 7947 7945 -2
🚀 New features to boost your workflow:
|
|
cc @nodejs/streams @mcollina @ronag This PR removes the legacy Since Node.js v6 has been EOL for over 8 years, this cleanup removes ~15 lines of unnecessary code and the associated tests that explicitly tested the fallback behavior. |
mcollina
left a comment
There was a problem hiding this comment.
It would be better to deprecate this first
73f182e to
dbb2a5f
Compare
|
Hey @mcollina The deprecation warning now fires when piping to an EventEmitter that lacks the Once this lands and goes through a few releases, we can remove the fallback code entirely. |
dbb2a5f to
dea878a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Commit Queue failed- Loading data for nodejs/node/pull/62435 ✔ Done loading data for nodejs/node/pull/62435 ----------------------------------- PR info ------------------------------------ Title stream: deprecate legacy prependListener fallback (#62435) Author Karan Mangtani <karanmangtani5@gmail.com> (@karan-lrn) Branch karan-lrn:stream-remove-prependlistener-hack -> nodejs:main Labels stream, semver-major, needs-ci Commits 1 - stream: deprecate piping to emitters without prependListener Committers 1 - Karan Mangtani <karan.mangtani@lrn.com> PR-URL: https://github.com/nodejs/node/pull/62435 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Mattias Buelens <mattias@buelens.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/62435 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Mattias Buelens <mattias@buelens.com> -------------------------------------------------------------------------------- ℹ This PR was created on Wed, 25 Mar 2026 20:25:37 GMT ✔ Approvals: 3 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/62435#pullrequestreview-4023940838 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/62435#pullrequestreview-4024660541 ✔ - Mattias Buelens (@MattiasBuelens): https://github.com/nodejs/node/pull/62435#pullrequestreview-4025663529 ✘ GitHub CI failed with status: FAILURE ℹ Last Full PR CI on 2026-03-28T22:58:04Z: https://ci.nodejs.org/job/node-test-pull-request/72172/ - Querying data for job/node-test-pull-request/72172/ ✔ Build data downloaded - Querying failures of job/node-test-commit/86441/ ✔ Data downloaded ✘ 3 failure(s) on the last Jenkins CI run -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/23706198878 |
|
Can you rebase? |
6753c8f to
a603461
Compare
|
@mcollina done! |
The prependListener method has been available on EventEmitter since Node.js v6.0.0 (April 2016). The fallback code that manipulated the internal _events object is no longer necessary.
This also removes tests that explicitly tested the fallback behavior.