Skip to content

Commit

Permalink
Merge pull request #136 from MattiasBuelens/transfer-using-structured…
Browse files Browse the repository at this point in the history
…-clone

Transfer buffers passed to `ReadableStreamBYOBReader.read(view)`
  • Loading branch information
MattiasBuelens authored Jan 4, 2024
2 parents e41321b + 55cb474 commit 41516dc
Show file tree
Hide file tree
Showing 10 changed files with 111 additions and 87 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
* Added `Transformer.cancel` method, which is called when the readable side of a `TransformStream` is cancelled or when its writable side is aborted.
* Added `min` option to `ReadableStreamBYOBReader.read(view, options)`.
* Added support for `AbortSignal.reason` when aborting a pipe.
* 🚀 Buffers passed to `ReadableStreamBYOBReader.read(view)` will now be correctly [transferred](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer#transferring_arraybuffers)
if either `ArrayBuffer.prototype.transfer()` or `structuredClone()` is available. ([#136](https://github.com/MattiasBuelens/web-streams-polyfill/pull/135))
* 🐛 Prevent [warnings from Bluebird](http://bluebirdjs.com/docs/warning-explanations.html#warning-a-promise-was-created-in-a-handler-but-was-not-returned-from-it) about a promise being created within a handler but not being returned from a handler. ([#131](https://github.com/MattiasBuelens/web-streams-polyfill/pull/131))
* 🏠 Improve internal `DOMException` polyfill. ([#133](https://github.com/MattiasBuelens/web-streams-polyfill/pull/133))

Expand Down
11 changes: 4 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,15 @@ The `polyfill/es2018` and `ponyfill/es2018` variants work in any ES2018-compatib

[`WritableStreamDefaultController.signal`][ws-controller-signal] is available in all variants, but requires a global `AbortController` constructor. If necessary, consider using a polyfill such as [abortcontroller-polyfill].

[Reading with a BYOB reader][mdn-byob-read] is available in all variants, but requires `ArrayBuffer.prototype.transfer()` or `structuredClone()` to exist in order to correctly transfer the given view's buffer. If not available, then the buffer won't be transferred during the read.

## Compliance

The polyfill implements [version `4dc123a` (13 Nov 2023)][spec-snapshot] of the streams specification.

The polyfill is tested against the same [web platform tests][wpt] that are used by browsers to test their native implementations.
The polyfill aims to pass all tests, although it allows some exceptions for practical reasons:
* The `es2018` variant passes all of the tests, except for the ["bad buffers and views" tests for readable byte streams][wpt-bad-buffers].
These tests require the implementation to synchronously transfer the contents of an `ArrayBuffer`, which is not yet possible from JavaScript (although there is a [proposal][proposal-arraybuffer-transfer] to make it possible).
The reference implementation "cheats" on these tests [by making a copy instead][ref-impl-transferarraybuffer], but that is unacceptable for the polyfill's performance ([#3][issue-3]).
* The `es2018` variant passes all of the tests.
* The `es6` variant passes the same tests as the `es2018` variant, except for the [test for the prototype of `ReadableStream`'s async iterator][wpt-async-iterator-prototype].
Retrieving the correct `%AsyncIteratorPrototype%` requires using an async generator (`async function* () {}`), which is invalid syntax before ES2018.
Instead, the polyfill [creates its own version][stub-async-iterator-prototype] which is functionally equivalent to the real prototype.
Expand Down Expand Up @@ -103,12 +103,9 @@ Thanks to these people for their work on [the original polyfill][creatorrr-polyf
[rs-asynciterator]: https://streams.spec.whatwg.org/#rs-asynciterator
[ws-controller-signal]: https://streams.spec.whatwg.org/#ws-default-controller-signal
[abortcontroller-polyfill]: https://www.npmjs.com/package/abortcontroller-polyfill
[mdn-byob-read]: https://developer.mozilla.org/en-US/docs/Web/API/ReadableStreamBYOBReader/read
[spec-snapshot]: https://streams.spec.whatwg.org/commit-snapshots/4dc123a6e7f7ba89a8c6a7975b021156f39cab52/
[wpt]: https://github.com/web-platform-tests/wpt/tree/2a298b616b7c865917d7198a287310881cbfdd8d/streams
[wpt-bad-buffers]: https://github.com/web-platform-tests/wpt/blob/2a298b616b7c865917d7198a287310881cbfdd8d/streams/readable-byte-streams/bad-buffers-and-views.any.js
[proposal-arraybuffer-transfer]: https://github.com/domenic/proposal-arraybuffer-transfer
[ref-impl-transferarraybuffer]: https://github.com/whatwg/streams/blob/4dc123a6e7f7ba89a8c6a7975b021156f39cab52/reference-implementation/lib/abstract-ops/ecmascript.js#L18
[issue-3]: https://github.com/MattiasBuelens/web-streams-polyfill/issues/3
[wpt-async-iterator-prototype]: https://github.com/web-platform-tests/wpt/blob/2a298b616b7c865917d7198a287310881cbfdd8d/streams/readable-streams/async-iterator.any.js#L24
[stub-async-iterator-prototype]: https://github.com/MattiasBuelens/web-streams-polyfill/blob/v2.0.0/src/target/es5/stub/async-iterator-prototype.ts
[creatorrr-polyfill]: https://github.com/creatorrr/web-streams-polyfill
47 changes: 33 additions & 14 deletions src/lib/abstract-ops/ecmascript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@ import { reflectCall } from 'lib/helpers/webidl';
import { typeIsObject } from '../helpers/miscellaneous';
import assert from '../../stub/assert';

declare global {
interface ArrayBuffer {
readonly detached: boolean;

transfer(): ArrayBuffer;
}

function structuredClone<T>(value: T, options: { transfer: ArrayBuffer[] }): T;
}

export function CreateArrayFromList<T extends any[]>(elements: T): T {
// We use arrays to represent lists, so this is basically a no-op.
// Do a slice though just in case we happen to depend on the unique-ness.
Expand All @@ -16,24 +26,33 @@ export function CopyDataBlockBytes(dest: ArrayBuffer,
new Uint8Array(dest).set(new Uint8Array(src, srcOffset, n), destOffset);
}

// Not implemented correctly
export function TransferArrayBuffer<T extends ArrayBufferLike>(O: T): T {
return O;
}
export let TransferArrayBuffer = (O: ArrayBuffer): ArrayBuffer => {
if (typeof O.transfer === 'function') {
TransferArrayBuffer = buffer => buffer.transfer();
} else if (typeof structuredClone === 'function') {
TransferArrayBuffer = buffer => structuredClone(buffer, { transfer: [buffer] });
} else {
// Not implemented correctly
TransferArrayBuffer = buffer => buffer;
}
return TransferArrayBuffer(O);
};

// Not implemented correctly
// eslint-disable-next-line @typescript-eslint/no-unused-vars
export function CanTransferArrayBuffer(O: ArrayBufferLike): boolean {
return true;
export function CanTransferArrayBuffer(O: ArrayBuffer): boolean {
return !IsDetachedBuffer(O);
}

// Not implemented correctly
// eslint-disable-next-line @typescript-eslint/no-unused-vars
export function IsDetachedBuffer(O: ArrayBufferLike): boolean {
return false;
}
export let IsDetachedBuffer = (O: ArrayBuffer): boolean => {
if (typeof O.detached === 'boolean') {
IsDetachedBuffer = buffer => buffer.detached;
} else {
// Not implemented correctly
IsDetachedBuffer = buffer => buffer.byteLength === 0;
}
return IsDetachedBuffer(O);
};

export function ArrayBufferSlice(buffer: ArrayBufferLike, begin: number, end: number): ArrayBufferLike {
export function ArrayBufferSlice(buffer: ArrayBuffer, begin: number, end: number): ArrayBuffer {
// ArrayBuffer.prototype.slice is not available on IE10
// https://www.caniuse.com/mdn-javascript_builtins_arraybuffer_slice
if (buffer.slice) {
Expand Down
5 changes: 3 additions & 2 deletions src/lib/abstract-ops/miscellaneous.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import NumberIsNaN from '../../stub/number-isnan';
import { ArrayBufferSlice } from './ecmascript';
import type { NonShared } from '../helpers/array-buffer-view';

export function IsNonNegativeNumber(v: number): boolean {
if (typeof v !== 'number') {
Expand All @@ -17,7 +18,7 @@ export function IsNonNegativeNumber(v: number): boolean {
return true;
}

export function CloneAsUint8Array(O: ArrayBufferView): Uint8Array {
export function CloneAsUint8Array(O: NonShared<ArrayBufferView>): NonShared<Uint8Array> {
const buffer = ArrayBufferSlice(O.buffer, O.byteOffset, O.byteOffset + O.byteLength);
return new Uint8Array(buffer);
return new Uint8Array(buffer) as NonShared<Uint8Array>;
}
10 changes: 6 additions & 4 deletions src/lib/helpers/array-buffer-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ export type TypedArray =
| Int32Array
| Uint32Array
| Float32Array
| Float64Array
| BigInt64Array
| BigUint64Array;
| Float64Array;

export type NonShared<T extends ArrayBufferView> = T & {
buffer: ArrayBuffer;
}

export interface ArrayBufferViewConstructor<T extends ArrayBufferView = ArrayBufferView> {
new(buffer: ArrayBufferLike, byteOffset: number, length?: number): T;
new(buffer: ArrayBuffer, byteOffset: number, length?: number): T;

readonly prototype: T;
}
Expand Down
3 changes: 2 additions & 1 deletion src/lib/readable-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,13 @@ import { convertPipeOptions } from './validators/pipe-options';
import type { ReadableWritablePair } from './readable-stream/readable-writable-pair';
import { convertReadableWritablePair } from './validators/readable-writable-pair';
import type { ReadableStreamDefaultReaderLike, ReadableStreamLike } from './readable-stream/readable-stream-like';
import type { NonShared } from './helpers/array-buffer-view';

export type DefaultReadableStream<R = any> = ReadableStream<R> & {
_readableStreamController: ReadableStreamDefaultController<R>
};

export type ReadableByteStream = ReadableStream<Uint8Array> & {
export type ReadableByteStream = ReadableStream<NonShared<Uint8Array>> & {
_readableStreamController: ReadableByteStreamController
};

Expand Down
16 changes: 9 additions & 7 deletions src/lib/readable-stream/byob-reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import type {
ValidatedReadableStreamBYOBReaderReadOptions
} from './reader-options';
import { convertByobReadOptions } from '../validators/reader-options';
import { isDataView, type TypedArray } from '../helpers/array-buffer-view';
import { isDataView, type NonShared, type TypedArray } from '../helpers/array-buffer-view';

/**
* A result returned by {@link ReadableStreamBYOBReader.read}.
Expand All @@ -40,13 +40,15 @@ export type ReadableStreamBYOBReadResult<T extends ArrayBufferView> = {
// Abstract operations for the ReadableStream.

export function AcquireReadableStreamBYOBReader(stream: ReadableByteStream): ReadableStreamBYOBReader {
return new ReadableStreamBYOBReader(stream);
return new ReadableStreamBYOBReader(stream as ReadableStream<Uint8Array>);
}

// ReadableStream API exposed for controllers.

export function ReadableStreamAddReadIntoRequest<T extends ArrayBufferView>(stream: ReadableByteStream,
readIntoRequest: ReadIntoRequest<T>): void {
export function ReadableStreamAddReadIntoRequest<T extends NonShared<ArrayBufferView>>(
stream: ReadableByteStream,
readIntoRequest: ReadIntoRequest<T>
): void {
assert(IsReadableStreamBYOBReader(stream._reader));
assert(stream._state === 'readable' || stream._state === 'closed');

Expand Down Expand Up @@ -88,7 +90,7 @@ export function ReadableStreamHasBYOBReader(stream: ReadableByteStream): boolean

// Readers

export interface ReadIntoRequest<T extends ArrayBufferView> {
export interface ReadIntoRequest<T extends NonShared<ArrayBufferView>> {
_chunkSteps(chunk: T): void;

_closeSteps(chunk: T | undefined): void;
Expand Down Expand Up @@ -167,7 +169,7 @@ export class ReadableStreamBYOBReader {
view: T,
options?: ReadableStreamBYOBReaderReadOptions
): Promise<ReadableStreamBYOBReadResult<T>>;
read<T extends ArrayBufferView>(
read<T extends NonShared<ArrayBufferView>>(
view: T,
rawOptions: ReadableStreamBYOBReaderReadOptions | null | undefined = {}
): Promise<ReadableStreamBYOBReadResult<T>> {
Expand Down Expand Up @@ -277,7 +279,7 @@ export function IsReadableStreamBYOBReader(x: any): x is ReadableStreamBYOBReade
return x instanceof ReadableStreamBYOBReader;
}

export function ReadableStreamBYOBReaderRead<T extends ArrayBufferView>(
export function ReadableStreamBYOBReaderRead<T extends NonShared<ArrayBufferView>>(
reader: ReadableStreamBYOBReader,
view: T,
min: number,
Expand Down
Loading

0 comments on commit 41516dc

Please sign in to comment.