Repository navigation
Pin late writes on the elided TransformStream path - #7711
Merged
Merged
Conversation
Contributor
|
Since last review: 0 resolved, 0 still open, 0 new. Reviewed commit: 81650bdb · github run |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7711 +/- ##
=======================================
Coverage 40.90% 40.90%
=======================================
Files 985 985
Lines 284215 284215
Branches 27545 27545
=======================================
Hits 116252 116252
Misses 151891 151891
Partials 16072 16072 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
guybedford
reviewed
Oct 13, 2026
jasnell
force-pushed
the
jasnell/ts-streams-duplicate-consolidaton-3
branch
from
October 13, 2026 16:58
1851b26 to
8664e6d
Compare
Base automatically changed from
jasnell/ts-streams-duplicate-consolidaton-2
to
main
October 14, 2026 00:36
A transformer with no hooks takes the TypeScript implementation's elided path, whose sink enqueues straight into the readable controller. Pin how its writes settle in shapes the standard path guards against, ahead of fixing the divergence: - writeReachesSinkAfterCancelClearedAlgorithms (ledger #19) adds a transformer with no hooks: a write released by a read reaches the sink after a same-turn readable.cancel(). - writeAfterCancelSameTurn (ledger #22) writes right after readable.cancel(), with room in the readable. C++ and the TypeScript hook paths reject the write with the cancel reason. - readableCancelInsideThrowingSize (ledger #23) has the readable side's size() cancel the readable and then throw. C++ fulfills the write; the TypeScript hook paths reject it with undefined, as the spec does. The elided path rejects the first two writes at once with the readable controller's TypeError, and the third with size()'s error.
jasnell
force-pushed
the
jasnell/ts-streams-duplicate-consolidaton-3
branch
from
October 14, 2026 00:36
8664e6d to
e887395
Compare
|
The generated output of |
A write that reached the elided path's sink after readable.cancel() had started found the readable closed, and rejected at once with the readable controller's TypeError. The standard path rejects it with the writable's stored error, the cancel reason, once the cancel settles. C++ does the same for a transformer with no hooks when the write follows the cancel in the same turn. One such case is a pipe whose destination errors: pipeTo() cancels the transform's readable while a producer may still be writing. The elided sink now checks the finish promise before it enqueues, as the standard path checks for cleared algorithms. If a finish operation has started, which at the sink can only be the cancel, it waits for the finish promise and rejects with the writable's stored error. Otherwise it enqueues through transformStreamEnqueue, as the default transform algorithm does, in place of its own copy of the enqueue and the backpressure mirror. A write that enqueues still settles after two microtasks. Enqueueing as the default transform algorithm does also settles a write whose readable-side size() cancels the readable and then throws as the hook paths and the spec do (ledger #23): the write rejects with undefined, the closed readable's stored error, and the writable is errored with size()'s error. The elided path rejected the write with size()'s error, and the cancel errored the writable with its reason; C++ fulfills the write. A size() that throws without re-entering now errors the writable as the enqueue throws, as on the standard path, not once the write has rejected. Flip the pins: ledger #19's case with no hooks, #22 (now parity) and #23 (now a C++ divergence only). typescript_implemented_streams is experimental, so this needs no compatibility flag.
The elided and standard constructor paths repeated, token for token, the writable's construction, the pull source, the readable's construction and the expectedLength hookup. Each branch now sets up only what differs, the controller, the algorithms and its sinkWrite, and the pair is built once after the branch. The start step resolves the start promise directly when there is no controller. No behavior change: the sinkWrite bodies are unchanged, every closure is still created in the constructor's scope, and the strategies are read in the same order.
jasnell
force-pushed
the
jasnell/ts-streams-duplicate-consolidaton-3
branch
from
October 14, 2026 01:44
e887395 to
81650bd
Compare
guybedford
approved these changes
Oct 14, 2026
guybedford
left a comment
guybedford
left a comment
Contributor
There was a problem hiding this comment.
Re-reviewed at 81650bd. The #23 change landed as suggested, and commit 3 is a clean hoist: closure scope, strategy read order and start sequencing are unchanged. transform-ts@, transform-cpp@ and //src/wpt:streams-ts@ pass locally.
Correction to my earlier note: going through transformStreamEnqueue also adds its canCloseOrEnqueue pre-check to every elided write, not just the field read. The standard path pays the same; I don't expect it to show up next to the async sink.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Settle late elided TransformStream writes consistently with the standard path.