Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ReadableStream.from ignores a null @@asyncIterator #46374

Conversation

Milly
Copy link
Contributor

@Milly Milly commented May 20, 2024

The proposal uses TC39 GetIterator and GetMethod within it.
GetMethod treats null as undefined.
So if @@asyncIterator is null it should be ignored and fallback to @@iterator.

This change breaks Deno v1.43.5 and Node v20.13.1.
However, Firefox 126.0 are implemented correctly.

Similarly, null @@iterator is ignored, but there is no fallback, so its expected result is throws no-iterator-found TypeError, which is indistinguishable from not-callable TypeError.

> deno eval "ReadableStream.from({ [Symbol.asyncIterator]: null, [Symbol.iterator]: () => ({ next: () => ({ done: true }) }) }).pipeTo(new WritableStream())"
error: Uncaught (in promise) TypeError: obj[SymbolAsyncIterator] is not a function
ReadableStream.from({ [Symbol.asyncIterator]: null, [Symbol.iterator]: () => ({ next: () => ({ done: true }) }) }).pipeTo(new WritableStream())
               ^
    at getIterator (ext:deno_web/06_streams.js:5105:38)
    at Function.from (ext:deno_web/06_streams.js:5207:22)
    at file:///D:/work/js/deno/tests/wpt/suite/$deno$eval:1:16

> node -e "ReadableStream.from({ [Symbol.asyncIterator]: null, [Symbol.iterator]: () => ({ next: () => ({ done: true }) }) }).pipeTo(new WritableStream())"
node:internal/webstreams/util:250
  const iterator = FunctionPrototypeCall(method, obj);
                   ^

TypeError: FunctionPrototypeCall is not a function
    at getIterator (node:internal/webstreams/util:250:20)
    at readableStreamFromIterable (node:internal/webstreams/readablestream:1279:26)
    at ReadableStream.from (node:internal/webstreams/readablestream:324:12)
    at [eval]:1:16
    at runScriptInThisContext (node:internal/vm:209:10)
    at node:internal/process/execution:118:14
    at [eval]-wrapper:6:24
    at runScript (node:internal/process/execution:101:62)
    at evalScript (node:internal/process/execution:133:3)
    at node:internal/main/eval_string:51:3

Node.js v20.13.1

The proposal uses TC39 [GetIterator][] and [GetMethod][] within it.
GetMethod treats null as undefined. So if `@@asyncIterator` is null it
should be ignored and fallback to `@@iterator`.

This change breaks Deno v1.43.5 and Node v20.13.1.
However, Firefox 126.0 are implemented correctly.

[GetIterator]: https://tc39.es/ecma262/#sec-getiterator
[GetMethod]: https://tc39.es/ecma262/#sec-getmethod
@domenic domenic merged commit 10dbaea into web-platform-tests:master May 20, 2024
19 checks passed
Milly added a commit to Milly/deno that referenced this pull request May 21, 2024
If `@@asyncIterator` is `null` or `undefined`, it should ignores and
fallback to `@@iterator`.

Tests have been PRed into WPT.
web-platform-tests/wpt#46374
Milly added a commit to Milly/deno that referenced this pull request May 21, 2024
If `@@asyncIterator` is `null` or `undefined`, it should ignores and
fallback to `@@iterator`.

Tests have been merged into WPT.
web-platform-tests/wpt#46374
bartlomieju pushed a commit to denoland/deno that referenced this pull request May 23, 2024
…tor` (#23910)

If `@@asyncIterator` is `null` or `undefined`, it should ignores and
fallback to `@@iterator`.

Tests have been merged into WPT.
web-platform-tests/wpt#46374

The proposal of `ReadableStream.from` uses TC39 [GetIterator][] and
[GetMethod][] within it.
GetMethod treats null as undefined.
So if `@@asyncIterator` is `null` it should be ignored and fallback to
`@@iterator`.

[GetIterator]: https://tc39.es/ecma262/#sec-getiterator
[GetMethod]: https://tc39.es/ecma262/#sec-getmethod

```bash
> deno eval "ReadableStream.from({ [Symbol.asyncIterator]: null, [Symbol.iterator]: () => ({ next: () => ({ done: true }) }) }).pipeTo(new WritableStream())"
error: Uncaught (in promise) TypeError: obj[SymbolAsyncIterator] is not a function
ReadableStream.from({ [Symbol.asyncIterator]: null, [Symbol.iterator]: () => ({ next: () => ({ done: true }) }) }).pipeTo(new WritableStream())
               ^
    at getIterator (ext:deno_web/06_streams.js:5105:38)
    at Function.from (ext:deno_web/06_streams.js:5207:22)
    at file:///D:/work/js/deno/tests/wpt/suite/$deno$eval:1:16
```

---------

Co-authored-by: Asher Gomez <ashersaupingomez@gmail.com>
@Milly Milly deleted the readable-stream-from-ignores-null-asynciterable branch June 7, 2024 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants