stream: use primordials in pipeline.js

[Ayoub-Mabrouk]

Replace native methods with primordials for consistency and security. This improves protection against prototype pollution.

[nodejs-github-bot]

Review requested:

• @nodejs/streams

[avivkeller]

Please amend your commit to abide by the guidelines

[Ayoub-Mabrouk]

Please amend your commit to abide by the guidelines

thanks, done it.

[gurgunday]

Apologies didn't mean to close

But can we benchmark this?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.53%. Comparing base (05d6b9b) to head (ad254f3).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #61210   +/-   ##
=======================================
  Coverage   88.53%   88.53%           
=======================================
  Files         703      703           
  Lines      208599   208616   +17     
  Branches    40229    40242   +13     
=======================================
+ Hits       184685   184702   +17     
- Misses      15939    15942    +3     
+ Partials     7975     7972    -3     

Files with missing lines	Coverage Δ
lib/internal/streams/pipeline.js	95.15% <100.00%> (+0.03%)	⬆️

... and 30 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.

[Ayoub-Mabrouk]

Apologies didn't mean to close

But can we benchmark this?

Primordials in Node.js are VM-captured built-ins used to protect core modules from user mutations and prototype pollution. Their main goal is safety and consistency, not performance. You can benchmark them if you want, but it’s well known that some primordials can regress performance in hot paths so benchmarks should not be used as a measure of correctness or suitability.

Some modules like http, http2, tls, and zlib prioritize performance over safety, but in most of the core, primordials are preferred for new code.

[ljharb]

@Ayoub-Mabrouk many collaborators are not willing to take a performance hit in exchange for the correctness that primordials provide, so benchmarks are definitely going to be needed.

[Ayoub-Mabrouk]

@Ayoub-Mabrouk many collaborators are not willing to take a performance hit in exchange for the correctness that primordials provide, so benchmarks are definitely going to be needed.

I understand the concern about performance, and you’re right, benchmarks can always be run. That said, the purpose of primordials is correctness and safety, not speed. In that sense, it’s a clear trade-off, either we prioritize protection against prototype pollution (primordials) or we prioritize raw performance.

If we believe primordials are “bad” for performance, then perhaps the decision should be explicit in the docs and we could consider removing them in performance-critical paths. Otherwise, we should keep them as they are and document that their use is about safety, not performance. This might be worth adding to the existing MD files for clarity.

[cjihrig]

@Ayoub-Mabrouk this has been debated ad nauseam in nodejs/TSC#1438 and a lot of other issues 😄

By the way, have you seen https://github.com/nodejs/node/blob/main/doc/contributing/primordials.md? If you think there is more to add there, feel free to open a PR.

[Ayoub-Mabrouk]

@Ayoub-Mabrouk this has been debated ad nauseam in nodejs/TSC#1438 and a lot of other issues 😄

By the way, have you seen https://github.com/nodejs/node/blob/main/doc/contributing/primordials.md? If you think there is more to add there, feel free to open a PR.

Thanks for the pointer 🙂 I’m aware this has been debated before, and I didn’t mean to re-open settled discussions. My intent was mainly to restate that primordials are a correctness and safety mechanism, not a performance optimization, since that distinction sometimes gets blurred during reviews.

If there’s room to improve clarity, a future PR to the primordials documentation could explicitly state that whether primordials should be used or avoided in a given area is ultimately a TSC-level decision. That guidance would also help set expectations for how PRs like my recent ones touching primordials are evaluated.

At the end of the day, it’s a conscious trade-off the project makes, favoring maximum performance by removing primordials, or keeping them to preserve safety and resilience against user-land mutation.