Skip to content

Revise backpressure examples in streams documentation#90

Open
baskakov wants to merge 1 commit into
nodejs:mainfrom
baskakov:patch-1
Open

Revise backpressure examples in streams documentation#90
baskakov wants to merge 1 commit into
nodejs:mainfrom
baskakov:patch-1

Conversation

@baskakov

Copy link
Copy Markdown

Discussed here: https://stackoverflow.com/questions/79821132/node-js-back-pressure-documentation-whats-the-relevance-of-their-example Original backpressure example contains following oddities: 1) Incorrect zip example 2) Incorrect assumption that zip console command loads full file into memory. Updated examples to demonstrate file compression using Node.js streams with backpressure handling.

Copilot AI review requested due to automatic review settings June 14, 2026 09:26
@vercel

vercel Bot commented Jun 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nodejs-learn Ready Ready Preview Jun 28, 2026 10:18pm

Request Review

@cursor

cursor Bot commented Jun 14, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Documentation-only edits to backpressuring-in-streams.md; no runtime or API behavior changes.

Overview
Replaces the old shell zip vs Node gzip pipe comparison with a clearer progression on the same ~9 GB file: readFileSync + gzipSync (buffer/heap limits), then manual data handlers that ignore write() return false (unbounded buffers / OOM), then inp.pipe(gzip).pipe(out) with prose on pause/drain/resume.

Removes the narrative that zip(1) produced a corrupt file while the stream version succeeded. The existing pump/pipeline callout after the pipe snippet is unchanged.

Reviewed by Cursor Bugbot for commit e094691. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

Copy link
Copy Markdown

👋 Codeowner Review Request

The following codeowners have been identified for the changed files:

Team reviewers: @nodejs/streams

Please review the changes when you have a chance. Thank you! 🙏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the backpressure module to better illustrate memory/throughput implications of buffering vs streaming in Node.js compression examples.

Changes:

  • Replaces the zip(1) example with fs.readFileSync + zlib.gzipSync examples (CJS + ESM).
  • Adds an explicit “streams without backpressure” example plus an explanation of why it can OOM.
  • Removes the prior note about comparing resulting archives.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pages/modules/backpressuring-in-streams.md Outdated
Comment thread pages/modules/backpressuring-in-streams.md Outdated
Comment thread pages/modules/backpressuring-in-streams.md Outdated
inp.on('data', (chunk) => gzip.write(chunk));
inp.on('end', () => gzip.end());
gzip.on('data', (chunk) => out.write(chunk));
gzip.on('end', () => out.end());
Comment thread pages/modules/backpressuring-in-streams.md Outdated
Comment thread pages/modules/backpressuring-in-streams.md Outdated
Comment thread pages/modules/backpressuring-in-streams.md Outdated
@avivkeller

Copy link
Copy Markdown
Member

Bump @baskakov

Discussed here: https://stackoverflow.com/questions/79821132/node-js-back-pressure-documentation-whats-the-relevance-of-their-example
Original backpressure example contains following oddities:
1) Incorrect zip example 2) Incorrect assumption that zip console command loads full file into memory.
Updated examples to demonstrate file compression using Node.js streams with backpressure handling.

Signed-off-by: Dmitry Baskakov <dmitry@bask.ws>
@baskakov

Copy link
Copy Markdown
Author

@avivkeller Thanks! Done. Rebased changes in a single commit

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants