Skip to content

perf(pm): drop reqwest stream feature, stream download via Response::chunk - #3188

Merged
killagu merged 1 commit into
nextfrom
fix/pm-download-progress-no-stream
Jun 21, 2026
Merged

killagu merged 1 commit into
nextfrom
fix/pm-download-progress-no-stream

Conversation

@elrrrrrrr

Copy link
Copy Markdown
Contributor

Background

crates/pm/src/util/downloader.rs reported live install progress via response.bytes_stream() (added in #3142 / a9ee35b), which requires reqwest's stream feature. #3183 (binary slimming) had removed stream on a base that predated that call site, so once both PRs landed on next the combination failed to compile:

error[E0599]: no method named `bytes_stream` found for struct `reqwest::Response`

#3187 unblocked next by re-adding the stream feature. This PR does it properly instead.

Why progress never needed stream

reqwest::Response::chunk() reads the body frame-by-frame with no feature gate (only bytes_stream() is #[cfg(feature = "stream")]). It feeds the exact same per-chunk DOWNLOADED_BYTES counter the spinner renders as ↓ 23.4 MB 8.2 MB/s.

The body is still fully buffered before return — the downstream extractor is not streaming (the decompression path isn't incremental) — so chunk() only buys the progress signal, not lower peak memory. Behavior is byte-for-byte identical: npm .tgz aren't transfer-gzip-encoded, so both paths count raw tarball bytes the same.

Changes

Verification

cargo check  -p utoo-pm                                   # ok
cargo clippy -p utoo-pm --all-targets -- -D warnings --no-deps   # ok
cargo fmt                                                 # no changes

🤖 Generated with Claude Code

…e::chunk`

#3142 added live install progress using `response.bytes_stream()`, which
requires reqwest's `stream` feature. #3183 (binary slimming) had removed
that feature on a base that predated the call site, so once both landed on
`next` the combination failed to compile (`E0599: no method bytes_stream`).
#3187 fixed the breakage by re-adding the `stream` feature.

But download progress never needed `stream`. `Response::chunk()` reads the
body frame-by-frame with no feature gate, so it feeds the same per-chunk
`DOWNLOADED_BYTES` counter the spinner renders. The body is still fully
buffered before return (the extractor is not streaming), so this only buys
the progress signal, not lower peak memory — identical behavior, one fewer
feature.

Revert #3187's `stream` feature (drops the wasm-only `wasm-streams` dep
from the lock) and rewrite the download loop to use `chunk()`, dropping the
`futures::StreamExt` import.

Verified: cargo check -p utoo-pm, cargo clippy --all-targets -D warnings,
cargo fmt all clean; Cargo.lock diff is the exact inverse of #3187.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request removes the stream feature from the reqwest dependency in crates/pm/Cargo.toml and removes the unused wasm-streams dependency from Cargo.lock. In crates/pm/src/util/downloader.rs, the code is refactored to use response.chunk().await instead of response.bytes_stream(), which avoids buffering the entire body at once while eliminating the need for the reqwest stream feature and the futures::StreamExt import. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@killagu killagu 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.

LGTM

@killagu
killagu merged commit 9c8489d into next Jun 21, 2026
33 checks passed
@killagu
killagu deleted the fix/pm-download-progress-no-stream branch June 21, 2026 15:41
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.

2 participants