Skip to content

Update to stream responses without dependency on react/promise-stream - #107

Merged
clue merged 1 commit into
clue:1.xfrom
clue-labs:remove-promise-stream
Oct 18, 2026
Merged

clue merged 1 commit into
clue:1.xfrom
clue-labs:remove-promise-stream

Conversation

@clue

@clue clue commented Oct 18, 2026

Copy link
Copy Markdown
Owner

This changeset updates the client to stream responses without the dependency on react/promise-stream. It now uses a small internal ReadableUnwrapStream in place of unwrapReadable() and buffers streams with a few lines of its own, so consumers of this package no longer install the extra package. The dependency was added with #27 back in 2016, and reactphp/http#482 removed it from the HTTP layer the same way.

All APIs and streaming behavior stay the same, so this should be safe to apply. Stream events now only pass the arguments documented for ReadableStreamInterface, without the extra stream argument the old package appended. The tests are ported from react/promise-stream v1.7.0 and the test suite confirms this keeps 100% code coverage. This should also make it easier to fix #96 in a follow-up PR.

Builds on top of #27 and reactphp/http#482
Refs #106 and #96

@clue clue added the new feature New feature or request label Oct 18, 2026
@clue clue added this to the v1.7.0 milestone Oct 18, 2026
@clue
clue requested a balanced review from Copilot October 18, 2026 21:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Pending resume requests can leave inputs stalled, and buffering cancellation retains accumulated output.

2 open issues
What changed in this PR

Replaces react/promise-stream with internal helpers for Docker response streaming and buffering.

Changes:

  • Adds an internal promise-backed readable stream.
  • Implements buffering and cancellation without the removed dependency.
  • Ports stream tests and updates functional tests.
File Description
tests/​Io/​StreamingParserTest.php Tests buffering, errors, and cancellation.
tests/​Io/​ReadableUnwrapStreamTest.php Tests the new stream wrapper.
tests/​FunctionalClientTest.php Uses internal buffering in integration tests.
src/​Io/​StreamingParser.php Replaces external streaming helpers.
src/​Io/​ReadableUnwrapStream.php Adds the internal readable-stream wrapper.
src/​Client.php Uses the wrapper for logs and attachment.
composer.json Removes react/promise-stream.

Review effort: Balanced

Comment thread src/Io/ReadableUnwrapStream.php
Comment thread src/Io/StreamingParser.php Outdated
@clue
clue force-pushed the remove-promise-stream branch from fc81479 to 6fb02d2 Compare October 18, 2026 21:40
@clue
clue merged commit 3857d75 into clue:1.x Oct 18, 2026
15 checks passed
@clue
clue deleted the remove-promise-stream branch October 18, 2026 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

containerAttachStream's stderrEvent is emitted on wrong stream

2 participants