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

Can pipeTo() be synchronous? #1243

Open
ricea opened this issue Oct 14, 2022 · 6 comments
Open

Can pipeTo() be synchronous? #1243

ricea opened this issue Oct 14, 2022 · 6 comments

Comments

@ricea
Copy link
Collaborator

ricea commented Oct 14, 2022

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
Copy link
Member

domenic commented Oct 16, 2022

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
Copy link
Collaborator Author

ricea commented Oct 26, 2022

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

@saschanaz
Copy link
Member

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
Copy link
Collaborator Author

ricea commented Mar 16, 2023

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

@saschanaz
Copy link
Member

saschanaz commented Mar 31, 2023

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 web-platform-tests/wpt#39103

@ricea
Copy link
Collaborator Author

ricea commented Apr 3, 2023

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants