whatwg / streams

Streams Standard
https://streams.spec.whatwg.org/
Other
1.35k stars 160 forks source link

Can pipeTo() be synchronous? #1243

Open ricea opened 1 year ago

ricea commented 1 year ago

Consider the following code:

let synchronous;
new ReadableStream({
  pull(controller) {
    synchronous = true;
    controller.enqueue();
    synchronous = false;
  }
}).pipeTo(new WritableStream({
  write() {
    console.log(synchronous);
  }
});
  1. Is this allowed to print true?
  2. Should it be allowed to print true?
  3. Is an implementation which prints true web-compatible?
domenic commented 1 year ago

I discussed this a bit with Adam in person, but for the record, my opinions are:

  1. Yes; per the current spec either true or false is allowed
  2. Probably no; allowing browsers to differ in this seems too much of a sharp edge for interop. It's too easy for web developers to write code which assumes either sync or async, and only test in one browser, and get bad results in the other.
  3. Unclear.

So my vote would be for ensuring that writes do not occur synchronously, and are at least delayed by a queued microtask.

ricea commented 1 year ago

Okay. If there are no conflicting opinions then @nidhijaju or I will write a PR to prohibit synchronous behaviour.

saschanaz commented 1 year ago

Okay. If there are no conflicting opinions then @nidhijaju or I will write a PR to prohibit synchronous behaviour.

Has this happened? I found Gecko is behaving synchronously and intend to write WPT for this.

ricea commented 1 year ago

Sorry, no, it slipped off my radar. Some WPT would be great!

saschanaz commented 1 year ago

Another issue:

await Promise.resolve().then(async () => {
  let synchronous = false;
  let readable = new ReadableStream({
    pull() {
      synchronous = true;
    }
  }, { highWaterMark: 0 });
  await new Promise(setTimeout); // make sure the stream is started
  readable.pipeTo(new WritableStream());
  console.log(synchronous);
});

This logs true on Gecko/Blink but false on WebKit. Maybe it's more natural to be synchronous here as getReader().read() pulls synchronously, except I'm not a fan of synchronous JS call...

Edit: The WPT test for the enqueue issue is landed here https://github.com/web-platform-tests/wpt/pull/39103

ricea commented 1 year ago

Another issue: ... This logs true on Gecko/Blink but false on WebKit. Maybe it's more natural to be synchronous here as getReader().read() pulls synchronously, except I'm not a fan of synchronous JS call...

Previously we considered this kind of thing "acceptable variation". If I recall correctly there was a similar difference between Blink's implementation and the reference implementation.

The aim in specifying pipeTo() was to specify a set of constraints, and permit implementations to very within those constraints in order to allow for optimisation. Maybe now we want to be more strict.

Edit: The WPT test for the enqueue issue is landed here web-platform-tests/wpt#39103

Thanks!