Skip to content

stream: address webstreams review blockers - #18

Draft
anonrig wants to merge 2 commits into
cursor/nodejs-main-base-1677from
cursor/address-pr-65273-comments-1677
Draft

stream: address webstreams review blockers#18
anonrig wants to merge 2 commits into
cursor/nodejs-main-base-1677from
cursor/address-pr-65273-comments-1677

Conversation

@anonrig

@anonrig anonrig commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Follow-up for nodejs#65273.

The same commits are on stream-speed-up-webstreams. Please review there.

This is a fixup commit on top of the squashed speed-up, addressing jasnell's 2026-08-18 review:

  • writer.ready / writer.closed (and cancel()) no longer share kResolvedPromise
  • Cold algorithms (cancel/close/abort/flush/transform) use async wrappers again
  • Write-queue drain loop removed so each write stays one microtask
  • Native webstreams binding removed; isNonThenable / cloneAsUint8Array stay in JS
  • Deferred controller field is initialized; cancel of a non-readable empty stream does not allocate one
  • Tests for promise identity, clone edge cases, and detached-buffer TypeError
Open in Web Open in Cursor 

@cursor
cursor Bot force-pushed the cursor/address-pr-65273-comments-1677 branch from d3ff6d1 to c7125f4 Compare August 17, 2026 23:15
@cursor cursor Bot changed the title stream: address remaining webstreams review comments stream: speed up WHATWG web streams Aug 17, 2026
@cursor
cursor Bot changed the base branch from main to cursor/nodejs-main-base-1677 August 17, 2026 23:15
@cursor cursor Bot changed the title stream: speed up WHATWG web streams stream: address webstreams review blockers Aug 18, 2026
Avoid per-chunk async wrappers for sync pull/write/start and
complete pipeTo writes without one microtask per chunk. Add a
native webstreams binding with a Fast API isNonThenable check
on the data plane and a memcpy clone for byte views.

Empty stream construction skips redundant validation and lazily
creates the writable AbortController, materializing it on abort()
so controller.signal still reflects the abort reason. Use the
shared kResolvedPromise on the pull/write hot path instead of
allocating PromiseResolve().

Assisted-by: Grok
Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
Stop sharing kResolvedPromise on writer.ready/closed and cancel().
Restore async wrappers for cancel/close/abort/flush/transform so
thenable results keep the previous microtask count. Remove the
write-queue drain loop so each write stays one microtask apart.

Drop the native webstreams binding. isNonThenable and
cloneAsUint8Array stay in JS so a detached buffer still throws
TypeError. Initialize the deferred controller field and skip
materializing it on cancel of a non-readable empty stream.

Assisted-by: Grok
Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
@cursor
cursor Bot force-pushed the cursor/address-pr-65273-comments-1677 branch from 08914f5 to cea5388 Compare August 18, 2026 01:38
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.

1 participant