From c31c65d83018d3aaf5daf90ce8d3a42df42e92a0 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 13 Oct 2026 01:04:38 +0000 Subject: [PATCH 1/5] Pin how stream constructors convert their arguments The ReadableStream, WritableStream and TransformStream constructors convert their arguments differently in each implementation, and little of it was pinned. Pin it on both, ahead of moving the TypeScript constructors to WebIDL argument conversion: - a first argument that is null, another primitive or a function; - a strategy argument that is null, another primitive or a function; - the order in which the arguments and the strategy members are read, including when highWaterMark is converted to a number; - which error wins when two are possible, including the cases where TypeScript already follows the spec and must keep doing so. This finds a C++ divergence: a TransformStream readable strategy without a highWaterMark (null or {}) gets the ReadableStream default of 1, because C++ applies the TransformStream default of 0 only when the argument is omitted. TypeScript applies 0, as the spec does. --- src/tests/streams/readable-byte/AGENTS.md | 2 +- .../streams/readable-byte/construction.js | 52 ++--- src/tests/streams/readable/AGENTS.md | 8 +- src/tests/streams/readable/construction.js | 127 +++++++++-- src/tests/streams/readable/main.js | 2 + src/tests/streams/transform/AGENTS.md | 10 +- src/tests/streams/transform/construction.js | 198 ++++++++++++++++++ src/tests/streams/transform/main.js | 4 + src/tests/streams/writable/AGENTS.md | 7 +- src/tests/streams/writable/construction.js | 119 ++++++++++- src/tests/streams/writable/main.js | 3 + 11 files changed, 480 insertions(+), 52 deletions(-) diff --git a/src/tests/streams/readable-byte/AGENTS.md b/src/tests/streams/readable-byte/AGENTS.md index 52b06c34a57..47911ad6f12 100644 --- a/src/tests/streams/readable-byte/AGENTS.md +++ b/src/tests/streams/readable-byte/AGENTS.md @@ -15,7 +15,7 @@ behavior-parity (messages aside). | # | Area | C++ | TypeScript | Pinned in | | --- | --- | --- | --- | --- | -| 1 | size() in a byte stream's strategy | silently accepted and ignored | RangeError (spec) — root of the WPT ctor seed | `sizeStrategyForBytes` | +| 1 | size() in a byte stream's strategy | silently accepted and ignored; with an invalid highWaterMark too, that highWaterMark's TypeError (integer conversion) | RangeError (spec), before the highWaterMark is checked — root of the WPT ctor seed | `sizeStrategyForBytes` | | 2 | autoAllocateChunkSize 0/-1/NaN | TypeError 'cannot be zero.' (all three) | TypeError 'must be a positive integer' | `autoAllocateChunkSizeValidated` | | 3 | pull counts (hwm 1, enqueue-in-pull) | 1,1,3 (readable ledger #4 mirror) | 1,2,3 (spec) | `pullCountShape` | | 4 | sync start() throw | captured; stream errored (readable #6 mirror) | escapes constructor (spec) | `syncStartThrow`, `jsSourceError` | diff --git a/src/tests/streams/readable-byte/construction.js b/src/tests/streams/readable-byte/construction.js index 31a29ccb75a..7fe7f7c16da 100644 --- a/src/tests/streams/readable-byte/construction.js +++ b/src/tests/streams/readable-byte/construction.js @@ -9,36 +9,36 @@ import { usingTsImpl } from 'which-impl'; // DIVERGENCE (the WPT general.any seed): a size() in the strategy of a // byte stream is rejected by TypeScript (spec: RangeError) but silently -// accepted — and ignored — by C++. +// accepted — and ignored — by C++. TypeScript rejects it before checking +// highWaterMark, as the spec does; C++ converts an invalid highWaterMark +// to an integer first and throws TypeError. export const sizeStrategyForBytes = { test() { - if (usingTsImpl) { - throws( - () => - new ReadableStream( - { type: 'bytes' }, - { - size() { - return 1; - }, - highWaterMark: 4, - } - ), - { + for (const highWaterMark of [4, -1]) { + const construct = () => + new ReadableStream( + { type: 'bytes' }, + { + size() { + return 1; + }, + highWaterMark, + } + ); + if (usingTsImpl) { + throws(construct, { name: 'RangeError', message: 'The strategy for a byte stream cannot have a size function', - } - ); - } else { - new ReadableStream( - { type: 'bytes' }, - { - size() { - return 1; - }, - highWaterMark: 4, - } - ); + }); + } else if (highWaterMark < 0) { + throws(construct, { + name: 'TypeError', + message: + 'The value cannot be converted because it is negative and this API expects a positive bigint.', + }); + } else { + construct(); + } } }, }; diff --git a/src/tests/streams/readable/AGENTS.md b/src/tests/streams/readable/AGENTS.md index bf23bb9eb5a..81ecdb73486 100644 --- a/src/tests/streams/readable/AGENTS.md +++ b/src/tests/streams/readable/AGENTS.md @@ -15,8 +15,8 @@ behavioral gaps; the reentrancy family is mostly parity at finite hwm. | # | Area | C++ | TypeScript | Pinned in | | --- | --- | --- | --- | --- | -| 1 | garbage underlyingSource | accepts null, rejects numbers | rejects null (spec), accepts numbers | `garbageSourceValidation` | -| 2 | ctor argument conversion order | source.type, hwm, size | strategy size, hwm, then source.type (spec) | `argumentConversionOrder` | +| 1 | garbage underlyingSource | accepts null; every other primitive throws TypeError | rejects null (spec: TypeError); accepts every other primitive, reading its members through the wrapper prototype. A function is accepted on both sides, its members read (spec) | `garbageSourceValidation` | +| 2 | ctor argument conversion order | source.type, hwm and its ToNumber, size | strategy size, hwm, then source.type, then hwm's ToNumber (spec: the strategy first, but hwm and its ToNumber before size) | `argumentConversionOrder` | | 3 | invalid hwm (-1/NaN) + Infinity | TypeError; Infinity REJECTED (integer conversion) — root cause of many WPT reentrant/errors failures | RangeError; Infinity accepted (spec) | `highWaterMarkValidated` | | 4 | pull counts (hwm 1, enqueue-in-pull) | 1,1,3 — first read served from queue without pull, deferred pulls batch | 1,2,3 (spec: pull after start and every read) | `pullCountShape` | | 5 | pull on reading the LAST queued chunk | no pull | pulls (spec) | `pullOnLastChunkRead` | @@ -45,6 +45,8 @@ behavioral gaps; the reentrancy family is mostly parity at finite hwm. | 28 | from() iterator protocol edges | reads `value` of an async iterator's final result; a sync iterator whose value rejects (not done) is left open; the `value` of a sync return() result is not resolved; a non-callable `next` ends the stream; DEFECT: a throwing `return` getter's error escapes into the I/O context as an uncaught exception and fails the request, whether or not the cancel's promise is handled (unpinnable) | spec (WebIDL async_sequence + ECMA-262 async-from-sync): only `done` of the final result; the sync iterator is closed through return(); the return() result's value is resolved (a rejection rejects the cancel); the read rejects with TypeError; the cancel rejects | `fromIteratorProtocolEdges`, `fromCancelReturnLookup` | | 29 | when a sync iterable's from() reads and cancels settle, relative to an async iterator whose next()/return() returns a settled promise | one tick earlier; a throwing next()/return() in the same tick | two ticks earlier, and a throwing one in the same tick: the async-from-sync continuation is reacted to directly, not through "a promise resolved with" it (the spec lands both on the same tick, and a throwing sync one two ticks later); intentional, sparing two microtasks and a Promise.prototype.then lookup per chunk | `fromSyncIterableSettlesEarly` | | 30 | close() inside size() while chunks are still to be read (no tee: a chunk queued; tee: every branch behind the close) | the in-flight chunk is dropped (the transform suite's ledger #10 shows the same through writer.close()) | the close is only requested and the chunk is still readable: every branch reads it (spec without a tee; Node agrees there). With a tee the chunk reaches every branch or none: one already drained to the close drops it for all (parity, `closeInsideSizeTeeDrainedBranch`) | `closeInsideSizeWithQueuedChunk`, `closeInsideSizeTeeBranchesBehind` | +| 31 | strategy argument shapes | WebIDL dictionary: null means no strategy; every other primitive throws TypeError | null throws TypeError (reading its size); every other primitive is accepted as no strategy, its members read through the wrapper prototype. A function's members are read on both sides (spec) | `strategyArgumentShapes` | +| 32 | which error wins when two are possible | a primitive source (TypeError) beats a throwing strategy getter; an invalid highWaterMark (TypeError, integer conversion) beats an invalid type and a non-callable size | the strategy is read first, so a throwing strategy getter beats a primitive source (spec: the source's TypeError); an invalid type and a non-callable size (TypeError) beat an invalid highWaterMark (spec) | `conversionErrorPrecedence` | Parity worth noting (probed, pinned): pull serialization (never re-entered); pull/async-start rejection identity; error-undefined @@ -101,7 +103,7 @@ C++ implementation; `draining-reader.js` asserts both sides. | Module | Coverage | | --- | --- | | `api-surface.js` | globals, controller not constructable, getReader modes, locked lifecycle | -| `construction.js` | ledger #1-#3, default hwm 1 | +| `construction.js` | ledger #1-#3, #31, #32, default hwm 1 | | `source-algorithms.js` | ledger #4-#6, #23, pull serialization, rejection identity, cancel-with-pending-pull | | `controller.js` | desiredSize accounting/terminal states, error idempotence, close terminality (#17), close-drains-queue, error during a pending close (#18) | | `reader.js` | read ordering, releaseLock, closed replacement (#7), undefined error, reader.cancel, reader swap | diff --git a/src/tests/streams/readable/construction.js b/src/tests/streams/readable/construction.js index 090e6ddac4c..29d999e3eb8 100644 --- a/src/tests/streams/readable/construction.js +++ b/src/tests/streams/readable/construction.js @@ -9,22 +9,38 @@ import { strictEqual, throws } from 'node:assert'; import { usingTsImpl } from 'which-impl'; -// DIVERGENCE (mirror-image!): a null underlyingSource is ACCEPTED by C++ -// but rejected by TypeScript (spec: converting null to a dictionary -// throws); a NUMBER underlyingSource is rejected by C++ but accepted by -// TypeScript (primitive wrapper objects convert). +// DIVERGENCE (mirror-image!): C++ converts the underlyingSource as a +// WebIDL dictionary: null means no source and every other primitive +// throws TypeError. TypeScript rejects null (spec: the argument is typed +// `object`, which null is not) but accepts every other primitive, +// reading its members through the wrapper prototype. Both accept a +// function and read its members, as the spec does. export const garbageSourceValidation = { test() { + const primitives = [42, 'x', true, 1n, Symbol('s')]; if (usingTsImpl) { throws(() => new ReadableStream(null), { name: 'TypeError', message: 'Cannot convert undefined or null to object', }); - new ReadableStream(42); + for (const source of primitives) { + new ReadableStream(source); + } } else { new ReadableStream(null); - throws(() => new ReadableStream(42), TypeError); + for (const source of primitives) { + throws(() => new ReadableStream(source), TypeError); + } } + let started = false; + new ReadableStream( + Object.assign(function source() {}, { + start() { + started = true; + }, + }) + ); + strictEqual(started, true); }, }; @@ -44,9 +60,11 @@ export const invalidTypeValidation = { }; // DIVERGENCE: argument conversion order (the WPT constructor.any seed). -// The spec (TypeScript) converts the queuingStrategy FIRST (size before -// highWaterMark), then reads the source's type; C++ reads the source's -// type first, then highWaterMark before size. +// The spec converts the queuingStrategy FIRST (highWaterMark and its +// ToNumber, then size), then reads the source's type. TypeScript reads +// the strategy first but size before highWaterMark, and converts +// highWaterMark only after reading the source's type; C++ reads the +// source's type first, then highWaterMark and its ToNumber, then size. export const argumentConversionOrder = { test() { const order = []; @@ -64,16 +82,101 @@ export const argumentConversionOrder = { }, get highWaterMark() { order.push('strategy.hwm'); - return 1; + return { + valueOf() { + order.push('hwm.valueOf'); + return 1; + }, + }; }, } ); strictEqual( order.join(','), usingTsImpl - ? 'strategy.size,strategy.hwm,source.type' - : 'source.type,strategy.hwm,strategy.size' + ? 'strategy.size,strategy.hwm,source.type,hwm.valueOf' + : 'source.type,strategy.hwm,hwm.valueOf,strategy.size' + ); + }, +}; + +// DIVERGENCE: C++ converts the strategy argument as a WebIDL dictionary: +// null means no strategy and every other primitive throws TypeError. +// TypeScript throws for null (reading its size) and accepts every other +// primitive as no strategy, reading its members through the wrapper +// prototype. Both read a function's members, as the spec does. +export const strategyArgumentShapes = { + test() { + const desiredSize = (strategy) => { + let seen; + new ReadableStream( + { + start(c) { + seen = c.desiredSize; + }, + }, + strategy + ); + return seen; + }; + const primitives = [5, 'x', true, 1n, Symbol('s')]; + if (usingTsImpl) { + throws(() => desiredSize(null), { + name: 'TypeError', + message: "Cannot read properties of null (reading 'size')", + }); + for (const strategy of primitives) { + strictEqual(desiredSize(strategy), 1); + } + } else { + strictEqual(desiredSize(null), 1); + for (const strategy of primitives) { + throws(() => desiredSize(strategy), TypeError); + } + } + const strategy = Object.assign(function strategy() {}, { + highWaterMark: 3, + }); + strictEqual(desiredSize(strategy), 3); + }, +}; + +// DIVERGENCE: which error wins when two are possible. C++ converts the +// source before reading the strategy, so a primitive source beats a +// throwing strategy getter, and converts highWaterMark to an integer +// before validating anything else, so an invalid highWaterMark beats an +// invalid type and a non-callable size. TypeScript reads the strategy +// first, so the getter's error beats the primitive source (the spec +// rejects the source first); an invalid type and a non-callable size +// beat an invalid highWaterMark, as the spec's ExtractHighWaterMark +// range check comes after both. +export const conversionErrorPrecedence = { + test() { + const boom = new Error('boom'); + throws( + () => + new ReadableStream(42, { + get highWaterMark() { + throw boom; + }, + }), + usingTsImpl ? (e) => e === boom : TypeError ); + throws( + () => new ReadableStream({ type: 'potato' }, { highWaterMark: -1 }), + { + name: 'TypeError', + message: usingTsImpl + ? 'Invalid underlying source type: potato' + : 'The value cannot be converted because it is negative and this API expects a positive bigint.', + } + ); + throws(() => new ReadableStream({}, { size: 'x', highWaterMark: NaN }), { + name: 'TypeError', + message: usingTsImpl + ? 'strategy.size must be a function' + : 'The value cannot be converted because it is not an integer.', + }); }, }; diff --git a/src/tests/streams/readable/main.js b/src/tests/streams/readable/main.js index 0d2bab42ddb..3880c059fb3 100644 --- a/src/tests/streams/readable/main.js +++ b/src/tests/streams/readable/main.js @@ -29,6 +29,8 @@ export { garbageSourceValidation, invalidTypeValidation, argumentConversionOrder, + strategyArgumentShapes, + conversionErrorPrecedence, highWaterMarkValidated, hwmDefaultIsOne, } from 'construction'; diff --git a/src/tests/streams/transform/AGENTS.md b/src/tests/streams/transform/AGENTS.md index 85f811acf7b..ce762dff6d8 100644 --- a/src/tests/streams/transform/AGENTS.md +++ b/src/tests/streams/transform/AGENTS.md @@ -40,6 +40,10 @@ basics) are parity under direct observation and are pinned as such. | 21 | (no divergence) `transformer.expectedLength` conversion, as the readable-byte suite's #35 describes: as any `uint64_t` converts, every failure a `TypeError` | same | same | `transformExpectedLengthRejected`, `transformExpectedLengthCoercion`, `transformExpectedLengthRange` (parity) | | 22 | (no divergence) readable.cancel() then writer.write() in the same turn, with room in the readable: the write reaches the sink at once, after the cancel cleared the algorithms and before it settles. With no hooks, flush() only or transform(), the write rejects with the cancel reason and transform() is not called | same | same | `writeAfterCancelSameTurn` (parity) | | 23 | a write whose readable-side size() calls readable.cancel() and then throws e2 (no hooks, flush() only, transform()) | size()'s throw is swallowed (#20): the write fulfills, and the writable is errored with the cancel reason | the spec (TransformStreamDefaultControllerEnqueue step 5), with or without hooks: the write rejects with the closed readable's stored error, undefined, and the writable is errored with e2 | `readableCancelInsideThrowingSize` | +| 24 | constructor argument conversion order | the transformer's members, then each strategy: hwm and its ToNumber, then size (writable, then readable) | the transformer's members, then the writable strategy's size, hwm and its ToNumber, then the readable strategy's hwm, size and hwm's ToNumber. The spec converts both strategies (hwm and its ToNumber, then size, writable first) before the transformer's members | `argumentConversionOrder` | +| 25 | transformer argument shapes | WebIDL dictionary: null means no transformer, a function is a transformer whose members are read; every other primitive throws TypeError | null is accepted as no transformer; every other primitive and a function throw TypeError 'transformer must be an object' (spec: null and the other primitives throw, a function is accepted) | `garbageTransformerValidation` | +| 26 | strategy argument shapes (either strategy) | WebIDL dictionary: null means a strategy with no members; every other primitive throws TypeError. A readable strategy without a highWaterMark (null or `{}`) gets the ReadableStream default (1): the TransformStream default (0) applies only when the argument is omitted | null and every other primitive are accepted as no strategy; any readable strategy without a highWaterMark gets 0 (spec ExtractHighWaterMark). A function's members are read on both sides (spec) | `strategyArgumentShapes` | +| 27 | which error wins when two are possible (a primitive transformer's TypeError beats a throwing strategy getter in both; spec) | all three arguments are converted before anything is validated: a throwing writable strategy getter beats readableType 'bytes'; an invalid writable highWaterMark (TypeError, integer conversion) beats readableType 'bytes' and a throwing readable strategy getter | readableType 'bytes' (RangeError) beats a throwing writable strategy getter and an invalid writable highWaterMark; an invalid writable highWaterMark (RangeError) beats a throwing readable strategy getter. The spec converts both strategies, then checks readableType, then each highWaterMark's range | `conversionErrorPrecedence` | Parity worth noting (probed, pinned): cancel hook runs (not flush) with the reason for both readable.cancel() and writable.abort(), exactly once @@ -47,8 +51,8 @@ even for cancel-then-abort; terminate() reads done + rejects the writable with the same TypeError message on both sides; hook invocation shape (start/transform/flush get 1/2/1 args, receiver = transformer, prototype-chain hooks work); the fixup backpressure LATCH engages on both -sides (only the release diverges, #8); default readable strategy is -hwm 0; chunk identity through the transform; a buffer detached while its +sides (only the release diverges, #8); an omitted readable strategy +is hwm 0 (one without a highWaterMark is #26); chunk identity through the transform; a buffer detached while its view is queued is observed detached by the reader; queue totals recorded at write time. @@ -82,7 +86,7 @@ C++ implementation; `draining-reader.js` asserts both sides. | Module | Asserts | | --- | --- | | `api-surface.js` | transform globals; controller not constructable; bare ctor is standard pass-through, not ITS | -| `construction.js` | ledger #5, #6, #9; both strategies' members read once each (parity) | +| `construction.js` | ledger #5, #6, #9, #24–#27; both strategies' members read once each (parity) | | `transformer-algorithms.js` | ledger #17; start/transform/flush ordering, async hooks, chunk-type freedom; hook shape + prototype-chain (parity) | | `error-propagation.js` | sync/async start/transform/flush error fan-out across writes/close/readable (#1); controller.error() rejects reads; error() no-op after hook throw (parity, identity); enqueue() once flush() has settled throws TypeError and leaves the writable's close untouched (parity) | | `backpressure.js` | writable desiredSize through the transform; dual strategies; default readable hwm 0; latch + racy release (#8); a parked write released by a read transforms even when a same-turn enqueue re-asserts backpressure (parity at hwm 0; #14 at hwm 1) or rejects when the turn also errors the controller (#15) | diff --git a/src/tests/streams/transform/construction.js b/src/tests/streams/transform/construction.js index 1626f3a0a6d..71b1cc0549d 100644 --- a/src/tests/streams/transform/construction.js +++ b/src/tests/streams/transform/construction.js @@ -77,6 +77,204 @@ export const hwmInfinityRejected = { }, }; +// DIVERGENCE: argument conversion order. The spec converts both +// strategies first, the writable then the readable (each highWaterMark +// and its ToNumber, then size), and only then reads the transformer's +// members. C++ reads the transformer's members first, then each strategy +// in that order. TypeScript reads the transformer's members first, then +// the writable strategy's size before its highWaterMark, and converts the +// readable strategy's highWaterMark after reading its size. An empty +// transformer and one with a transform() hook cover the TS +// implementation's elided and standard construction paths. +export const argumentConversionOrder = { + test() { + const loggedStrategy = (order, name) => ({ + get highWaterMark() { + order.push(`${name}.hwm`); + return { + valueOf() { + order.push(`${name}.hwm.valueOf`); + return 1; + }, + }; + }, + get size() { + order.push(`${name}.size`); + return undefined; + }, + }); + for (const hooks of [{}, { transform() {} }]) { + const order = []; + new TransformStream( + { + get start() { + order.push('transformer.start'); + return undefined; + }, + ...hooks, + }, + loggedStrategy(order, 'writable'), + loggedStrategy(order, 'readable') + ); + deepStrictEqual( + order, + usingTsImpl + ? [ + 'transformer.start', + 'writable.size', + 'writable.hwm', + 'writable.hwm.valueOf', + 'readable.hwm', + 'readable.size', + 'readable.hwm.valueOf', + ] + : [ + 'transformer.start', + 'writable.hwm', + 'writable.hwm.valueOf', + 'writable.size', + 'readable.hwm', + 'readable.hwm.valueOf', + 'readable.size', + ] + ); + } + }, +}; + +// DIVERGENCE: C++ converts the transformer as a WebIDL dictionary: null +// means no transformer, a function is a transformer whose members are +// read, and every other primitive throws TypeError. TypeScript accepts +// null too, but throws TypeError for every other primitive and for a +// function. The spec rejects null and the other primitives (the argument +// is typed `object`) and accepts a function. +export const garbageTransformerValidation = { + test() { + const transformerError = { + name: 'TypeError', + message: 'transformer must be an object', + }; + for (const transformer of [42, 'x', true, 1n, Symbol('s')]) { + throws( + () => new TransformStream(transformer), + usingTsImpl ? transformerError : TypeError + ); + } + new TransformStream(null); + let started = false; + const transformer = Object.assign(function transformer() {}, { + start() { + started = true; + }, + }); + if (usingTsImpl) { + throws(() => new TransformStream(transformer), transformerError); + strictEqual(started, false); + } else { + new TransformStream(transformer); + strictEqual(started, true); + } + }, +}; + +// DIVERGENCE: C++ converts each strategy argument as a WebIDL dictionary: +// null means a strategy with no members and every other primitive throws +// TypeError. TypeScript accepts null and every other primitive as no +// strategy, reading a primitive's members through its wrapper prototype. +// A readable strategy without a highWaterMark (null or {}) differs too: +// C++ applies the TransformStream default highWaterMark (0) only when the +// argument is omitted, so the readable side takes the ReadableStream +// default (1); TypeScript applies 0 to any strategy without one, as the +// spec's ExtractHighWaterMark does. Both read a function's members, as +// the spec does. +export const strategyArgumentShapes = { + test() { + // The writable's and the readable's desiredSize at construction. + const desiredSizes = (writableStrategy, readableStrategy) => { + let readable; + const ts = new TransformStream( + { + start(c) { + readable = c.desiredSize; + }, + }, + writableStrategy, + readableStrategy + ); + return [ts.writable.getWriter().desiredSize, readable]; + }; + deepStrictEqual(desiredSizes(undefined, undefined), [1, 0]); + for (const empty of [null, {}]) { + deepStrictEqual(desiredSizes(empty, empty), [1, usingTsImpl ? 0 : 1]); + } + const strategy = Object.assign(function strategy() {}, { + highWaterMark: 3, + }); + deepStrictEqual(desiredSizes(strategy, strategy), [3, 3]); + for (const primitive of [5, 'x', true, 1n, Symbol('s')]) { + if (usingTsImpl) { + deepStrictEqual(desiredSizes(primitive, undefined), [1, 0]); + deepStrictEqual(desiredSizes(undefined, primitive), [1, 0]); + } else { + throws(() => desiredSizes(primitive, undefined), TypeError); + throws(() => desiredSizes(undefined, primitive), TypeError); + } + } + }, +}; + +// DIVERGENCE: which error wins when two are possible. Both reject a +// primitive transformer before reading the strategies, so its TypeError +// beats a throwing strategy getter (spec). C++ converts all three +// arguments before validating anything, converting each highWaterMark +// to an integer: a throwing writable strategy getter beats readableType: +// 'bytes', and an invalid writable highWaterMark beats both readableType: +// 'bytes' and a throwing readable strategy getter. TypeScript checks +// readableType before reading either strategy, and range-checks the +// writable highWaterMark before reading the readable strategy. The spec +// converts both strategies first, then checks readableType, then each +// highWaterMark's range. +export const conversionErrorPrecedence = { + test() { + const boom = new Error('boom'); + const throwingStrategy = { + get highWaterMark() { + throw boom; + }, + }; + const readableTypeError = { + name: 'RangeError', + message: 'transformer.readableType must be undefined', + }; + const negativeHighWaterMark = { + name: 'TypeError', + message: + 'The value cannot be converted because it is negative and this API expects a positive bigint.', + }; + throws( + () => new TransformStream(42, throwingStrategy), + usingTsImpl + ? { name: 'TypeError', message: 'transformer must be an object' } + : TypeError + ); + throws( + () => new TransformStream({ readableType: 'bytes' }, throwingStrategy), + usingTsImpl ? readableTypeError : (e) => e === boom + ); + throws( + () => + new TransformStream({ readableType: 'bytes' }, { highWaterMark: -1 }), + usingTsImpl ? readableTypeError : negativeHighWaterMark + ); + throws( + () => new TransformStream({}, { highWaterMark: -1 }, throwingStrategy), + usingTsImpl + ? { name: 'RangeError', message: 'Invalid highWaterMark' } + : negativeHighWaterMark + ); + }, +}; + // A strategy dictionary whose members count their reads. function countingStrategy(highWaterMark) { const reads = { highWaterMark: 0, size: 0 }; diff --git a/src/tests/streams/transform/main.js b/src/tests/streams/transform/main.js index a7b9fc117b3..9c47906614b 100644 --- a/src/tests/streams/transform/main.js +++ b/src/tests/streams/transform/main.js @@ -26,6 +26,10 @@ export { readableWritableTypeValidation, highWaterMarkValidated, hwmInfinityRejected, + argumentConversionOrder, + garbageTransformerValidation, + strategyArgumentShapes, + conversionErrorPrecedence, strategyMembersReadOnce, } from 'construction'; diff --git a/src/tests/streams/writable/AGENTS.md b/src/tests/streams/writable/AGENTS.md index 87e898fae58..dd3add45859 100644 --- a/src/tests/streams/writable/AGENTS.md +++ b/src/tests/streams/writable/AGENTS.md @@ -35,7 +35,7 @@ the abort reason). | --- | --- | --- | --- | --- | | 1 | invalid highWaterMark at ctor | TypeError (jsg uint64 conversion messages) | RangeError "Invalid highWaterMark" (spec) | `highWaterMarkValidated` | | 2 | sink.type validation | ignored (pedantic_wpt: RangeError) | RangeError "Invalid underlying sink type" (spec) | `sinkTypeValidation` | -| 3 | argument conversion order | sink dictionary first (sink.write, hwm, size) | strategy first (size, hwm, sink.write); strategy-before-sink is spec, but WebIDL reads hwm before size | `argumentConversionOrder` | +| 3 | argument conversion order | sink dictionary first (sink.write, hwm and its ToNumber, size) | strategy first (size, hwm and its ToNumber, sink.write); strategy-before-sink is spec, but WebIDL reads hwm and its ToNumber before size | `argumentConversionOrder` | | 4 | ready at construction (no backpressure) | pending after a microtask, fulfills on a later turn | fulfilled within a microtask (spec) | `readyFulfillTiming` | | 5 | sync start() throw | captured; stream errored, writes reject | escapes the constructor (spec) | `newWritableStreamStartError` | | 6 | abort() on an errored stream | rejects with the stored error | fulfills with undefined (spec) | `newWritableStreamAbortError` | @@ -49,6 +49,9 @@ the abort reason). | 13 | releaseLock() with writes still queued | cancels them: queued writes reject with the released-writer error and their chunks are dropped (a C++ deviation — the source of its WPT piping/flow-control release-then-pipe expectedFailures) | spec: release rejects only ready/closed; queued writes stay in [[writeRequests]] and drain on the sink's schedule (a new writer can relock and they still complete — WPT flow-control pins that; behind a never-settling in-flight write they wait forever on backpressure) | `cancelWriteOnReleaseLock` | | 14 | when a promise returned by start() starts the stream | adopts it: a write queued during start reaches the sink before the first marker chained on it | spec (Node agrees): a new promise is resolved with it, so the write reaches the sink after the second marker, whether it was fulfilled on return or later | `startPromiseSettledInNewPromise` | | 15 | a write from inside sink.close() whose size() throws | rejects with size()'s error | TypeError (closing): the close in flight is checked before the erroring state (spec; Node agrees); the close completes either way | `sizeConsultedForWriteInsideSinkClose` | +| 16 | underlyingSink argument shapes | WebIDL dictionary: null means no sink, a function is a sink whose members are read; every other primitive throws TypeError | null, every other primitive and a function all throw TypeError 'underlyingSink must be an object' (spec: null and the other primitives throw, a function is accepted) | `garbageSinkValidation` | +| 17 | strategy argument shapes | WebIDL dictionary: null means no strategy; every other primitive throws TypeError | null throws TypeError (reading its size); every other primitive is accepted as no strategy. A function's members are read on both sides (spec) | `strategyArgumentShapes` | +| 18 | which error wins when two are possible | a primitive sink (TypeError) beats a throwing strategy getter; an invalid highWaterMark (TypeError, integer conversion) beats an invalid sink type, pedantic_wpt included | the strategy is read first: a throwing strategy getter beats a primitive sink, and an invalid highWaterMark (RangeError 'Invalid highWaterMark') beats an invalid sink type (spec: the primitive sink's TypeError, then the sink type's RangeError) | `conversionErrorPrecedence` | Parity worth noting (probed, pinned): the whole in-flight abort matrix — abort-before-start reason identity on ready/closed, errored-state reason @@ -91,7 +94,7 @@ promises (they resolve with undefined). | Module | Asserts | | --- | --- | | `api-surface.js` | writable globals exist; controller not constructable; bare ctor works (full IDL shape is WPT's) | -| `construction.js` | ledger #1–#5, #12; fractional and ToNumber-coerced hwm accepted; strategy members read once each (parity) | +| `construction.js` | ledger #1–#5, #12, #16–#18; fractional and ToNumber-coerced hwm accepted; strategy members read once each (parity) | | `sink-algorithms.js` | ledger #14; which sink hooks run with what arguments/controller; sync+async hook errors surface on writer promises (#5, #6); size() consulted per write; hook getters read once; second-write rejection fan-out; hooks silent after start throw | | `write-semantics.js` | chunk identity (subarrays, any JS value via Object.is); multiple pending writes; settlement ordering incl. under abort (#7); queued-write fate at releaseLock (#13) | | `buffer-lifecycle.js` | chunks never copied/validated: already-detached AB accepted (byteLength 0); post-write() mutation, detach, resizable grow, and shrink-out-of-bounds all observed per the startedness model (#7); size() runs inside write() so queue totals are immune to later detach | diff --git a/src/tests/streams/writable/construction.js b/src/tests/streams/writable/construction.js index b48550770d3..29fda2c775c 100644 --- a/src/tests/streams/writable/construction.js +++ b/src/tests/streams/writable/construction.js @@ -60,10 +60,11 @@ export const sinkTypeValidation = { // DIVERGENCE: the spec converts the queuingStrategy argument before // inspecting the underlying sink (the WPT constructor.any "converted // after queuingStrategy" case). TypeScript reads strategy.size, then -// strategy.highWaterMark, then sink.write. C++ converts the sink -// dictionary first. Within the strategy, WebIDL converts members in -// lexicographic order (highWaterMark, then size), as C++ does; TypeScript -// reads size first, so only its strategy-before-sink order is spec. +// strategy.highWaterMark and its ToNumber, then sink.write. C++ converts +// the sink dictionary first. Within the strategy, WebIDL converts members +// in lexicographic order (highWaterMark and its ToNumber, then size), as +// C++ does; TypeScript reads size first, so only its strategy-before-sink +// order is spec. export const argumentConversionOrder = { test() { const order = []; @@ -84,7 +85,12 @@ export const argumentConversionOrder = { Object.defineProperty(strategy, 'highWaterMark', { get() { order.push('strategy.highWaterMark'); - return 1; + return { + valueOf() { + order.push('hwm.valueOf'); + return 1; + }, + }; }, }); new WritableStream(sink, strategy); @@ -92,18 +98,121 @@ export const argumentConversionOrder = { deepStrictEqual(order, [ 'strategy.size', 'strategy.highWaterMark', + 'hwm.valueOf', 'sink.write', ]); } else { deepStrictEqual(order, [ 'sink.write', 'strategy.highWaterMark', + 'hwm.valueOf', 'strategy.size', ]); } }, }; +// DIVERGENCE: C++ converts the underlyingSink as a WebIDL dictionary: +// null means no sink, a function is a sink whose members are read, and +// every other primitive throws TypeError. TypeScript throws TypeError for +// null, every other primitive and a function alike. The spec rejects +// null and the other primitives (the argument is typed `object`) and +// accepts a function. +export const garbageSinkValidation = { + test() { + const sinkError = { + name: 'TypeError', + message: 'underlyingSink must be an object', + }; + for (const sink of [42, 'x', true, 1n, Symbol('s')]) { + throws( + () => new WritableStream(sink), + usingTsImpl ? sinkError : TypeError + ); + } + let started = false; + const sink = Object.assign(function sink() {}, { + start() { + started = true; + }, + }); + if (usingTsImpl) { + throws(() => new WritableStream(null), sinkError); + throws(() => new WritableStream(sink), sinkError); + strictEqual(started, false); + } else { + new WritableStream(null); + new WritableStream(sink); + strictEqual(started, true); + } + }, +}; + +// DIVERGENCE: C++ converts the strategy argument as a WebIDL dictionary: +// null means no strategy and every other primitive throws TypeError. +// TypeScript throws for null (reading its size) and accepts every other +// primitive as no strategy, reading its members through the wrapper +// prototype. Both read a function's members, as the spec does. +export const strategyArgumentShapes = { + test() { + const desiredSize = (strategy) => + new WritableStream({}, strategy).getWriter().desiredSize; + const primitives = [5, 'x', true, 1n, Symbol('s')]; + if (usingTsImpl) { + throws(() => desiredSize(null), { + name: 'TypeError', + message: "Cannot read properties of null (reading 'size')", + }); + for (const strategy of primitives) { + strictEqual(desiredSize(strategy), 1); + } + } else { + strictEqual(desiredSize(null), 1); + for (const strategy of primitives) { + throws(() => desiredSize(strategy), TypeError); + } + } + const strategy = Object.assign(function strategy() {}, { + highWaterMark: 3, + }); + strictEqual(desiredSize(strategy), 3); + }, +}; + +// DIVERGENCE: which error wins when two are possible. C++ converts the +// sink before reading the strategy, so a primitive sink beats a throwing +// strategy getter, and converts highWaterMark to an integer before the +// sink's type is checked (pedantic_wpt included), so an invalid +// highWaterMark beats an invalid sink type. TypeScript reads the strategy +// first: the getter's error beats the primitive sink, and the +// highWaterMark RangeError beats the sink type's. The spec rejects the +// primitive sink first, and checks the sink's type before the +// highWaterMark. +export const conversionErrorPrecedence = { + test() { + const boom = new Error('boom'); + throws( + () => + new WritableStream(42, { + get highWaterMark() { + throw boom; + }, + }), + usingTsImpl ? (e) => e === boom : TypeError + ); + throws( + () => new WritableStream({ type: 'bytes' }, { highWaterMark: -1 }), + usingTsImpl + ? { name: 'RangeError', message: 'Invalid highWaterMark' } + : { + name: 'TypeError', + message: + 'The value cannot be converted because it is negative and this API expects a positive bigint.', + } + ); + }, +}; + // DIVERGENCE: with no backpressure the spec starts the stream with an // already-resolved ready promise (the WPT constructor.any "ready should // fulfill immediately" case). TypeScript's ready is fulfilled within a diff --git a/src/tests/streams/writable/main.js b/src/tests/streams/writable/main.js index 6a9d42aa180..54ce4c0e273 100644 --- a/src/tests/streams/writable/main.js +++ b/src/tests/streams/writable/main.js @@ -16,6 +16,9 @@ export { highWaterMarkValidated, sinkTypeValidation, argumentConversionOrder, + garbageSinkValidation, + strategyArgumentShapes, + conversionErrorPrecedence, readyFulfillTiming, nonCallableSizeThrows, globalScopePipe, From b816c174a0f95b0e763676d4c32ced0bdfa3eee5 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 13 Oct 2026 01:31:16 +0000 Subject: [PATCH 2/5] Convert stream constructor arguments as WebIDL does The TypeScript ReadableStream, WritableStream and TransformStream constructors each converted their arguments in their own way. ReadableStream accepted a primitive source; WritableStream and TransformStream rejected a function as a sink or transformer; ReadableStream and WritableStream threw on a null strategy, while all three accepted a primitive one; and a strategy's size was read before its highWaterMark. They now convert as WebIDL does, which matches C++ wherever C++ and the spec agree: - the source, sink or transformer must be an object, a function included; null and every other primitive throw TypeError; - each strategy converts as a dictionary: null means no strategy, every other primitive throws TypeError, and highWaterMark (with its ToNumber) is read before size; - the strategies are converted before the first argument's members are read, TransformStream's included; - the highWaterMark RangeError comes after the type checks, as the spec's ExtractHighWaterMark does. Two changes move TypeScript away from C++. new TransformStream(null) throws, as new ReadableStream(null) and new WritableStream(null) already did, where C++ accepts all three. TransformStream reads its strategies before its transformer, where C++ reads the transformer first. The conversion lives in common.ts: requireObject, convertQueuingStrategy, and the spec's ExtractHighWaterMark and ExtractSizeAlgorithm, shared by the three constructors and by IdentityTransformStream and FixedLengthStream, whose behavior is unchanged. The highWaterMark range check, repeated four times, is now done in one place. --- src/per_isolate/webstreams/AGENTS.md | 2 +- src/per_isolate/webstreams/common.ts | 103 +++++++++++++++++- src/per_isolate/webstreams/identity.ts | 25 +---- src/per_isolate/webstreams/readable.ts | 57 ++++------ src/per_isolate/webstreams/transform.ts | 34 +++--- src/per_isolate/webstreams/writable.ts | 45 +++----- src/tests/streams/readable/AGENTS.md | 8 +- src/tests/streams/readable/construction.js | 96 ++++++++-------- src/tests/streams/transform/AGENTS.md | 8 +- src/tests/streams/transform/construction.js | 115 +++++++++----------- src/tests/streams/writable/AGENTS.md | 8 +- src/tests/streams/writable/construction.js | 105 ++++++++---------- 12 files changed, 317 insertions(+), 289 deletions(-) diff --git a/src/per_isolate/webstreams/AGENTS.md b/src/per_isolate/webstreams/AGENTS.md index 77d72da2af9..3c3591ba674 100644 --- a/src/per_isolate/webstreams/AGENTS.md +++ b/src/per_isolate/webstreams/AGENTS.md @@ -14,7 +14,7 @@ private-brand dispatch, no `instanceof`) apply here — see | ------------- | -------------------------------------------------------------------------------------------- | | `ring-buffer.ts` | O(1) FIFO with indexed access backing every internal queue below; leaf module | | `view-extent.ts` | A caller-supplied view's byte extent through captured getters, and any BufferSource's (`bufferSourceExtent`); detached/out-of-bounds inputs (DataViews included) read as empty; `copyExtent` copies an extent; leaf module | -| `common.ts` | Helpers shared by the webstreams modules, including `normalizeExpectedLength` (every `expectedLength` and `FixedLengthStream` length converts through it, as C++ converts a uint64_t); leaf module | +| `common.ts` | Helpers shared by the webstreams modules, including the constructors' WebIDL argument conversion (`requireObject` for a source, sink or transformer; `convertQueuingStrategy`, then `extractHighWaterMark` and `extractSizeAlgorithm`, for a strategy) and `normalizeExpectedLength` (every `expectedLength` and `FixedLengthStream` length converts through it, as C++ converts a uint64_t); leaf module | | `queue.ts` | QUEUED backend: single-queue/multi-cursor, JS sources; fence interfaces; invariant list | | `native.ts` | NATIVE backend: C++-backed pull conduit; **the C++/JS contract** + invariants | | `readable.ts` | Reader layer + queued controllers + the BACKEND-DISPATCH points (constructor, tee, chains, byte-capable gate, JS-to-C++ extraction) | diff --git a/src/per_isolate/webstreams/common.ts b/src/per_isolate/webstreams/common.ts index d356b2939de..667f3106689 100644 --- a/src/per_isolate/webstreams/common.ts +++ b/src/per_isolate/webstreams/common.ts @@ -2,7 +2,17 @@ // Helpers shared by the webstreams modules. Leaf module: requires nothing. -const { BigInt, NumberIsFinite, ObjectFreeze, TypeError } = primordials; +import type { QueuingStrategy } from './types'; + +const { + BigInt, + NumberIsFinite, + NumberIsNaN, + ObjectFreeze, + RangeError, + TypeError, + uncurryThis, +} = primordials; function isActualObject(value: unknown): value is object { return value != null && typeof value === 'object'; @@ -58,12 +68,97 @@ function normalizeExpectedLength(value: unknown): bigint | undefined { return BigInt(length - (length % 1)); } +// WebIDL conversion of a constructor argument typed `object` (an underlying +// source or sink, a transformer): any object, a function included, passes; +// null and every other primitive throw. Nothing is read. An omitted argument +// defaults to kNullProtoObject before it gets here. +function requireObject(value: unknown, message: string): void { + if (!isActualObject(value) && typeof value !== 'function') { + throw new TypeError(message); + } +} + +// WebIDL dictionary conversion of a strategy argument, as in C++: undefined +// and null mean no strategy; any other non-object throws. +function toStrategyDictionary(value: unknown): QueuingStrategy { + if (value === undefined || value === null) return kNullProtoObject; + if (typeof value !== 'object' && typeof value !== 'function') { + throw new TypeError('The queuing strategy must be an object.'); + } + return value as QueuingStrategy; +} + +// A strategy argument as WebIDL converts a QueuingStrategy dictionary: each +// member read once, in lexicographic order. highWaterMark has been through +// ToNumber; size is a function. An absent member is undefined. +interface QueuingStrategyDict { + highWaterMark: number | undefined; + size: ((chunk: unknown) => number) | undefined; +} + +function convertQueuingStrategy(value: unknown): QueuingStrategyDict { + const strategy = toStrategyDictionary(value); + const rawHighWaterMark: unknown = strategy.highWaterMark; + // Unary plus is ToNumber (see normalizeExpectedLength). + const highWaterMark = + rawHighWaterMark === undefined ? undefined : +(rawHighWaterMark as number); + const size: unknown = strategy.size; + if (size !== undefined && typeof size !== 'function') { + throw new TypeError('strategy.size must be a function'); + } + return { + __proto__: null, + highWaterMark, + size, + } as QueuingStrategyDict; +} + +// ToNumber and the range check of the spec's ExtractHighWaterMark: NaN and +// negative numbers are a RangeError; Infinity and fractions are kept. +// Converting a number again has no side effects. +function convertHighWaterMark(value: unknown): number { + const highWaterMark = +(value as number); + if (NumberIsNaN(highWaterMark) || highWaterMark < 0) { + throw new RangeError('Invalid highWaterMark'); + } + return highWaterMark; +} + +// The spec's ExtractHighWaterMark, on a converted strategy. +function extractHighWaterMark( + strategy: QueuingStrategyDict, + defaultHighWaterMark: number +): number { + const { highWaterMark } = strategy; + return highWaterMark === undefined + ? defaultHighWaterMark + : convertHighWaterMark(highWaterMark); +} + +// The spec's ExtractSizeAlgorithm, on a converted strategy: size() runs with +// an undefined receiver and the chunk as its only argument. Without one, +// every chunk counts 1. +function extractSizeAlgorithm( + strategy: QueuingStrategyDict +): (chunk: T) => number { + const { size } = strategy; + if (size === undefined) return () => 1; + const callSize = uncurryThis(size); + return (chunk: T) => callSize(undefined, chunk); +} + export type CommonHelpers = { isActualObject: typeof isActualObject; kNullProtoObject: typeof kNullProtoObject; kEnumerable: typeof kEnumerable; toStringTagProperty: typeof toStringTagProperty; normalizeExpectedLength: typeof normalizeExpectedLength; + requireObject: typeof requireObject; + toStrategyDictionary: typeof toStrategyDictionary; + convertQueuingStrategy: typeof convertQueuingStrategy; + convertHighWaterMark: typeof convertHighWaterMark; + extractHighWaterMark: typeof extractHighWaterMark; + extractSizeAlgorithm: typeof extractSizeAlgorithm; }; module.exports = ObjectFreeze({ @@ -72,4 +167,10 @@ module.exports = ObjectFreeze({ kEnumerable, toStringTagProperty, normalizeExpectedLength, + requireObject, + toStrategyDictionary, + convertQueuingStrategy, + convertHighWaterMark, + extractHighWaterMark, + extractSizeAlgorithm, }); diff --git a/src/per_isolate/webstreams/identity.ts b/src/per_isolate/webstreams/identity.ts index 0bd6631389d..5a7edc43c3b 100644 --- a/src/per_isolate/webstreams/identity.ts +++ b/src/per_isolate/webstreams/identity.ts @@ -99,7 +99,6 @@ const { ArrayPrototypePush, BigInt, Number, - NumberIsNaN, ObjectDefineProperties, PromisePrototypeThen, PromiseResolve, @@ -136,6 +135,8 @@ const { kEnumerable, normalizeExpectedLength, toStringTagProperty, + toStrategyDictionary, + convertHighWaterMark, } = require('webstreams/common') as CommonHelpers; const { bufferSourceExtent, copyExtent } = require('webstreams/view-extent') as ViewExtentHelpers; @@ -204,28 +205,6 @@ let assertIsIdentityTransformStream: (self: IdentityTransformStream) => void; const kPrivateSymbol: symbol = Symbol('private'); -// The writable's WHATWG high-water-mark conversion (ToNumber; NaN or a -// negative number is a RangeError), for FixedLengthStream, which caps the -// converted number at its length before IdentityTransformStream hands it -// on. Converting a number again there has no side effects. -function convertHighWaterMark(value: unknown): number { - const highWaterMark = +(value as number); - if (NumberIsNaN(highWaterMark) || highWaterMark < 0) { - throw new RangeError('Invalid highWaterMark'); - } - return highWaterMark; -} - -// WebIDL dictionary conversion of a constructor's strategy argument, as in -// C++: undefined and null mean no strategy; any other non-object throws. -function toStrategyDictionary(value: unknown): QueuingStrategy { - if (value === undefined || value === null) return kNullProtoObject; - if (typeof value !== 'object' && typeof value !== 'function') { - throw new TypeError('The queuing strategy must be an object.'); - } - return value as QueuingStrategy; -} - // A write accepted by the writable, snapshotted in its size() callback (see // sizeAndSnapshot). Entries are tagged with an own `ok` data property rather // than discriminated with an `in` check so that a polluted Object.prototype diff --git a/src/per_isolate/webstreams/readable.ts b/src/per_isolate/webstreams/readable.ts index 5e44bce4cd0..3a8df72242a 100644 --- a/src/per_isolate/webstreams/readable.ts +++ b/src/per_isolate/webstreams/readable.ts @@ -123,6 +123,10 @@ const { kEnumerable, normalizeExpectedLength, toStringTagProperty, + requireObject, + convertQueuingStrategy, + extractHighWaterMark, + extractSizeAlgorithm, } = require('webstreams/common') as CommonHelpers; import type { ViewExtentHelpers } from './view-extent'; @@ -4513,12 +4517,16 @@ class ReadableStream { return; } + // WebIDL argument conversion: the source is typed `object`, so null and + // every other primitive throw before the strategy is read. + requireObject(underlyingSource, 'underlyingSource must be an object'); + // BACKEND-DISPATCH: stream construction (one of the five sanctioned // dispatch points): native vs queued byte vs queued default. // - // The native check runs FIRST: native sources are forbidden from - // declaring `type` (enforced during native extraction), so a - // native-marked source must never fall through to the queued + // The native check runs before any member is read: native sources are + // forbidden from declaring `type` (enforced during native extraction), + // so a native-marked source must never fall through to the queued // branches. The strategy argument is PERMANENTLY ignored on the // native branch: native pacing is purely demand-driven (the source // contractually ignores desiredSize). @@ -4565,23 +4573,10 @@ class ReadableStream { return; } - // --- WebIDL strategy dictionary conversion (BEFORE source reads) --- - // Per WebIDL, dictionary-typed arguments are converted (property reads - // happen) at the IDL layer before the constructor body runs. The - // `strategy` parameter is QueuingStrategy (a dictionary); the - // `underlyingSource` parameter is plain `object` (no dictionary - // conversion). We simulate this by reading strategy properties first. - const sizeFn = strategy.size; - let sizeAlgorithm: (chunk: R) => number; - if (sizeFn === undefined) { - sizeAlgorithm = () => 1; - } else if (typeof sizeFn !== 'function') { - throw new TypeError('strategy.size must be a function'); - } else { - const callSize = uncurryThis(sizeFn); - sizeAlgorithm = (chunk: R) => callSize(undefined, chunk); - } - const rawHWM = strategy.highWaterMark; + // WebIDL converts the strategy dictionary at the IDL layer, before the + // constructor body converts the source to an UnderlyingSource + // dictionary and reads its members. + const strategyDict = convertQueuingStrategy(strategy); // WebIDL: the 'type' member is a string enum — perform ToString() on // non-undefined values so that objects with toString()/valueOf() work @@ -4592,22 +4587,15 @@ class ReadableStream { if (type === 'bytes') { // Byte streams: size() is forbidden, highWaterMark defaults to 0. - if (sizeFn !== undefined) { + if (strategyDict.size !== undefined) { throw new RangeError( 'The strategy for a byte stream cannot have a size function' ); } - let highWaterMark = 0; - if (rawHWM !== undefined) { - highWaterMark = +rawHWM; - if (NumberIsNaN(highWaterMark) || highWaterMark < 0) { - throw new RangeError('Invalid highWaterMark'); - } - } this.#controller = setupReadableByteStreamControllerFromUnderlyingSource( this, underlyingSource, - highWaterMark + extractHighWaterMark(strategyDict, 0) ); } else { // The spec says, "Assert: underlyingSourceDict["type"] does not exist" @@ -4617,19 +4605,12 @@ class ReadableStream { throw new TypeError(`Invalid underlying source type: ${type}`); } // Default streams: highWaterMark defaults to 1. - let highWaterMark = 1; - if (rawHWM !== undefined) { - highWaterMark = +rawHWM; - if (NumberIsNaN(highWaterMark) || highWaterMark < 0) { - throw new RangeError('Invalid highWaterMark'); - } - } this.#controller = setupReadableStreamDefaultControllerFromUnderlyingSource( this, underlyingSource, - sizeAlgorithm, - highWaterMark + extractSizeAlgorithm(strategyDict), + extractHighWaterMark(strategyDict, 1) ); } } diff --git a/src/per_isolate/webstreams/transform.ts b/src/per_isolate/webstreams/transform.ts index 61def2e845a..169581fe7ba 100644 --- a/src/per_isolate/webstreams/transform.ts +++ b/src/per_isolate/webstreams/transform.ts @@ -44,6 +44,8 @@ const { kEnumerable, normalizeExpectedLength, toStringTagProperty, + requireObject, + convertQueuingStrategy, } = require('webstreams/common') as CommonHelpers; const kPrivateSymbol: symbol = Symbol('private'); @@ -471,17 +473,17 @@ class TransformStream { } constructor( - transformer?: Transformer, - writableStrategy?: QueuingStrategy, - readableStrategy?: QueuingStrategy + transformer: Transformer = kNullProtoObject as Transformer, + writableStrategy: QueuingStrategy = kNullProtoObject as QueuingStrategy, + readableStrategy: QueuingStrategy = kNullProtoObject as QueuingStrategy ) { - transformer ??= kNullProtoObject as Transformer; - writableStrategy ??= kNullProtoObject as QueuingStrategy; - readableStrategy ??= kNullProtoObject as QueuingStrategy; - - if (!isActualObject(transformer)) { - throw new TypeError('transformer must be an object'); - } + // WebIDL converts the arguments at the IDL layer, before the constructor + // body runs: the transformer is typed `object` (null and every other + // primitive throw, nothing is read), and each strategy is a + // QueuingStrategy dictionary, the writable's first. + requireObject(transformer, 'transformer must be an object'); + const writableStrategyDict = convertQueuingStrategy(writableStrategy); + const readableStrategyDict = convertQueuingStrategy(readableStrategy); // --- Transformer method extraction (alphabetical property reads) --- const cancelFn = transformer.cancel; @@ -648,16 +650,15 @@ class TransformStream { close: () => this.#sinkClose(), abort: (reason: unknown) => this.#sinkAbort(reason), }, - writableStrategy + // Converted already: the inner stream reads its own data properties, + // and its default highWaterMark (1) is the TransformStream's. + writableStrategyDict as QueuingStrategy ); const sourcePull = (): Promise => { this.#setBackpressure(false); return this.#backpressureChange.promise; }; - // Read once, as WebIDL dictionary conversion reads each member once. - const rawReadableHWM = readableStrategy.highWaterMark; - const readableHWM = rawReadableHWM === undefined ? 0 : rawReadableHWM; this.#readable = new ReadableStream( { __proto__: null, @@ -670,8 +671,9 @@ class TransformStream { }, { __proto__: null, - highWaterMark: readableHWM, - size: readableStrategy.size, + // The TransformStream default, where the ReadableStream's is 1. + highWaterMark: readableStrategyDict.highWaterMark ?? 0, + size: readableStrategyDict.size, } ); if (expectedLength !== undefined) { diff --git a/src/per_isolate/webstreams/writable.ts b/src/per_isolate/webstreams/writable.ts index 559a097adde..905bf29ba43 100644 --- a/src/per_isolate/webstreams/writable.ts +++ b/src/per_isolate/webstreams/writable.ts @@ -61,8 +61,16 @@ const { nativeStreamInternals } = require('webstreams/native') as { const { kExtractNativeSink, isNativeUnderlyingSink } = nativeStreamInternals; import type { CommonHelpers } from './common'; -const { isActualObject, kNullProtoObject, kEnumerable, toStringTagProperty } = - require('webstreams/common') as CommonHelpers; +const { + isActualObject, + kNullProtoObject, + kEnumerable, + toStringTagProperty, + requireObject, + convertQueuingStrategy, + extractHighWaterMark, + extractSizeAlgorithm, +} = require('webstreams/common') as CommonHelpers; import type { ViewExtentHelpers } from './view-extent'; const { viewByteLength } = @@ -791,37 +799,18 @@ class WritableStream { value: true, } as PropertyDescriptor); - // --- WebIDL strategy dictionary conversion (BEFORE sink reads) --- - // Per WebIDL, dictionary-typed arguments are converted at the IDL - // layer before the constructor body runs. strategy is QueuingStrategy - // (a dictionary); underlyingSink is plain object. - const sizeFn = strategy.size; - let sizeAlgorithm: (chunk: W) => number; - if (sizeFn === undefined) { - sizeAlgorithm = () => 1; - } else if (typeof sizeFn !== 'function') { - throw new TypeError('strategy.size must be a function'); - } else { - const callSize = uncurryThis(sizeFn); - sizeAlgorithm = (chunk: W) => callSize(undefined, chunk); - } - // Read once, as WebIDL dictionary conversion reads each member once. - const rawHWM = strategy.highWaterMark; - let highWaterMark = 1; - if (rawHWM !== undefined) { - highWaterMark = +rawHWM; - if (NumberIsNaN(highWaterMark) || highWaterMark < 0) { - throw new RangeError('Invalid highWaterMark'); - } - } + // WebIDL converts the arguments at the IDL layer, before the constructor + // body runs: the sink is typed `object` (null and every other primitive + // throw, nothing is read), the strategy is a QueuingStrategy dictionary. + requireObject(underlyingSink, 'underlyingSink must be an object'); + const strategyDict = convertQueuingStrategy(strategy); // --- Now read underlyingSink properties --- - if (!isActualObject(underlyingSink)) { - throw new TypeError('underlyingSink must be an object'); - } if (underlyingSink.type !== undefined) { throw new RangeError('Invalid underlying sink type'); } + const sizeAlgorithm = extractSizeAlgorithm(strategyDict); + const highWaterMark = extractHighWaterMark(strategyDict, 1); // Native sink detection: the standard WritableStream machinery // drives the sink via start/write/close/abort as-is. The marker diff --git a/src/tests/streams/readable/AGENTS.md b/src/tests/streams/readable/AGENTS.md index 81ecdb73486..48117866dad 100644 --- a/src/tests/streams/readable/AGENTS.md +++ b/src/tests/streams/readable/AGENTS.md @@ -15,8 +15,8 @@ behavioral gaps; the reentrancy family is mostly parity at finite hwm. | # | Area | C++ | TypeScript | Pinned in | | --- | --- | --- | --- | --- | -| 1 | garbage underlyingSource | accepts null; every other primitive throws TypeError | rejects null (spec: TypeError); accepts every other primitive, reading its members through the wrapper prototype. A function is accepted on both sides, its members read (spec) | `garbageSourceValidation` | -| 2 | ctor argument conversion order | source.type, hwm and its ToNumber, size | strategy size, hwm, then source.type, then hwm's ToNumber (spec: the strategy first, but hwm and its ToNumber before size) | `argumentConversionOrder` | +| 1 | null underlyingSource (every other primitive throws TypeError, and a function is accepted with its members read, in both; spec) | accepted as no source (a WebIDL dictionary) | TypeError 'underlyingSource must be an object' (spec: the argument is typed `object`) | `garbageSourceValidation` | +| 2 | ctor argument conversion order | source.type, then the strategy: hwm and its ToNumber, size | the strategy first (spec): hwm and its ToNumber, size, then source.type | `argumentConversionOrder` | | 3 | invalid hwm (-1/NaN) + Infinity | TypeError; Infinity REJECTED (integer conversion) — root cause of many WPT reentrant/errors failures | RangeError; Infinity accepted (spec) | `highWaterMarkValidated` | | 4 | pull counts (hwm 1, enqueue-in-pull) | 1,1,3 — first read served from queue without pull, deferred pulls batch | 1,2,3 (spec: pull after start and every read) | `pullCountShape` | | 5 | pull on reading the LAST queued chunk | no pull | pulls (spec) | `pullOnLastChunkRead` | @@ -45,8 +45,8 @@ behavioral gaps; the reentrancy family is mostly parity at finite hwm. | 28 | from() iterator protocol edges | reads `value` of an async iterator's final result; a sync iterator whose value rejects (not done) is left open; the `value` of a sync return() result is not resolved; a non-callable `next` ends the stream; DEFECT: a throwing `return` getter's error escapes into the I/O context as an uncaught exception and fails the request, whether or not the cancel's promise is handled (unpinnable) | spec (WebIDL async_sequence + ECMA-262 async-from-sync): only `done` of the final result; the sync iterator is closed through return(); the return() result's value is resolved (a rejection rejects the cancel); the read rejects with TypeError; the cancel rejects | `fromIteratorProtocolEdges`, `fromCancelReturnLookup` | | 29 | when a sync iterable's from() reads and cancels settle, relative to an async iterator whose next()/return() returns a settled promise | one tick earlier; a throwing next()/return() in the same tick | two ticks earlier, and a throwing one in the same tick: the async-from-sync continuation is reacted to directly, not through "a promise resolved with" it (the spec lands both on the same tick, and a throwing sync one two ticks later); intentional, sparing two microtasks and a Promise.prototype.then lookup per chunk | `fromSyncIterableSettlesEarly` | | 30 | close() inside size() while chunks are still to be read (no tee: a chunk queued; tee: every branch behind the close) | the in-flight chunk is dropped (the transform suite's ledger #10 shows the same through writer.close()) | the close is only requested and the chunk is still readable: every branch reads it (spec without a tee; Node agrees there). With a tee the chunk reaches every branch or none: one already drained to the close drops it for all (parity, `closeInsideSizeTeeDrainedBranch`) | `closeInsideSizeWithQueuedChunk`, `closeInsideSizeTeeBranchesBehind` | -| 31 | strategy argument shapes | WebIDL dictionary: null means no strategy; every other primitive throws TypeError | null throws TypeError (reading its size); every other primitive is accepted as no strategy, its members read through the wrapper prototype. A function's members are read on both sides (spec) | `strategyArgumentShapes` | -| 32 | which error wins when two are possible | a primitive source (TypeError) beats a throwing strategy getter; an invalid highWaterMark (TypeError, integer conversion) beats an invalid type and a non-callable size | the strategy is read first, so a throwing strategy getter beats a primitive source (spec: the source's TypeError); an invalid type and a non-callable size (TypeError) beat an invalid highWaterMark (spec) | `conversionErrorPrecedence` | +| 31 | (no divergence) the strategy argument converts as a WebIDL dictionary: null means no strategy, a function's members are read, and every other primitive throws TypeError (messages differ) | same | same | `strategyArgumentShapes` (parity) | +| 32 | which error wins when two are possible (a primitive source's TypeError beats a throwing strategy getter in both; spec) | an invalid highWaterMark (TypeError, integer conversion) beats an invalid type and a non-callable size | an invalid type and a non-callable size (TypeError) beat an invalid highWaterMark (spec) | `conversionErrorPrecedence` | Parity worth noting (probed, pinned): pull serialization (never re-entered); pull/async-start rejection identity; error-undefined diff --git a/src/tests/streams/readable/construction.js b/src/tests/streams/readable/construction.js index 29d999e3eb8..70f69b60de9 100644 --- a/src/tests/streams/readable/construction.js +++ b/src/tests/streams/readable/construction.js @@ -9,28 +9,27 @@ import { strictEqual, throws } from 'node:assert'; import { usingTsImpl } from 'which-impl'; -// DIVERGENCE (mirror-image!): C++ converts the underlyingSource as a -// WebIDL dictionary: null means no source and every other primitive -// throws TypeError. TypeScript rejects null (spec: the argument is typed -// `object`, which null is not) but accepts every other primitive, -// reading its members through the wrapper prototype. Both accept a -// function and read its members, as the spec does. +// DIVERGENCE: a null underlyingSource is accepted by C++, which converts +// it as a WebIDL dictionary (no source), but rejected by TypeScript +// (spec: the argument is typed `object`, which null is not). Every other +// primitive throws TypeError, and a function is accepted and its members +// read, in both (spec). export const garbageSourceValidation = { test() { - const primitives = [42, 'x', true, 1n, Symbol('s')]; + const sourceError = { + name: 'TypeError', + message: 'underlyingSource must be an object', + }; if (usingTsImpl) { - throws(() => new ReadableStream(null), { - name: 'TypeError', - message: 'Cannot convert undefined or null to object', - }); - for (const source of primitives) { - new ReadableStream(source); - } + throws(() => new ReadableStream(null), sourceError); } else { new ReadableStream(null); - for (const source of primitives) { - throws(() => new ReadableStream(source), TypeError); - } + } + for (const source of [42, 'x', true, 1n, Symbol('s')]) { + throws( + () => new ReadableStream(source), + usingTsImpl ? sourceError : TypeError + ); } let started = false; new ReadableStream( @@ -60,11 +59,9 @@ export const invalidTypeValidation = { }; // DIVERGENCE: argument conversion order (the WPT constructor.any seed). -// The spec converts the queuingStrategy FIRST (highWaterMark and its -// ToNumber, then size), then reads the source's type. TypeScript reads -// the strategy first but size before highWaterMark, and converts -// highWaterMark only after reading the source's type; C++ reads the -// source's type first, then highWaterMark and its ToNumber, then size. +// The spec (TypeScript) converts the queuingStrategy FIRST (highWaterMark +// and its ToNumber, then size), then reads the source's type; C++ reads +// the source's type first, then the strategy in the same order. export const argumentConversionOrder = { test() { const order = []; @@ -94,17 +91,15 @@ export const argumentConversionOrder = { strictEqual( order.join(','), usingTsImpl - ? 'strategy.size,strategy.hwm,source.type,hwm.valueOf' + ? 'strategy.hwm,hwm.valueOf,strategy.size,source.type' : 'source.type,strategy.hwm,hwm.valueOf,strategy.size' ); }, }; -// DIVERGENCE: C++ converts the strategy argument as a WebIDL dictionary: -// null means no strategy and every other primitive throws TypeError. -// TypeScript throws for null (reading its size) and accepts every other -// primitive as no strategy, reading its members through the wrapper -// prototype. Both read a function's members, as the spec does. +// The strategy argument converts as a WebIDL dictionary in both (spec): +// null means no strategy, a function's members are read, and every other +// primitive throws TypeError (messages differ). export const strategyArgumentShapes = { test() { const desiredSize = (strategy) => { @@ -119,20 +114,17 @@ export const strategyArgumentShapes = { ); return seen; }; - const primitives = [5, 'x', true, 1n, Symbol('s')]; - if (usingTsImpl) { - throws(() => desiredSize(null), { - name: 'TypeError', - message: "Cannot read properties of null (reading 'size')", - }); - for (const strategy of primitives) { - strictEqual(desiredSize(strategy), 1); - } - } else { - strictEqual(desiredSize(null), 1); - for (const strategy of primitives) { - throws(() => desiredSize(strategy), TypeError); - } + strictEqual(desiredSize(null), 1); + for (const strategy of [5, 'x', true, 1n, Symbol('s')]) { + throws( + () => desiredSize(strategy), + usingTsImpl + ? { + name: 'TypeError', + message: 'The queuing strategy must be an object.', + } + : TypeError + ); } const strategy = Object.assign(function strategy() {}, { highWaterMark: 3, @@ -141,15 +133,13 @@ export const strategyArgumentShapes = { }, }; -// DIVERGENCE: which error wins when two are possible. C++ converts the -// source before reading the strategy, so a primitive source beats a -// throwing strategy getter, and converts highWaterMark to an integer -// before validating anything else, so an invalid highWaterMark beats an -// invalid type and a non-callable size. TypeScript reads the strategy -// first, so the getter's error beats the primitive source (the spec -// rejects the source first); an invalid type and a non-callable size -// beat an invalid highWaterMark, as the spec's ExtractHighWaterMark -// range check comes after both. +// DIVERGENCE: which error wins when two are possible. Both reject a +// primitive source before reading the strategy, so its TypeError beats a +// throwing strategy getter (spec). An invalid type and a non-callable +// size beat an invalid highWaterMark under TypeScript, as the spec's +// ExtractHighWaterMark range check comes after both; C++ converts +// highWaterMark to an integer before validating anything else, so its +// TypeError wins. export const conversionErrorPrecedence = { test() { const boom = new Error('boom'); @@ -160,7 +150,9 @@ export const conversionErrorPrecedence = { throw boom; }, }), - usingTsImpl ? (e) => e === boom : TypeError + usingTsImpl + ? { name: 'TypeError', message: 'underlyingSource must be an object' } + : TypeError ); throws( () => new ReadableStream({ type: 'potato' }, { highWaterMark: -1 }), diff --git a/src/tests/streams/transform/AGENTS.md b/src/tests/streams/transform/AGENTS.md index ce762dff6d8..13f8ab9aff2 100644 --- a/src/tests/streams/transform/AGENTS.md +++ b/src/tests/streams/transform/AGENTS.md @@ -40,10 +40,10 @@ basics) are parity under direct observation and are pinned as such. | 21 | (no divergence) `transformer.expectedLength` conversion, as the readable-byte suite's #35 describes: as any `uint64_t` converts, every failure a `TypeError` | same | same | `transformExpectedLengthRejected`, `transformExpectedLengthCoercion`, `transformExpectedLengthRange` (parity) | | 22 | (no divergence) readable.cancel() then writer.write() in the same turn, with room in the readable: the write reaches the sink at once, after the cancel cleared the algorithms and before it settles. With no hooks, flush() only or transform(), the write rejects with the cancel reason and transform() is not called | same | same | `writeAfterCancelSameTurn` (parity) | | 23 | a write whose readable-side size() calls readable.cancel() and then throws e2 (no hooks, flush() only, transform()) | size()'s throw is swallowed (#20): the write fulfills, and the writable is errored with the cancel reason | the spec (TransformStreamDefaultControllerEnqueue step 5), with or without hooks: the write rejects with the closed readable's stored error, undefined, and the writable is errored with e2 | `readableCancelInsideThrowingSize` | -| 24 | constructor argument conversion order | the transformer's members, then each strategy: hwm and its ToNumber, then size (writable, then readable) | the transformer's members, then the writable strategy's size, hwm and its ToNumber, then the readable strategy's hwm, size and hwm's ToNumber. The spec converts both strategies (hwm and its ToNumber, then size, writable first) before the transformer's members | `argumentConversionOrder` | -| 25 | transformer argument shapes | WebIDL dictionary: null means no transformer, a function is a transformer whose members are read; every other primitive throws TypeError | null is accepted as no transformer; every other primitive and a function throw TypeError 'transformer must be an object' (spec: null and the other primitives throw, a function is accepted) | `garbageTransformerValidation` | -| 26 | strategy argument shapes (either strategy) | WebIDL dictionary: null means a strategy with no members; every other primitive throws TypeError. A readable strategy without a highWaterMark (null or `{}`) gets the ReadableStream default (1): the TransformStream default (0) applies only when the argument is omitted | null and every other primitive are accepted as no strategy; any readable strategy without a highWaterMark gets 0 (spec ExtractHighWaterMark). A function's members are read on both sides (spec) | `strategyArgumentShapes` | -| 27 | which error wins when two are possible (a primitive transformer's TypeError beats a throwing strategy getter in both; spec) | all three arguments are converted before anything is validated: a throwing writable strategy getter beats readableType 'bytes'; an invalid writable highWaterMark (TypeError, integer conversion) beats readableType 'bytes' and a throwing readable strategy getter | readableType 'bytes' (RangeError) beats a throwing writable strategy getter and an invalid writable highWaterMark; an invalid writable highWaterMark (RangeError) beats a throwing readable strategy getter. The spec converts both strategies, then checks readableType, then each highWaterMark's range | `conversionErrorPrecedence` | +| 24 | constructor argument conversion order | the transformer's members, then each strategy: hwm and its ToNumber, then size (writable, then readable) | both strategies in that order, then the transformer's members (spec) | `argumentConversionOrder` | +| 25 | null transformer (every other primitive throws TypeError, and a function is accepted with its members read, in both; spec) | accepted as no transformer (a WebIDL dictionary) | TypeError 'transformer must be an object' (spec: the argument is typed `object`) | `garbageTransformerValidation` | +| 26 | readable strategy without a highWaterMark (null or `{}`); either strategy otherwise converts as a WebIDL dictionary in both: a function's members are read, and every primitive except null throws TypeError (messages differ) | the ReadableStream default (1): the TransformStream default (0) applies only when the argument is omitted | 0 (spec ExtractHighWaterMark) | `strategyArgumentShapes` | +| 27 | which error wins when two are possible (a primitive transformer's TypeError beats a throwing strategy getter, and a throwing writable strategy getter beats readableType 'bytes', in both; spec) | each highWaterMark is converted to an integer with the arguments: an invalid writable highWaterMark (TypeError) beats readableType 'bytes' and a throwing readable strategy getter | spec: both strategies are converted, then readableType is checked, then each highWaterMark's range, so readableType 'bytes' (RangeError) and a throwing readable strategy getter beat an invalid writable highWaterMark | `conversionErrorPrecedence` | Parity worth noting (probed, pinned): cancel hook runs (not flush) with the reason for both readable.cancel() and writable.abort(), exactly once diff --git a/src/tests/streams/transform/construction.js b/src/tests/streams/transform/construction.js index 71b1cc0549d..4db0c5e0c3b 100644 --- a/src/tests/streams/transform/construction.js +++ b/src/tests/streams/transform/construction.js @@ -77,15 +77,13 @@ export const hwmInfinityRejected = { }, }; -// DIVERGENCE: argument conversion order. The spec converts both -// strategies first, the writable then the readable (each highWaterMark -// and its ToNumber, then size), and only then reads the transformer's -// members. C++ reads the transformer's members first, then each strategy -// in that order. TypeScript reads the transformer's members first, then -// the writable strategy's size before its highWaterMark, and converts the -// readable strategy's highWaterMark after reading its size. An empty -// transformer and one with a transform() hook cover the TS -// implementation's elided and standard construction paths. +// DIVERGENCE: argument conversion order. The spec (TypeScript) converts +// both strategies first, the writable then the readable (each +// highWaterMark and its ToNumber, then size), and only then reads the +// transformer's members. C++ reads the transformer's members first, then +// each strategy in that order. An empty transformer and one with a +// transform() hook cover the TS implementation's elided and standard +// construction paths. export const argumentConversionOrder = { test() { const loggedStrategy = (order, name) => ({ @@ -120,13 +118,13 @@ export const argumentConversionOrder = { order, usingTsImpl ? [ - 'transformer.start', - 'writable.size', 'writable.hwm', 'writable.hwm.valueOf', + 'writable.size', 'readable.hwm', - 'readable.size', 'readable.hwm.valueOf', + 'readable.size', + 'transformer.start', ] : [ 'transformer.start', @@ -142,51 +140,47 @@ export const argumentConversionOrder = { }, }; -// DIVERGENCE: C++ converts the transformer as a WebIDL dictionary: null -// means no transformer, a function is a transformer whose members are -// read, and every other primitive throws TypeError. TypeScript accepts -// null too, but throws TypeError for every other primitive and for a -// function. The spec rejects null and the other primitives (the argument -// is typed `object`) and accepts a function. +// DIVERGENCE: a null transformer is accepted by C++, which converts it as +// a WebIDL dictionary (no transformer), but rejected by TypeScript (spec: +// the argument is typed `object`, which null is not). Every other +// primitive throws TypeError, and a function is accepted and its members +// read, in both (spec). export const garbageTransformerValidation = { test() { const transformerError = { name: 'TypeError', message: 'transformer must be an object', }; + if (usingTsImpl) { + throws(() => new TransformStream(null), transformerError); + } else { + new TransformStream(null); + } for (const transformer of [42, 'x', true, 1n, Symbol('s')]) { throws( () => new TransformStream(transformer), usingTsImpl ? transformerError : TypeError ); } - new TransformStream(null); let started = false; - const transformer = Object.assign(function transformer() {}, { - start() { - started = true; - }, - }); - if (usingTsImpl) { - throws(() => new TransformStream(transformer), transformerError); - strictEqual(started, false); - } else { - new TransformStream(transformer); - strictEqual(started, true); - } + new TransformStream( + Object.assign(function transformer() {}, { + start() { + started = true; + }, + }) + ); + strictEqual(started, true); }, }; -// DIVERGENCE: C++ converts each strategy argument as a WebIDL dictionary: -// null means a strategy with no members and every other primitive throws -// TypeError. TypeScript accepts null and every other primitive as no -// strategy, reading a primitive's members through its wrapper prototype. -// A readable strategy without a highWaterMark (null or {}) differs too: -// C++ applies the TransformStream default highWaterMark (0) only when the -// argument is omitted, so the readable side takes the ReadableStream -// default (1); TypeScript applies 0 to any strategy without one, as the -// spec's ExtractHighWaterMark does. Both read a function's members, as -// the spec does. +// DIVERGENCE: each strategy argument converts as a WebIDL dictionary in +// both (spec): a function's members are read, and every primitive except +// null throws TypeError (messages differ). A readable strategy without a +// highWaterMark (null or {}) differs: C++ applies the TransformStream +// default highWaterMark (0) only when the argument is omitted, so the +// readable side takes the ReadableStream default (1); TypeScript applies +// 0 to any strategy without one, as the spec's ExtractHighWaterMark does. export const strategyArgumentShapes = { test() { // The writable's and the readable's desiredSize at construction. @@ -211,29 +205,30 @@ export const strategyArgumentShapes = { highWaterMark: 3, }); deepStrictEqual(desiredSizes(strategy, strategy), [3, 3]); + const strategyError = usingTsImpl + ? { + name: 'TypeError', + message: 'The queuing strategy must be an object.', + } + : TypeError; for (const primitive of [5, 'x', true, 1n, Symbol('s')]) { - if (usingTsImpl) { - deepStrictEqual(desiredSizes(primitive, undefined), [1, 0]); - deepStrictEqual(desiredSizes(undefined, primitive), [1, 0]); - } else { - throws(() => desiredSizes(primitive, undefined), TypeError); - throws(() => desiredSizes(undefined, primitive), TypeError); - } + throws(() => desiredSizes(primitive, undefined), strategyError); + throws(() => desiredSizes(undefined, primitive), strategyError); } }, }; // DIVERGENCE: which error wins when two are possible. Both reject a // primitive transformer before reading the strategies, so its TypeError -// beats a throwing strategy getter (spec). C++ converts all three -// arguments before validating anything, converting each highWaterMark -// to an integer: a throwing writable strategy getter beats readableType: -// 'bytes', and an invalid writable highWaterMark beats both readableType: -// 'bytes' and a throwing readable strategy getter. TypeScript checks -// readableType before reading either strategy, and range-checks the -// writable highWaterMark before reading the readable strategy. The spec -// converts both strategies first, then checks readableType, then each -// highWaterMark's range. +// beats a throwing strategy getter (spec), and both convert the +// strategies before checking readableType, so a throwing writable +// strategy getter beats readableType: 'bytes' (spec). C++ converts each +// highWaterMark to an integer while converting the arguments, so an +// invalid writable highWaterMark (TypeError) beats readableType: 'bytes' +// and a throwing readable strategy getter. TypeScript follows the spec: +// both strategies are converted, then readableType is checked, then each +// highWaterMark's range, so readableType: 'bytes' and the readable +// strategy getter's error both beat an invalid writable highWaterMark. export const conversionErrorPrecedence = { test() { const boom = new Error('boom'); @@ -259,7 +254,7 @@ export const conversionErrorPrecedence = { ); throws( () => new TransformStream({ readableType: 'bytes' }, throwingStrategy), - usingTsImpl ? readableTypeError : (e) => e === boom + (e) => e === boom ); throws( () => @@ -268,9 +263,7 @@ export const conversionErrorPrecedence = { ); throws( () => new TransformStream({}, { highWaterMark: -1 }, throwingStrategy), - usingTsImpl - ? { name: 'RangeError', message: 'Invalid highWaterMark' } - : negativeHighWaterMark + usingTsImpl ? (e) => e === boom : negativeHighWaterMark ); }, }; diff --git a/src/tests/streams/writable/AGENTS.md b/src/tests/streams/writable/AGENTS.md index dd3add45859..ba2508af19f 100644 --- a/src/tests/streams/writable/AGENTS.md +++ b/src/tests/streams/writable/AGENTS.md @@ -35,7 +35,7 @@ the abort reason). | --- | --- | --- | --- | --- | | 1 | invalid highWaterMark at ctor | TypeError (jsg uint64 conversion messages) | RangeError "Invalid highWaterMark" (spec) | `highWaterMarkValidated` | | 2 | sink.type validation | ignored (pedantic_wpt: RangeError) | RangeError "Invalid underlying sink type" (spec) | `sinkTypeValidation` | -| 3 | argument conversion order | sink dictionary first (sink.write, hwm and its ToNumber, size) | strategy first (size, hwm and its ToNumber, sink.write); strategy-before-sink is spec, but WebIDL reads hwm and its ToNumber before size | `argumentConversionOrder` | +| 3 | argument conversion order | sink dictionary first (sink.write, hwm and its ToNumber, size) | strategy first (spec): hwm and its ToNumber, size, then sink.write | `argumentConversionOrder` | | 4 | ready at construction (no backpressure) | pending after a microtask, fulfills on a later turn | fulfilled within a microtask (spec) | `readyFulfillTiming` | | 5 | sync start() throw | captured; stream errored, writes reject | escapes the constructor (spec) | `newWritableStreamStartError` | | 6 | abort() on an errored stream | rejects with the stored error | fulfills with undefined (spec) | `newWritableStreamAbortError` | @@ -49,9 +49,9 @@ the abort reason). | 13 | releaseLock() with writes still queued | cancels them: queued writes reject with the released-writer error and their chunks are dropped (a C++ deviation — the source of its WPT piping/flow-control release-then-pipe expectedFailures) | spec: release rejects only ready/closed; queued writes stay in [[writeRequests]] and drain on the sink's schedule (a new writer can relock and they still complete — WPT flow-control pins that; behind a never-settling in-flight write they wait forever on backpressure) | `cancelWriteOnReleaseLock` | | 14 | when a promise returned by start() starts the stream | adopts it: a write queued during start reaches the sink before the first marker chained on it | spec (Node agrees): a new promise is resolved with it, so the write reaches the sink after the second marker, whether it was fulfilled on return or later | `startPromiseSettledInNewPromise` | | 15 | a write from inside sink.close() whose size() throws | rejects with size()'s error | TypeError (closing): the close in flight is checked before the erroring state (spec; Node agrees); the close completes either way | `sizeConsultedForWriteInsideSinkClose` | -| 16 | underlyingSink argument shapes | WebIDL dictionary: null means no sink, a function is a sink whose members are read; every other primitive throws TypeError | null, every other primitive and a function all throw TypeError 'underlyingSink must be an object' (spec: null and the other primitives throw, a function is accepted) | `garbageSinkValidation` | -| 17 | strategy argument shapes | WebIDL dictionary: null means no strategy; every other primitive throws TypeError | null throws TypeError (reading its size); every other primitive is accepted as no strategy. A function's members are read on both sides (spec) | `strategyArgumentShapes` | -| 18 | which error wins when two are possible | a primitive sink (TypeError) beats a throwing strategy getter; an invalid highWaterMark (TypeError, integer conversion) beats an invalid sink type, pedantic_wpt included | the strategy is read first: a throwing strategy getter beats a primitive sink, and an invalid highWaterMark (RangeError 'Invalid highWaterMark') beats an invalid sink type (spec: the primitive sink's TypeError, then the sink type's RangeError) | `conversionErrorPrecedence` | +| 16 | null underlyingSink (every other primitive throws TypeError, and a function is accepted with its members read, in both; spec) | accepted as no sink (a WebIDL dictionary) | TypeError 'underlyingSink must be an object' (spec: the argument is typed `object`) | `garbageSinkValidation` | +| 17 | (no divergence) the strategy argument converts as a WebIDL dictionary: null means no strategy, a function's members are read, and every other primitive throws TypeError (messages differ) | same | same | `strategyArgumentShapes` (parity) | +| 18 | which error wins when two are possible (a primitive sink's TypeError beats a throwing strategy getter in both; spec) | an invalid highWaterMark (TypeError, integer conversion) beats an invalid sink type, pedantic_wpt included | the sink type's RangeError ('Invalid underlying sink type') beats an invalid highWaterMark's (spec) | `conversionErrorPrecedence` | Parity worth noting (probed, pinned): the whole in-flight abort matrix — abort-before-start reason identity on ready/closed, errored-state reason diff --git a/src/tests/streams/writable/construction.js b/src/tests/streams/writable/construction.js index 29fda2c775c..a302f643c07 100644 --- a/src/tests/streams/writable/construction.js +++ b/src/tests/streams/writable/construction.js @@ -59,12 +59,10 @@ export const sinkTypeValidation = { // DIVERGENCE: the spec converts the queuingStrategy argument before // inspecting the underlying sink (the WPT constructor.any "converted -// after queuingStrategy" case). TypeScript reads strategy.size, then -// strategy.highWaterMark and its ToNumber, then sink.write. C++ converts -// the sink dictionary first. Within the strategy, WebIDL converts members -// in lexicographic order (highWaterMark and its ToNumber, then size), as -// C++ does; TypeScript reads size first, so only its strategy-before-sink -// order is spec. +// after queuingStrategy" case), its members in lexicographic order: +// highWaterMark and its ToNumber, then size. TypeScript follows it. C++ +// converts the sink dictionary first, then the strategy in the same +// order. export const argumentConversionOrder = { test() { const order = []; @@ -96,9 +94,9 @@ export const argumentConversionOrder = { new WritableStream(sink, strategy); if (usingTsImpl) { deepStrictEqual(order, [ - 'strategy.size', 'strategy.highWaterMark', 'hwm.valueOf', + 'strategy.size', 'sink.write', ]); } else { @@ -112,18 +110,22 @@ export const argumentConversionOrder = { }, }; -// DIVERGENCE: C++ converts the underlyingSink as a WebIDL dictionary: -// null means no sink, a function is a sink whose members are read, and -// every other primitive throws TypeError. TypeScript throws TypeError for -// null, every other primitive and a function alike. The spec rejects -// null and the other primitives (the argument is typed `object`) and -// accepts a function. +// DIVERGENCE: a null underlyingSink is accepted by C++, which converts it +// as a WebIDL dictionary (no sink), but rejected by TypeScript (spec: the +// argument is typed `object`, which null is not). Every other primitive +// throws TypeError, and a function is accepted and its members read, in +// both (spec). export const garbageSinkValidation = { test() { const sinkError = { name: 'TypeError', message: 'underlyingSink must be an object', }; + if (usingTsImpl) { + throws(() => new WritableStream(null), sinkError); + } else { + new WritableStream(null); + } for (const sink of [42, 'x', true, 1n, Symbol('s')]) { throws( () => new WritableStream(sink), @@ -131,46 +133,35 @@ export const garbageSinkValidation = { ); } let started = false; - const sink = Object.assign(function sink() {}, { - start() { - started = true; - }, - }); - if (usingTsImpl) { - throws(() => new WritableStream(null), sinkError); - throws(() => new WritableStream(sink), sinkError); - strictEqual(started, false); - } else { - new WritableStream(null); - new WritableStream(sink); - strictEqual(started, true); - } + new WritableStream( + Object.assign(function sink() {}, { + start() { + started = true; + }, + }) + ); + strictEqual(started, true); }, }; -// DIVERGENCE: C++ converts the strategy argument as a WebIDL dictionary: -// null means no strategy and every other primitive throws TypeError. -// TypeScript throws for null (reading its size) and accepts every other -// primitive as no strategy, reading its members through the wrapper -// prototype. Both read a function's members, as the spec does. +// The strategy argument converts as a WebIDL dictionary in both (spec): +// null means no strategy, a function's members are read, and every other +// primitive throws TypeError (messages differ). export const strategyArgumentShapes = { test() { const desiredSize = (strategy) => new WritableStream({}, strategy).getWriter().desiredSize; - const primitives = [5, 'x', true, 1n, Symbol('s')]; - if (usingTsImpl) { - throws(() => desiredSize(null), { - name: 'TypeError', - message: "Cannot read properties of null (reading 'size')", - }); - for (const strategy of primitives) { - strictEqual(desiredSize(strategy), 1); - } - } else { - strictEqual(desiredSize(null), 1); - for (const strategy of primitives) { - throws(() => desiredSize(strategy), TypeError); - } + strictEqual(desiredSize(null), 1); + for (const strategy of [5, 'x', true, 1n, Symbol('s')]) { + throws( + () => desiredSize(strategy), + usingTsImpl + ? { + name: 'TypeError', + message: 'The queuing strategy must be an object.', + } + : TypeError + ); } const strategy = Object.assign(function strategy() {}, { highWaterMark: 3, @@ -179,15 +170,13 @@ export const strategyArgumentShapes = { }, }; -// DIVERGENCE: which error wins when two are possible. C++ converts the -// sink before reading the strategy, so a primitive sink beats a throwing -// strategy getter, and converts highWaterMark to an integer before the -// sink's type is checked (pedantic_wpt included), so an invalid -// highWaterMark beats an invalid sink type. TypeScript reads the strategy -// first: the getter's error beats the primitive sink, and the -// highWaterMark RangeError beats the sink type's. The spec rejects the -// primitive sink first, and checks the sink's type before the -// highWaterMark. +// DIVERGENCE: which error wins when two are possible. Both reject a +// primitive sink before reading the strategy, so its TypeError beats a +// throwing strategy getter (spec). TypeScript checks the sink's type +// before the highWaterMark's range (spec), so an invalid sink type's +// RangeError beats an invalid highWaterMark's; C++ converts highWaterMark +// to an integer before the sink's type is checked (pedantic_wpt +// included), so its TypeError wins. export const conversionErrorPrecedence = { test() { const boom = new Error('boom'); @@ -198,12 +187,14 @@ export const conversionErrorPrecedence = { throw boom; }, }), - usingTsImpl ? (e) => e === boom : TypeError + usingTsImpl + ? { name: 'TypeError', message: 'underlyingSink must be an object' } + : TypeError ); throws( () => new WritableStream({ type: 'bytes' }, { highWaterMark: -1 }), usingTsImpl - ? { name: 'RangeError', message: 'Invalid highWaterMark' } + ? { name: 'RangeError', message: 'Invalid underlying sink type' } : { name: 'TypeError', message: From 55afb44c89c1c46ad799e3c01a5f797316aefd9f Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 13 Oct 2026 01:54:24 +0000 Subject: [PATCH 3/5] Convert stream options dictionaries as WebIDL does The options of getReader(), a BYOB reader's read(), values() and the pipe methods are WebIDL dictionaries: undefined and null mean the defaults, a function's members are read, and any other primitive throws TypeError. The TypeScript implementation rejected a function at all four, and null at read() and values(). It also rejected a function as the init of CountQueuingStrategy and ByteLengthQueuingStrategy. C++ accepts all of these. A new toDictionary() in common.ts converts the four options arguments, and toStrategyDictionary() is now built on it. The strategy constructors use requireObject() instead: their init has a required member, so undefined and null still throw. Error messages are unchanged. New parity tests pin each site with null, a function and the other primitives, in both implementations. Only the TypeScript implementation changes, and it is still behind the experimental typescript_implemented_streams flag. --- src/per_isolate/webstreams/AGENTS.md | 2 +- src/per_isolate/webstreams/common.ts | 30 ++++++++------ src/per_isolate/webstreams/readable.ts | 36 ++++++++--------- src/per_isolate/webstreams/strategies.ts | 13 +++---- src/tests/streams/piping/AGENTS.md | 2 +- src/tests/streams/piping/api-surface.js | 37 ++++++++++++++++++ src/tests/streams/piping/main.js | 1 + src/tests/streams/readable-byte/AGENTS.md | 2 +- src/tests/streams/readable-byte/main.js | 1 + src/tests/streams/readable-byte/read-min.js | 38 +++++++++++++++++- src/tests/streams/readable/AGENTS.md | 4 +- src/tests/streams/readable/api-surface.js | 30 ++++++++++++++ src/tests/streams/readable/async-iteration.js | 39 +++++++++++++++++++ src/tests/streams/readable/main.js | 2 + src/tests/streams/strategies/AGENTS.md | 2 +- src/tests/streams/strategies/construction.js | 19 +++++++++ src/tests/streams/strategies/main.js | 1 + 17 files changed, 214 insertions(+), 45 deletions(-) diff --git a/src/per_isolate/webstreams/AGENTS.md b/src/per_isolate/webstreams/AGENTS.md index 3c3591ba674..044f53fcca5 100644 --- a/src/per_isolate/webstreams/AGENTS.md +++ b/src/per_isolate/webstreams/AGENTS.md @@ -14,7 +14,7 @@ private-brand dispatch, no `instanceof`) apply here — see | ------------- | -------------------------------------------------------------------------------------------- | | `ring-buffer.ts` | O(1) FIFO with indexed access backing every internal queue below; leaf module | | `view-extent.ts` | A caller-supplied view's byte extent through captured getters, and any BufferSource's (`bufferSourceExtent`); detached/out-of-bounds inputs (DataViews included) read as empty; `copyExtent` copies an extent; leaf module | -| `common.ts` | Helpers shared by the webstreams modules, including the constructors' WebIDL argument conversion (`requireObject` for a source, sink or transformer; `convertQueuingStrategy`, then `extractHighWaterMark` and `extractSizeAlgorithm`, for a strategy) and `normalizeExpectedLength` (every `expectedLength` and `FixedLengthStream` length converts through it, as C++ converts a uint64_t); leaf module | +| `common.ts` | Helpers shared by the webstreams modules, including WebIDL argument conversion (`requireObject` for a source, sink, transformer or strategy `init`; `toDictionary` for an options dictionary; `convertQueuingStrategy`, then `extractHighWaterMark` and `extractSizeAlgorithm`, for a strategy) and `normalizeExpectedLength` (every `expectedLength` and `FixedLengthStream` length converts through it, as C++ converts a uint64_t); leaf module | | `queue.ts` | QUEUED backend: single-queue/multi-cursor, JS sources; fence interfaces; invariant list | | `native.ts` | NATIVE backend: C++-backed pull conduit; **the C++/JS contract** + invariants | | `readable.ts` | Reader layer + queued controllers + the BACKEND-DISPATCH points (constructor, tee, chains, byte-capable gate, JS-to-C++ extraction) | diff --git a/src/per_isolate/webstreams/common.ts b/src/per_isolate/webstreams/common.ts index 667f3106689..479cefba009 100644 --- a/src/per_isolate/webstreams/common.ts +++ b/src/per_isolate/webstreams/common.ts @@ -68,24 +68,30 @@ function normalizeExpectedLength(value: unknown): bigint | undefined { return BigInt(length - (length % 1)); } -// WebIDL conversion of a constructor argument typed `object` (an underlying -// source or sink, a transformer): any object, a function included, passes; -// null and every other primitive throw. Nothing is read. An omitted argument -// defaults to kNullProtoObject before it gets here. +// WebIDL's check that a value is an object, a function included: null and +// every other primitive throw. Nothing is read. It converts an argument typed +// `object` (an underlying source or sink, a transformer), and a dictionary +// with a required member, which undefined and null cannot be. function requireObject(value: unknown, message: string): void { if (!isActualObject(value) && typeof value !== 'function') { throw new TypeError(message); } } -// WebIDL dictionary conversion of a strategy argument, as in C++: undefined -// and null mean no strategy; any other non-object throws. -function toStrategyDictionary(value: unknown): QueuingStrategy { +// WebIDL conversion of an optional dictionary argument (a strategy, an +// options bag), as in C++: undefined and null mean an empty dictionary, and +// anything else must pass requireObject. +function toDictionary(value: unknown, message: string): object { if (value === undefined || value === null) return kNullProtoObject; - if (typeof value !== 'object' && typeof value !== 'function') { - throw new TypeError('The queuing strategy must be an object.'); - } - return value as QueuingStrategy; + requireObject(value, message); + return value as object; +} + +function toStrategyDictionary(value: unknown): QueuingStrategy { + return toDictionary( + value, + 'The queuing strategy must be an object.' + ) as QueuingStrategy; } // A strategy argument as WebIDL converts a QueuingStrategy dictionary: each @@ -154,6 +160,7 @@ export type CommonHelpers = { toStringTagProperty: typeof toStringTagProperty; normalizeExpectedLength: typeof normalizeExpectedLength; requireObject: typeof requireObject; + toDictionary: typeof toDictionary; toStrategyDictionary: typeof toStrategyDictionary; convertQueuingStrategy: typeof convertQueuingStrategy; convertHighWaterMark: typeof convertHighWaterMark; @@ -168,6 +175,7 @@ module.exports = ObjectFreeze({ toStringTagProperty, normalizeExpectedLength, requireObject, + toDictionary, toStrategyDictionary, convertQueuingStrategy, convertHighWaterMark, diff --git a/src/per_isolate/webstreams/readable.ts b/src/per_isolate/webstreams/readable.ts index 3a8df72242a..245e3ddb1d6 100644 --- a/src/per_isolate/webstreams/readable.ts +++ b/src/per_isolate/webstreams/readable.ts @@ -124,6 +124,7 @@ const { normalizeExpectedLength, toStringTagProperty, requireObject, + toDictionary, convertQueuingStrategy, extractHighWaterMark, extractSizeAlgorithm, @@ -1137,9 +1138,10 @@ class ReadableStreamBYOBReader implements ReadableStreamBYOBReaderType { "view's backing buffer must have a non-zero byteLength" ); } - if (!isActualObject(options)) { - throw new TypeError('options must be an object'); - } + options = toDictionary( + options, + 'options must be an object' + ) as ReadableStreamBYOBReaderReadOptions; // --- min validation (steps 4-6). min is in ELEMENTS for typed arrays // (bytes for DataView, where info.length === byteLength). NO clamping: // out-of-range is an error, not a request to do less. @@ -2966,12 +2968,10 @@ interface ConvertedPipeOptions { // Members are read once each in spec order (WPT // piping/throwing-options.any.js). function convertPipeOptions(options: unknown): ConvertedPipeOptions { - // WebIDL: null and undefined become {}. - if (options == null) options = kNullProtoObject; - if (!isActualObject(options)) { - throw new TypeError('Pipe options must be an object'); - } - const dict = options as StreamPipeOptions; + const dict = toDictionary( + options, + 'Pipe options must be an object' + ) as StreamPipeOptions; const preventAbort = !!dict.preventAbort; const preventCancel = !!dict.preventCancel; const preventClose = !!dict.preventClose; @@ -4636,15 +4636,13 @@ class ReadableStream { options: { mode?: 'byob' } | null = kNullProtoObject ): ReadableStreamReaderType { assertIsReadableStream(this); - // WebIDL dictionary conversion: null and undefined become {}, - // objects have their properties read, primitives are rejected. - if (options != null && !isActualObject(options)) { - throw new TypeError('Reader options must be an object'); - } + const dict = toDictionary(options, 'Reader options must be an object') as { + mode?: 'byob'; + }; // WebIDL: the 'mode' member is a string enum — perform ToString() on // non-undefined values so that objects with toString()/valueOf() work. - const rawMode = isActualObject(options) ? options.mode : undefined; + const rawMode = dict.mode; const mode = rawMode === undefined ? undefined : `${rawMode}`; if (mode === undefined) { return acquireReadableStreamDefaultReader(this); @@ -4742,10 +4740,10 @@ class ReadableStream { options: { preventCancel?: boolean } = kNullProtoObject ): AsyncIterableIterator { assertIsReadableStream(this); - if (!isActualObject(options)) { - throw new TypeError('Options must be an object'); - } - const { preventCancel } = options; + const { preventCancel } = toDictionary( + options, + 'Options must be an object' + ) as { preventCancel?: boolean }; if (isReadableStreamLocked(this)) { throw new TypeError('Cannot get an iterator for a stream that is locked'); diff --git a/src/per_isolate/webstreams/strategies.ts b/src/per_isolate/webstreams/strategies.ts index f291d8cb5e4..837be2847ea 100644 --- a/src/per_isolate/webstreams/strategies.ts +++ b/src/per_isolate/webstreams/strategies.ts @@ -6,7 +6,7 @@ import type { CommonHelpers } from './common'; const { ObjectDefineProperties, SymbolToStringTag, TypeError } = primordials; -const { isActualObject, kEnumerable, toStringTagProperty } = +const { isActualObject, kEnumerable, requireObject, toStringTagProperty } = require('webstreams/common') as CommonHelpers; // Per spec, the size function is the SAME function object for every @@ -44,11 +44,10 @@ class ByteLengthQueuingStrategy { } // The init type is optional-shaped because user input is arbitrary; the - // required-member check is the explicit TypeError below. + // required-member check is the explicit TypeError below. A dictionary with + // a required member cannot be converted from undefined or null. constructor(init: { highWaterMark?: number }) { - if (!isActualObject(init)) { - throw new TypeError('init must be an object'); - } + requireObject(init, 'init must be an object'); // Read once, as WebIDL dictionary conversion reads each member once. const highWaterMark = init.highWaterMark; if (highWaterMark === undefined) { @@ -82,9 +81,7 @@ class CountQueuingStrategy { } constructor(init: { highWaterMark?: number }) { - if (!isActualObject(init)) { - throw new TypeError('init must be an object'); - } + requireObject(init, 'init must be an object'); // Read once, as WebIDL dictionary conversion reads each member once. const highWaterMark = init.highWaterMark; if (highWaterMark === undefined) { diff --git a/src/tests/streams/piping/AGENTS.md b/src/tests/streams/piping/AGENTS.md index 2d5bd933c26..7471704c0d1 100644 --- a/src/tests/streams/piping/AGENTS.md +++ b/src/tests/streams/piping/AGENTS.md @@ -79,7 +79,7 @@ the source FIRST, then releasing the write (`pipeStopsPullingWhenDestStalls`). | Module | Coverage | | --- | --- | | `pipe-matrix.js` | migrated pipe-streams-test.js wholesale (35): pipeThrough + pipeTo across JS↔native in all directions, prevent* combos, pre-aborted and mid-read AbortSignals, tee'd pipes, queued-destination close (ledger #1-#4, #13) | -| `api-surface.js` | brand checks (ledger #5), option getter order, throwing getters, invalid signal, locked pipeThrough endpoints, lock safety when validation fails | +| `api-surface.js` | brand checks (ledger #5), option getter order, throwing getters, invalid signal, options as a WebIDL dictionary (parity: null, a function's members, primitives rejected), locked pipeThrough endpoints, lock safety when validation fails | | `abort-signal.js` | the pipe's AbortSignal is an abort algorithm: a synthetic 'abort' event is ignored, a listener's stopImmediatePropagation() cannot block the abort, an abort after the pipe settles does nothing | | `error-propagation.js` | forward matrix (starts-errored × prevent* × truthy), hwm-0 dest (ledger #6), custom-error preservation (migrated from streams-error-edge-cases-test.js) | | `close-propagation.js` | the WPT-disabled backward territory, bounded: external close/abort on piped dest, write-throw backward propagation, idle dest-controller error and one with a write in flight (ledger #7) | diff --git a/src/tests/streams/piping/api-surface.js b/src/tests/streams/piping/api-surface.js index 473948a543d..d7db2442478 100644 --- a/src/tests/streams/piping/api-surface.js +++ b/src/tests/streams/piping/api-surface.js @@ -75,6 +75,43 @@ export const invalidSignalRejected = { }, }; +// The pipe options convert as a WebIDL dictionary (parity): null means no +// options, so the pipe closes the destination; a function's members are +// read, here preventClose; every other primitive throws TypeError from +// pipeThrough (messages differ). +export const pipeOptionsShapes = { + async test() { + const closed = []; + const pipeClosedSource = (name, options) => + new ReadableStream({ + start(c) { + c.close(); + }, + }).pipeTo( + new WritableStream({ + close() { + closed.push(name); + }, + }), + options + ); + await pipeClosedSource('null', null); + await pipeClosedSource( + 'function', + Object.assign(function options() {}, { preventClose: true }) + ); + deepStrictEqual(closed, ['null']); + for (const options of [5, 'x', true, 1n, Symbol('s')]) { + throws( + () => new ReadableStream().pipeThrough(new TransformStream(), options), + usingTsImpl + ? { name: 'TypeError', message: 'Pipe options must be an object' } + : TypeError + ); + } + }, +}; + // Brand checks: pipeTo on a non-stream this, a non-WritableStream // destination, and pipeThrough with a bad pair. DIVERGENCE (the WPT // general.any brand seed): C++ THROWS the TypeError synchronously from diff --git a/src/tests/streams/piping/main.js b/src/tests/streams/piping/main.js index 77c451fe5be..4fec1c79ab4 100644 --- a/src/tests/streams/piping/main.js +++ b/src/tests/streams/piping/main.js @@ -46,6 +46,7 @@ export { optionGetterReadOrder, throwingOptionGetter, invalidSignalRejected, + pipeOptionsShapes, brandChecks, pipeThroughLockedEndpoints, badDestinationLeavesSourceUnlocked, diff --git a/src/tests/streams/readable-byte/AGENTS.md b/src/tests/streams/readable-byte/AGENTS.md index 47911ad6f12..ae854fbc619 100644 --- a/src/tests/streams/readable-byte/AGENTS.md +++ b/src/tests/streams/readable-byte/AGENTS.md @@ -128,7 +128,7 @@ named suite test pins directly, differing only in incidental asserts. | `byob-reader.js` | ledger #20, #28, #31; view-type matrix + offsets + auto-allocate sizing (migrated streams-byob-edge-cases) + mismatched sizes/types, subarray, multi-pending-reads, byobreaderRegression (migrated streams-js-test) | | `respond.js` | ledger #6, #8, #15, #16; all 31 streams-respond-test tests (respond/respondWithNewView/pumps/cancel races/UAF shapes) + js-test respond family | | `release-relock.js` | ledger #9, #10, #32; the WPT releaseLock→second-reader cluster; release with two pending reads or a partially filled head | -| `read-min.js` | ledger #11-#13, #27; byobMin/constraints/readAtLeast (migrated streams-test.js); /chunked SELF endpoint | +| `read-min.js` | ledger #11-#13, #27; byobMin/constraints/readAtLeast (migrated streams-test.js); read(view, options) options as a WebIDL dictionary (parity: null, a function's members, primitives rejected); /chunked SELF endpoint | | `tee.js` | ledger #7 (on a branch), #14, #24, #25, #29, #32; clone-per-branch; migrated byte-tee pair; error propagation; released branch reads, incl. partially filled ones and tee() after a release; byobRequest held across tee(); tee() of a closed native body locking it | | `buffer-lifecycle.js` | ledger #18, #30; resizable ArrayBuffers; WASM Memory; SharedArrayBuffer-backed typed arrays and DataViews rejected by read(view), readAtLeast(), enqueue() and respondWithNewView() (TS also after close(), #19) | | `gc.js` | pending BYOB read + byobRequest survive gc(); both tee branches collected while the controller is held: enqueue() accepted, desiredSize at the high-water mark, byobRequest null, close() then enqueue() as ever (a parity pin of the observable surface — the retention checks are the readable suite's, a transferred buffer leaving nothing to WeakRef), one branch cancelled and the other collected, observed in the gc()'s own job (the readable suite's teeSurvivorBranchCollected, plus byobRequest null), and pull() stops (ledger #26) | diff --git a/src/tests/streams/readable-byte/main.js b/src/tests/streams/readable-byte/main.js index 8fcf7e88f02..16d07592e49 100644 --- a/src/tests/streams/readable-byte/main.js +++ b/src/tests/streams/readable-byte/main.js @@ -162,6 +162,7 @@ export { byobMin, readMinStagedFulfillment, readMinValidation, + byobReadOptionsShapes, closeBelowMin, closedOrderAtEndOfData, minMetThenClose, diff --git a/src/tests/streams/readable-byte/read-min.js b/src/tests/streams/readable-byte/read-min.js index dbc41ef7175..47410c6d58d 100644 --- a/src/tests/streams/readable-byte/read-min.js +++ b/src/tests/streams/readable-byte/read-min.js @@ -10,7 +10,7 @@ // resolves done with an empty view. The WPT read-min.any suite stays disabled on the C++ // side for its own hangs; the TypeScript side runs it. -import { strictEqual, ok, throws, rejects } from 'node:assert'; +import { strictEqual, deepStrictEqual, ok, throws, rejects } from 'node:assert'; import { usingTsImpl } from 'which-impl'; const enc = new TextEncoder(); @@ -105,6 +105,42 @@ export const readMinValidation = { }, }; +// read(view, options) converts its options as a WebIDL dictionary +// (parity): null means no options (min 1), a function's members are read +// (here a min of 0, rejected as in readMinValidation), and every other +// primitive rejects with TypeError (messages differ). +export const byobReadOptionsShapes = { + async test() { + const { rs, controller } = byteStream(); + const reader = rs.getReader({ mode: 'byob' }); + const read = reader.read(new Uint8Array(4), null); + controller().enqueue(new Uint8Array([1, 2])); + const { value, done } = await read; + strictEqual(done, false); + deepStrictEqual([...value], [1, 2]); + await rejects( + reader.read( + new Uint8Array(4), + Object.assign(function options() {}, { min: 0 }) + ), + { + name: 'TypeError', + message: usingTsImpl + ? 'options.min must be greater than 0' + : 'Requested invalid minimum number of bytes to read (0).', + } + ); + for (const options of [5, 'x', true, 1n, Symbol('s')]) { + await rejects( + reader.read(new Uint8Array(4), options), + usingTsImpl + ? { name: 'TypeError', message: 'options must be an object' } + : TypeError + ); + } + }, +}; + // close() while a min-read holds SOME bytes (2 of 3): the read fulfills // with the partial bytes, done=false, and a subsequent read resolves // done with an empty view — the readAtLeast tail contract, parity on diff --git a/src/tests/streams/readable/AGENTS.md b/src/tests/streams/readable/AGENTS.md index 48117866dad..4eb93ae50aa 100644 --- a/src/tests/streams/readable/AGENTS.md +++ b/src/tests/streams/readable/AGENTS.md @@ -102,7 +102,7 @@ C++ implementation; `draining-reader.js` asserts both sides. | Module | Coverage | | --- | --- | -| `api-surface.js` | globals, controller not constructable, getReader modes, locked lifecycle | +| `api-surface.js` | globals, controller not constructable, getReader modes, getReader options as a WebIDL dictionary (parity: null, a function's members, primitives rejected), locked lifecycle | | `construction.js` | ledger #1-#3, #31, #32, default hwm 1 | | `source-algorithms.js` | ledger #4-#6, #23, pull serialization, rejection identity, cancel-with-pending-pull | | `controller.js` | desiredSize accounting/terminal states, error idempotence, close terminality (#17), close-drains-queue, error during a pending close (#18) | @@ -113,7 +113,7 @@ C++ implementation; `draining-reader.js` asserts both sides. | `tee.js` | migrated edge cases + error propagation + cancel composite (#11) + pull-per-read + slowest-branch backpressure + a branch leaving after the close, cancelled or errored through the interop hook (TS only), while the others have drained: a parked cancel settles with undefined and the source is not cancelled (parity) | | `tee-reentrancy.js` | the three C++ push-loop crash regressions (from api/streams/streams-test.js) | | `from.js` | 11 migrated + fromString (#12) + return validation messages + iterator protocol (next read once, done before value, gets not `has`, objects only, return() lookup; parity) + ArrayBufferView as one chunk (#27) + async-from-sync edges (#28) + sync-path schedule (#29) | -| `async-iteration.js` | 7 migrated + no-await interleavings + proto shape and class string (#13) + foreign `this` (#25) + first next() pulls synchronously, next() from that pull() (parity) + ongoing-promise interleavings (#22) | +| `async-iteration.js` | 7 migrated + values() options as a WebIDL dictionary (parity) + no-await interleavings + proto shape and class string (#13) + foreign `this` (#25) + first next() pulls synchronously, next() from that pull() (parity) + ongoing-promise interleavings (#22) | | `reentrancy.js` | enqueue/close/cancel-in-size (parity) + read-in-size (#14; guard the size() or C++ captures every later chunk) + close-in-size with chunks still to read (#30) and on a tee with a drained branch (the chunk dropped for every branch, the other branch's cancel settling; parity) | | `buffer-lifecycle.js` | chunk by reference, detach observed | | `integration-body.js` | readAll family, normalization (incl. detached and out-of-bounds views, DataViews included, SharedArrayBuffer-backed views, resizable-extent pinning), clone, cancel-then-consume, SELF round-trips | diff --git a/src/tests/streams/readable/api-surface.js b/src/tests/streams/readable/api-surface.js index dac9cba2254..855268e810b 100644 --- a/src/tests/streams/readable/api-surface.js +++ b/src/tests/streams/readable/api-surface.js @@ -5,6 +5,7 @@ // Surface shape of ReadableStream and its default reader/controller. import { strictEqual, throws } from 'node:assert'; +import { usingTsImpl } from 'which-impl'; export const readableGlobalsExist = { test() { @@ -43,6 +44,35 @@ export const defaultStreamNoByobReader = { }, }; +// getReader()'s options convert as a WebIDL dictionary (parity): null +// means no options, a function's members are read, and every other +// primitive throws TypeError (messages differ). +export const getReaderOptionsShapes = { + test() { + let modeReads = 0; + const options = Object.defineProperty(function options() {}, 'mode', { + get() { + modeReads++; + return undefined; + }, + }); + for (const opts of [null, options]) { + const rs = new ReadableStream(); + rs.getReader(opts); + strictEqual(rs.locked, true); + } + strictEqual(modeReads, 1); + for (const opts of [5, 'x', true, 1n, Symbol('s')]) { + throws( + () => new ReadableStream().getReader(opts), + usingTsImpl + ? { name: 'TypeError', message: 'Reader options must be an object' } + : TypeError + ); + } + }, +}; + // locked flips with getReader()/releaseLock(), and a locked stream // rejects a second getReader() and tee() (migrated from streams-test.js // and streams-js-test.js readableStreamReleaseLock). diff --git a/src/tests/streams/readable/async-iteration.js b/src/tests/streams/readable/async-iteration.js index 807e510612e..cd90e299d5c 100644 --- a/src/tests/streams/readable/async-iteration.js +++ b/src/tests/streams/readable/async-iteration.js @@ -133,6 +133,45 @@ export const asyncIteratorPreventCancel = { }, }; +// values()'s options convert as a WebIDL dictionary (parity): null means +// no options, so return() cancels; a function's members are read, here +// preventCancel; every other primitive throws TypeError (messages +// differ). +export const valuesOptionsShapes = { + async test() { + const cancelled = []; + const returnAfterOneChunk = async (name, options) => { + const rs = new ReadableStream({ + pull(controller) { + controller.enqueue('chunk'); + }, + cancel() { + cancelled.push(name); + }, + }); + const iterator = rs.values(options); + strictEqual((await iterator.next()).value, 'chunk'); + await iterator.return(); + return rs; + }; + await returnAfterOneChunk('null', null); + const rs = await returnAfterOneChunk( + 'function', + Object.assign(function options() {}, { preventCancel: true }) + ); + deepStrictEqual(cancelled, ['null']); + strictEqual(rs.locked, false); + for (const options of [5, 'x', true, 1n, Symbol('s')]) { + throws( + () => new ReadableStream().values(options), + usingTsImpl + ? { name: 'TypeError', message: 'Options must be an object' } + : TypeError + ); + } + }, +}; + // Test iterating over an already-closed stream // Inspired by: Deno tests/unit/streams_test.ts export const asyncIteratorOnClosedStream = { diff --git a/src/tests/streams/readable/main.js b/src/tests/streams/readable/main.js index 3880c059fb3..d13ec867281 100644 --- a/src/tests/streams/readable/main.js +++ b/src/tests/streams/readable/main.js @@ -22,6 +22,7 @@ export { controllerNotConstructable, getReaderBadModeThrows, defaultStreamNoByobReader, + getReaderOptionsShapes, lockedLifecycle, } from 'api-surface'; @@ -142,6 +143,7 @@ export { asyncIteratorReturnMethod, asyncIteratorReturnThenNext, asyncIteratorPreventCancel, + valuesOptionsShapes, asyncIteratorOnClosedStream, asyncIteratorOnErroredStream, asyncIteratorLocksStream, diff --git a/src/tests/streams/strategies/AGENTS.md b/src/tests/streams/strategies/AGENTS.md index 26948e9f3f4..a536fa9edbf 100644 --- a/src/tests/streams/strategies/AGENTS.md +++ b/src/tests/streams/strategies/AGENTS.md @@ -47,7 +47,7 @@ with `pedantic_wpt` added, pinning the absence of pedantic effects. | Module | Asserts | | --- | --- | | `api-surface.js` | branding; accessor placement + brand checks; no own instance props; hwm reflection incl. fractional; size identity (#1) and shape (#2, #3) | -| `construction.js` | init required with per-impl message (#4); missing highWaterMark (#5); unrestricted-double storage + ToNumber coercion; `init.highWaterMark` read once (parity) | +| `construction.js` | init required with per-impl message (#4); init as a WebIDL dictionary (parity: a function's members read, primitives rejected); missing highWaterMark (#5); unrestricted-double storage + ToNumber coercion; `init.highWaterMark` read once (parity) | | `size-semantics.js` | CQS constant 1; BLQS on views/buffers/DataViews with offsets, detached (0), length-tracking resizable views; plain-object byteLength reads; nullish handling (#6); shadowing getter (#6) | | `integration.js` | strategies drive RS/WS desiredSize; detached class size fn in a plain bag; fractional HWM stream truncation (#7); zero HWM | | `reentrancy.js` | plain-chunk byteLength getter runs inside the class size fn: throws propagate, re-entrant size calls work; size fns are receiver-agnostic; init-bag highWaterMark getter re-entering the constructor is safe (all parity). In-stream user-size-callback reentrancy is readable/writable-suite territory | diff --git a/src/tests/streams/strategies/construction.js b/src/tests/streams/strategies/construction.js index 48fd0f47ff5..f4c6fc7ee96 100644 --- a/src/tests/streams/strategies/construction.js +++ b/src/tests/streams/strategies/construction.js @@ -35,6 +35,25 @@ export const initDictionaryIsRequired = { }, }; +// init converts as a WebIDL dictionary (parity): a function's members are +// read, and every other primitive throws TypeError (messages differ). +export const initDictionaryShapes = { + test() { + for (const Ctor of classes) { + const init = Object.assign(function init() {}, { highWaterMark: 3 }); + strictEqual(new Ctor(init).highWaterMark, 3, Ctor.name); + for (const bad of [5, 'x', true, 1n, Symbol('s')]) { + throws( + () => new Ctor(bad), + usingTsImpl + ? { name: 'TypeError', message: 'init must be an object' } + : TypeError + ); + } + } + }, +}; + export const missingHighWaterMarkDiverges = { test() { for (const Ctor of classes) { diff --git a/src/tests/streams/strategies/main.js b/src/tests/streams/strategies/main.js index 7c0882f2b54..53e870f0a90 100644 --- a/src/tests/streams/strategies/main.js +++ b/src/tests/streams/strategies/main.js @@ -16,6 +16,7 @@ export { export { initDictionaryIsRequired, + initDictionaryShapes, missingHighWaterMarkDiverges, highWaterMarkIsUnrestrictedDouble, initHighWaterMarkReadOnce, From 5820a7a88c22b69c28ae144d42a66ce342e0f394 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 13 Oct 2026 01:57:32 +0000 Subject: [PATCH 4/5] Correct the identity chunk-types test header The header said an invalid chunk errors an IdentityTransformStream under TypeScript. In both implementations it rejects only its own write and the stream stays usable, as the file's tests assert and the suite's divergence ledger already records. --- src/tests/streams/identity/chunk-types.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/tests/streams/identity/chunk-types.js b/src/tests/streams/identity/chunk-types.js index 0ab9736fbb3..7bfd62e8c77 100644 --- a/src/tests/streams/identity/chunk-types.js +++ b/src/tests/streams/identity/chunk-types.js @@ -10,11 +10,8 @@ // returns a rejected promise here like TypeScript does, rather than its // pre-flag synchronous throw. It still emits a console warning.) // -// What happens to the stream afterward deliberately diverges and is asserted -// per implementation below: -// - C++: the stream is unaffected by the invalid chunk and remains usable. -// - TypeScript: the invalid chunk errors the stream — the writer's closed -// promise rejects and subsequent writes reject. +// An invalid chunk rejects only its own write, in both implementations: the +// stream survives and later writes still flow (ledger #6). import { strictEqual, deepStrictEqual, rejects } from 'node:assert'; import { usingTsImpl } from 'which-impl'; From 85902abbc861e366c282a0280ae8de3007e65a2e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 13 Oct 2026 13:37:09 +0000 Subject: [PATCH 5/5] Apply additional minor coding cleanups --- src/per_isolate/webstreams/common.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/per_isolate/webstreams/common.ts b/src/per_isolate/webstreams/common.ts index 479cefba009..17e0267c95a 100644 --- a/src/per_isolate/webstreams/common.ts +++ b/src/per_isolate/webstreams/common.ts @@ -82,7 +82,7 @@ function requireObject(value: unknown, message: string): void { // options bag), as in C++: undefined and null mean an empty dictionary, and // anything else must pass requireObject. function toDictionary(value: unknown, message: string): object { - if (value === undefined || value === null) return kNullProtoObject; + if (value == null) return kNullProtoObject; requireObject(value, message); return value as object; } @@ -113,7 +113,7 @@ function convertQueuingStrategy(value: unknown): QueuingStrategyDict { throw new TypeError('strategy.size must be a function'); } return { - __proto__: null, + __proto__: kNullProtoObject, highWaterMark, size, } as QueuingStrategyDict;