stream: fix pipeTo to defer writes per WHATWG spec#61800
stream: fix pipeTo to defer writes per WHATWG spec#61800mcollina wants to merge 1 commit intonodejs:mainfrom
Conversation
The WHATWG Streams spec requires that pipeTo's chunk handling must queue a microtask before calling the write algorithm. This ensures that enqueue() does not synchronously trigger writes. Previously, PipeToReadableStreamReadRequest[kChunk] would synchronously call writableStreamDefaultWriterWrite(), which violated the spec and caused the WPT test "enqueue() must not synchronously call write algorithm" to fail. Fix by wrapping the write operation in queueMicrotask(), which defers it to the next microtask as required by the spec. Refs: whatwg/streams#1243
|
Review requested:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #61800 +/- ##
==========================================
- Coverage 89.75% 89.74% -0.01%
==========================================
Files 674 675 +1
Lines 204416 204679 +263
Branches 39285 39337 +52
==========================================
+ Hits 183472 183697 +225
- Misses 13227 13282 +55
+ Partials 7717 7700 -17
🚀 New features to boost your workflow:
|
jasnell
left a comment
There was a problem hiding this comment.
I've always thought this one was a bit silly but ok.
Is it just me, or does this not even appear in the spec text? Step 15 doesn't even mention "chunk steps". 🤔 I agree that the specification for |
| "piping/general-addition.any.js": { | ||
| "fail": { | ||
| "note": "Blocked on https://github.com/whatwg/streams/issues/1243", | ||
| "expected": [ | ||
| "enqueue() must not synchronously call write algorithm" | ||
| ] | ||
| } | ||
| }, |
There was a problem hiding this comment.
Even the reference implementation skips this test. 😅
Is this extra microtask now a "de facto" standard? Looking at wpt.fyi, all major browsers are already passing this test, even though the spec text doesn't require it...
Summary
PipeToReadableStreamReadRequest[kChunk]to defer writes viaqueueMicrotask()per the WHATWG Streams specenqueue()Details
Previously,
PipeToReadableStreamReadRequest[kChunk]would synchronously callwritableStreamDefaultWriterWrite(), which violated the WHATWG Streams spec (ReadableStreamPipeTo step 15's "chunk steps").This caused the WPT test
piping/general-addition.any.js→"enqueue() must not synchronously call write algorithm"to fail, which was tracked as an expected failure (see whatwg/streams#1243).Wrapping the write in
queueMicrotask()defers it to the next microtask, matching the spec's required timing semantics.Refs: whatwg/streams#1243
Test plan
test/wpt/test-streams.js)test/parallel/test-webstreams-*.jstests pass