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

idea: Accept a iterator as a body #809

Closed
jimmywarting opened this issue Sep 14, 2018 · 8 comments
Closed

idea: Accept a iterator as a body #809

jimmywarting opened this issue Sep 14, 2018 · 8 comments
Labels
addition/proposal New features or enhancements topic: api

Comments

@jimmywarting
Copy link

jimmywarting commented Sep 14, 2018

I where just thinking, what if Response and Request where able to accept a @@asyncIterator and a @@iterator?

That is the minium you need to create a producer that don't consume lot of RAM and are able to generate lot of data.

Node.js and the the browser have different streaming api's but what they both will have in common is the @@asyncIterator that yields Uint8array chunks.

Node's streaming api can be exported to browsers.
And whatwg-streams can be polyfilled in Node.

But having to handle both in a uniformed way is tough and requires a large overhead and a dependency of the one or the other. A iterator can easily be created with neither of them and works in both the browser and Node.js

// if using Node:
// const { Request, Response } = require('node-fetch')

async function* iterator() {
  yield Uint8Array.from([97, 98, 99])
}

await new Response(iterator()).text() // abc
await new Request(iterator()).text() // abc

// With either Node-stream or whatwg-stream you would be able to do:
await new Response(whatwgStream || nodeStream).text()
await new Request(whatwgStream || nodeStream).text()
// ...since it would be able to detect the `Symbol.asyncIterator`

There are already many modules out there that already uses node streaming api but none of them can be used by window.fetch together with Node's stream api unless they are converted to a Blob or a whatwg stream first

Taking jsZip, WebTorrent and socket.io-stream as an example, they all utilize Nodes streaming api. But the fetch api can't handle them cuz it's a different streaming api (unless you are using node-fetch which is the complete opposite since they can't handle whatwg-streams)

@jakearchibald
Copy link
Collaborator

jakearchibald commented Sep 14, 2018 via email

@annevk annevk added topic: api addition/proposal New features or enhancements labels Sep 14, 2018
@jimmywarting
Copy link
Author

jimmywarting commented Sep 14, 2018

Would a constructor that takes an iterator and returns a ReadableStream of
those values fix this problem?

I guess...
There is already one node specific module (stream-from-iterator)

I guess it could be useful to have a iterator to ReadableStream constructor converter for other purposes too, but then i would wish for this to be included in the steps of creating a body in the Request/Response constructor

if body is a iterable let body be the result of converting a iterator to ReadableStream using the x constructor


But then I think we are digging a little bit into this issue also: w3c/FileAPI#40
@annevk said something about using Symbol to have have something any IDL convertible to streams

I think the main benefit of a protocol (similar to Symbol.iterator) is that a) userland objects can participate, b) we can potentially make it easier to add such functionality to future IDL classes, and c) we settle any future naming disputes.

so if all iterators had Symbol.toStream you could perhaps do something like this:

// make up some symbol name
Symbol.toStream = Symbol('toStream')

// polyfill iterator to have `Symbol.toStream`
// Not sure if i'm doing this correctly
Object.getPrototypeOf(generator())[Symbol.toStream] = function(){
  // quick and dirty not optional hack to convert iterator to ReadableStream
  // Should use ReadableStream + pull + this.next() instead...
  return new Response([...this].join('')).body
}

// sample iterator
function* generator() {
  yield 'abc'
  yield 'def'
}

var iterator = generator()
var stream = iterator[Symbol.toStream]()
new Response(stream).text().then(console.log) // abcdef

// or just simply iterator if the spec understood Symbol.toStream
new Response(iterator).text().then(console.log)

So instead of having a constructor to convert a iterable you can work with Symbol.toStream

I'm digging the Symbol approach, would like to see that on the Blob prototype too

@jimmywarting
Copy link
Author

jimmywarting commented Sep 14, 2018

ofc, this would be nice too:

new ReadableStream({
  pull: sync_or_async_iterator_or_generator
})

// Node
new Readable({
  read: sync_or_async_iterator_or_generator
})

@ThisIsMissEm
Copy link

I feel like this proposal could definitely improve interoperability between the two most popular stream implementations at the moment: Node.js streams and WHATWG streams. Arguably, we could probably use iterators though as the foundation — pass in a stream and internally the logic is to actually call Symbol.iterator to consume that stream. This would mean that the implementation code wouldn't have to care if the input is a Node.js Stream or WHATWG Stream.

@jimmywarting shouldn't it be Symbol.stream (akin to Symbol.iterator where it's .iterator not .toIterator)

@jimmywarting
Copy link
Author

jimmywarting commented Jul 20, 2019

don't mather so much now, The blob implementation never got something like Symbol.stream or Symbol.toStream, they went for blob.stream() function instead

@jimmywarting
Copy link
Author

Not the only one that thought of it.

WHATWG streams

  • We won’t be able to get rid of the existing node streams with the current ecosystem depending heavily on them (readable-stream being the 4th most downloaded npm package)
    We need interop
    • Interop idea - async iterators. Can write function with the exact same code that reads from WHATWG and Node readable streams.
    • Interop idea - pipeline function. Make sure you can use pipeline(node_stream, whatwg_stream). Build the logic of how this works into the runtime.
    • Interop idea - ask the spec to add a getAsyncIterator method and not implement whatwg streams.

ref:
(Implement window.fetch into core) nodejs/node#19393
https://docs.google.com/document/d/1tn_-0S_FG_sla81wFohi8Sc8YI5PJiTopEzSA7UaLKM/edit

@jimmywarting
Copy link
Author

close in favor of whatwg/streams#1018 ?

@annevk
Copy link
Member

annevk commented Jan 12, 2020

Sure thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: api
Development

No branches or pull requests

4 participants