From e53678fd5841fe7b94f8f9e16d6521201a3d2993 Mon Sep 17 00:00:00 2001 From: Luca Casonato Date: Mon, 26 Aug 2024 12:24:27 +0200 Subject: [PATCH] Revert "feat(fetch): accept async iterables for body" (#25207) Unfortunately this caused a regression: https://github.com/denoland/deno/issues/25203. Need to do some more upstream spec work to fix this before this can be re-landed. Reverts denoland/deno#24623 --- ext/fetch/22_body.js | 13 --- ext/fetch/lib.deno_fetch.d.ts | 1 - ext/web/06_streams.js | 63 ++++++++++---- ext/webidl/00_webidl.js | 126 --------------------------- ext/webidl/internal.d.ts | 21 ----- tests/integration/node_unit_tests.rs | 1 - tests/unit/streams_test.ts | 15 +--- tests/unit_node/fetch_test.ts | 18 ---- tests/wpt/runner/expectation.json | 10 +-- 9 files changed, 51 insertions(+), 217 deletions(-) delete mode 100644 tests/unit_node/fetch_test.ts diff --git a/ext/fetch/22_body.js b/ext/fetch/22_body.js index ae5aef8acb..82f41411d8 100644 --- a/ext/fetch/22_body.js +++ b/ext/fetch/22_body.js @@ -458,8 +458,6 @@ function extractBody(object) { if (object.locked || isReadableStreamDisturbed(object)) { throw new TypeError("ReadableStream is locked or disturbed"); } - } else if (object[webidl.AsyncIterable] === webidl.AsyncIterable) { - stream = ReadableStream.from(object.open()); } if (typeof source === "string") { // WARNING: this deviates from spec (expects length to be set) @@ -477,9 +475,6 @@ function extractBody(object) { return { body, contentType }; } -webidl.converters["async iterable"] = webidl - .createAsyncIterableConverter(webidl.converters.Uint8Array); - webidl.converters["BodyInit_DOMString"] = (V, prefix, context, opts) => { // Union for (ReadableStream or Blob or ArrayBufferView or ArrayBuffer or FormData or URLSearchParams or USVString) if (ObjectPrototypeIsPrototypeOf(ReadableStreamPrototype, V)) { @@ -498,14 +493,6 @@ webidl.converters["BodyInit_DOMString"] = (V, prefix, context, opts) => { if (ArrayBufferIsView(V)) { return webidl.converters["ArrayBufferView"](V, prefix, context, opts); } - if (webidl.isAsyncIterator(V)) { - return webidl.converters["async iterable"]( - V, - prefix, - context, - opts, - ); - } } // BodyInit conversion is passed to extractBody(), which calls core.encode(). // core.encode() will UTF-8 encode strings with replacement, being equivalent to the USV normalization. diff --git a/ext/fetch/lib.deno_fetch.d.ts b/ext/fetch/lib.deno_fetch.d.ts index 3bf608cdb4..c27313903d 100644 --- a/ext/fetch/lib.deno_fetch.d.ts +++ b/ext/fetch/lib.deno_fetch.d.ts @@ -163,7 +163,6 @@ declare type BodyInit = | FormData | URLSearchParams | ReadableStream - | AsyncIterable | string; /** @category Fetch */ declare type RequestDestination = diff --git a/ext/web/06_streams.js b/ext/web/06_streams.js index 69d333c0d0..130b24783e 100644 --- a/ext/web/06_streams.js +++ b/ext/web/06_streams.js @@ -70,6 +70,7 @@ const { String, Symbol, SymbolAsyncIterator, + SymbolIterator, SymbolFor, TypeError, TypedArrayPrototypeGetBuffer, @@ -5083,6 +5084,34 @@ function initializeCountSizeFunction(globalObject) { WeakMapPrototypeSet(countSizeFunctionWeakMap, globalObject, size); } +// Ref: https://tc39.es/ecma262/#sec-getiterator +function getAsyncOrSyncIterator(obj) { + let iterator; + if (obj[SymbolAsyncIterator] != null) { + iterator = obj[SymbolAsyncIterator](); + if (!isObject(iterator)) { + throw new TypeError( + "[Symbol.asyncIterator] returned a non-object value", + ); + } + } else if (obj[SymbolIterator] != null) { + iterator = obj[SymbolIterator](); + if (!isObject(iterator)) { + throw new TypeError("[Symbol.iterator] returned a non-object value"); + } + } else { + throw new TypeError("No iterator found"); + } + if (typeof iterator.next !== "function") { + throw new TypeError("iterator.next is not a function"); + } + return iterator; +} + +function isObject(x) { + return (typeof x === "object" && x != null) || typeof x === "function"; +} + const _resourceBacking = Symbol("[[resourceBacking]]"); // This distinction exists to prevent unrefable streams being used in // regular fast streams that are unaware of refability @@ -5168,22 +5197,21 @@ class ReadableStream { } static from(asyncIterable) { - const prefix = "Failed to execute 'ReadableStream.from'"; webidl.requiredArguments( arguments.length, 1, - prefix, + "Failed to execute 'ReadableStream.from'", ); - asyncIterable = webidl.converters["async iterable"]( - asyncIterable, - prefix, - "Argument 1", - ); - const iter = asyncIterable.open(); + asyncIterable = webidl.converters.any(asyncIterable); + + const iterator = getAsyncOrSyncIterator(asyncIterable); const stream = createReadableStream(noop, async () => { // deno-lint-ignore prefer-primordials - const res = await iter.next(); + const res = await iterator.next(); + if (!isObject(res)) { + throw new TypeError("iterator.next value is not an object"); + } if (res.done) { readableStreamDefaultControllerClose(stream[_controller]); } else { @@ -5193,8 +5221,17 @@ class ReadableStream { ); } }, async (reason) => { - // deno-lint-ignore prefer-primordials - await iter.return(reason); + if (iterator.return == null) { + return undefined; + } else { + // deno-lint-ignore prefer-primordials + const res = await iterator.return(reason); + if (!isObject(res)) { + throw new TypeError("iterator.return value is not an object"); + } else { + return undefined; + } + } }, 0); return stream; } @@ -6854,10 +6891,6 @@ webidl.converters.StreamPipeOptions = webidl { key: "signal", converter: webidl.converters.AbortSignal }, ]); -webidl.converters["async iterable"] = webidl.createAsyncIterableConverter( - webidl.converters.any, -); - internals.resourceForReadableStream = resourceForReadableStream; export { diff --git a/ext/webidl/00_webidl.js b/ext/webidl/00_webidl.js index 7440e47e7b..9ea2200f33 100644 --- a/ext/webidl/00_webidl.js +++ b/ext/webidl/00_webidl.js @@ -26,7 +26,6 @@ const { Float32Array, Float64Array, FunctionPrototypeBind, - FunctionPrototypeCall, Int16Array, Int32Array, Int8Array, @@ -78,7 +77,6 @@ const { StringPrototypeToWellFormed, Symbol, SymbolIterator, - SymbolAsyncIterator, SymbolToStringTag, TypedArrayPrototypeGetBuffer, TypedArrayPrototypeGetSymbolToStringTag, @@ -921,127 +919,6 @@ function createSequenceConverter(converter) { }; } -function isAsyncIterator(obj) { - if (obj[SymbolAsyncIterator] === undefined) { - if (obj[SymbolIterator] === undefined) { - return false; - } - } - - return true; -} - -const AsyncIterable = Symbol("[[asyncIterable]]"); - -function createAsyncIterableConverter(converter) { - return function ( - V, - prefix = undefined, - context = undefined, - opts = { __proto__: null }, - ) { - if (type(V) !== "Object") { - throw makeException( - TypeError, - "can not be converted to async iterable.", - prefix, - context, - ); - } - - let isAsync = true; - let method = V[SymbolAsyncIterator]; - if (method === undefined) { - method = V[SymbolIterator]; - - if (method === undefined) { - throw makeException( - TypeError, - "is not iterable.", - prefix, - context, - ); - } - - isAsync = false; - } - - return { - value: V, - [AsyncIterable]: AsyncIterable, - open(context) { - const iter = FunctionPrototypeCall(method, V); - if (type(iter) !== "Object") { - throw new TypeError( - `${context} could not be iterated because iterator method did not return object, but ${ - type(iter) - }.`, - ); - } - - let asyncIterator = iter; - - if (!isAsync) { - asyncIterator = { - // deno-lint-ignore require-await - async next() { - // deno-lint-ignore prefer-primordials - return iter.next(); - }, - }; - } - - return { - async next() { - // deno-lint-ignore prefer-primordials - const iterResult = await asyncIterator.next(); - if (type(iterResult) !== "Object") { - throw TypeError( - `${context} failed to iterate next value because the next() method did not return an object, but ${ - type(iterResult) - }.`, - ); - } - - if (iterResult.done) { - return { done: true }; - } - - const iterValue = converter( - iterResult.value, - `${context} failed to iterate next value`, - `The value returned from the next() method`, - opts, - ); - - return { done: false, value: iterValue }; - }, - async return(reason) { - if (asyncIterator.return === undefined) { - return undefined; - } - - // deno-lint-ignore prefer-primordials - const returnPromiseResult = await asyncIterator.return(reason); - if (type(returnPromiseResult) !== "Object") { - throw TypeError( - `${context} failed to close iterator because the return() method did not return an object, but ${ - type(returnPromiseResult) - }.`, - ); - } - - return undefined; - }, - [SymbolAsyncIterator]() { - return this; - }, - }; - }, - }; - }; -} - function createRecordConverter(keyConverter, valueConverter) { return (V, prefix, context, opts) => { if (type(V) !== "Object") { @@ -1410,11 +1287,9 @@ function setlike(obj, objPrototype, readonly) { export { assertBranded, - AsyncIterable, brand, configureInterface, converters, - createAsyncIterableConverter, createBranded, createDictionaryConverter, createEnumConverter, @@ -1425,7 +1300,6 @@ export { createSequenceConverter, illegalConstructor, invokeCallbackFunction, - isAsyncIterator, makeException, mixinPairIterable, requiredArguments, diff --git a/ext/webidl/internal.d.ts b/ext/webidl/internal.d.ts index d9266f5f54..1ce45463ec 100644 --- a/ext/webidl/internal.d.ts +++ b/ext/webidl/internal.d.ts @@ -438,27 +438,6 @@ declare module "ext:deno_webidl/00_webidl.js" { opts?: any, ) => T[]; - /** - * Create a converter that converts an async iterable of the inner type. - */ - function createAsyncIterableConverter( - converter: ( - v: V, - prefix?: string, - context?: string, - opts?: any, - ) => T, - ): ( - v: any, - prefix?: string, - context?: string, - opts?: any, - ) => ConvertedAsyncIterable; - - interface ConvertedAsyncIterable extends AsyncIterableIterator { - value: V; - } - /** * Create a converter that converts a Promise of the inner type. */ diff --git a/tests/integration/node_unit_tests.rs b/tests/integration/node_unit_tests.rs index 56c111398d..dfe15a11cd 100644 --- a/tests/integration/node_unit_tests.rs +++ b/tests/integration/node_unit_tests.rs @@ -72,7 +72,6 @@ util::unit_test_factory!( dgram_test, domain_test, fs_test, - fetch_test, http_test, http2_test, inspector_test, diff --git a/tests/unit/streams_test.ts b/tests/unit/streams_test.ts index c0adbda07c..80b45e6024 100644 --- a/tests/unit/streams_test.ts +++ b/tests/unit/streams_test.ts @@ -1,10 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -import { - assertEquals, - assertRejects, - assertThrows, - fail, -} from "./test_util.ts"; +import { assertEquals, assertRejects, fail } from "./test_util.ts"; const { core, @@ -538,11 +533,3 @@ Deno.test(async function decompressionStreamInvalidGzipStillReported() { "corrupt gzip stream does not have a matching checksum", ); }); - -Deno.test(function readableStreamFromWithStringThrows() { - assertThrows( - () => ReadableStream.from("string"), - TypeError, - "Failed to execute 'ReadableStream.from': Argument 1 can not be converted to async iterable.", - ); -}); diff --git a/tests/unit_node/fetch_test.ts b/tests/unit_node/fetch_test.ts deleted file mode 100644 index 399d6052a5..0000000000 --- a/tests/unit_node/fetch_test.ts +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. - -import { assertEquals } from "@std/assert"; -import { createReadStream } from "node:fs"; - -Deno.test("fetch node stream", async () => { - const file = createReadStream("tests/testdata/assets/fixture.json"); - - const response = await fetch("http://localhost:4545/echo_server", { - method: "POST", - body: file, - }); - - assertEquals( - await response.text(), - await Deno.readTextFile("tests/testdata/assets/fixture.json"), - ); -}); diff --git a/tests/wpt/runner/expectation.json b/tests/wpt/runner/expectation.json index 68d93c2c65..9e32fd5409 100644 --- a/tests/wpt/runner/expectation.json +++ b/tests/wpt/runner/expectation.json @@ -4007,14 +4007,8 @@ "owning-type-message-port.any.worker.html": false, "owning-type.any.html": false, "owning-type.any.worker.html": false, - "from.any.html": [ - "ReadableStream.from ignores a null @@asyncIterator", - "ReadableStream.from accepts a string" - ], - "from.any.worker.html": [ - "ReadableStream.from ignores a null @@asyncIterator", - "ReadableStream.from accepts a string" - ] + "from.any.html": true, + "from.any.worker.html": true }, "transform-streams": { "backpressure.any.html": true,