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

Fetch streaming upload #24

Closed
yutakahirano opened this issue Jul 5, 2022 · 12 comments
Closed

Fetch streaming upload #24

yutakahirano opened this issue Jul 5, 2022 · 12 comments
Labels
from: Google Proposed, edited, or co-edited by Google. position: support topic: loading topic: networking topic: web apis Spec relates to web APIs (entry points for script) venue: WHATWG Fetch Workstream

Comments

@yutakahirano
Copy link

yutakahirano commented Jul 5, 2022

Request for position on fetch streaming upload

  • WebKittens who can provide input: @youennf

Information about the spec

Design reviews and vendor positions

Bugs tracking this feature

  • WebKit Bugzilla:
  • Radar:

Anything else we need to know

Streaming upload allows web developers to attach a ReadableStream to a Request. The web developer can pass the Request to fetch() to upload the ReadableStream.

@jakearchibald
Copy link

I'd like developers have a way to synchronously feature detect this feature. My current recommendation is:

const supportsRequestStreams = (() => {
  let duplexAccessed = false;

  const hasContentType = new Request('', {
    body: new ReadableStream(),
    method: 'POST',
    get duplex() {
      duplexAccessed = true;
      return 'half';
    },
  }).headers.has('Content-Type');

  return duplexAccessed && !hasContentType;
})();

This tests that:

  • duplex is implemented as part of RequestInit
  • body can be a ReadableStream. Otherwise, it's converted to a string and gets a text/plain content type.

Safari has already implemented streams as bodies of Request, but it rejects if this is passed to fetch().

Could Safari avoid implementing duplex until streams as request bodies are fully implemented (including the fetch parts)? Otherwise the feature detect will get messy and asynchronous.

duplex is only currently useful with this feature.

@annevk
Copy link
Contributor

annevk commented Sep 22, 2022

@jakearchibald now that Yutaka left Chrome, is there someone else picking this up? I suppose it has shipped already, but it seems that duplex should probably also be added to the Request object as per whatwg/fetch#1486.

I tend to agree with you with regards to feature detection, does WPT verify that by chance?

@jakearchibald
Copy link

I'll pick this up, and whatwg/fetch#1486

What's the right way to write a WPT for this? Something like:

try {
  await fetchWithStreamBody();
  return;
} catch (err) {
  assert(!supportsRequestStreams);
}

@annevk
Copy link
Contributor

annevk commented Sep 22, 2022

Yeah, that looks correct. I suppose we'll only need this test while engines are still implementing this so maybe add a comment in case someone wonders what's up in the future.

@jakearchibald
Copy link

PR up: web-platform-tests/wpt#36048

@othermaciej othermaciej added the from: Google Proposed, edited, or co-edited by Google. label Sep 25, 2022
@annevk
Copy link
Contributor

annevk commented Sep 26, 2022

Colleagues and I have looked at this and are supportive. If there is no further feedback from the WebKit community by October 10 I suggest we label this as "position: support".

@HazhoHuman
Copy link

Hey awesome folks;
any updates about this, what is the destiny of it?
can we safely implement it without worrying about backward compatibility if major changes happen later??

@annevk
Copy link
Contributor

annevk commented Dec 7, 2022

@HazhoHuman I'm afraid I don't understand the question. Could you provide more context?

@hawkinsw

This comment was marked as off-topic.

@hawkinsw
Copy link

hawkinsw commented Jan 3, 2023

Hello -- I am terribly sorry that my comment was unhelpful. I was attempting to respond to @jakearchibald 's awesome note above. Again, I sincerely apologize for the noise.

@hober
Copy link
Member

hober commented Mar 23, 2023

Closing as we've identified our position.

@jtbandes
Copy link

Hi, just wondering if there is another issue or bugzilla ticket tracking the implementation of this feature? I was not able to find anything in a quick search.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
from: Google Proposed, edited, or co-edited by Google. position: support topic: loading topic: networking topic: web apis Spec relates to web APIs (entry points for script) venue: WHATWG Fetch Workstream
Development

No branches or pull requests

8 participants