Skip to content

Feature detecting streaming requests #1470

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

Closed
jakearchibald opened this issue Jul 12, 2022 · 6 comments
Closed

Feature detecting streaming requests #1470

jakearchibald opened this issue Jul 12, 2022 · 6 comments

Comments

@jakearchibald
Copy link
Collaborator

It would be nice to have a synchronous feature detect for streaming requests. This currently works with the Chrome implementation:

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.

However, it's possible that an implementation could support these two things, but reject when that request is passed to fetch() because it doesn't support request streams. In fact, Safari already behaves like this, but it doesn't implement duplex so the test still works.

Is it reasonable to expect the above feature detect to mean "fetch supports streaming requests"? Should this be enforced with spec text and/or a wpt?

@yutakahirano
Copy link
Member

Does Request.prototype.hasOwnProperty('body') work?

@jakearchibald
Copy link
Collaborator Author

Nah, that's true in Safari.

The main problem here is that Safari supports requests with stream bodies, but it rejects when they're passed to fetch. Here was the old feature detect I used to work around the Safari issue:

const supportsRequestStreamsP = (async () => {
  const supportsStreamsInRequestObjects = !new Request('', {
    body: new ReadableStream(),
    method: 'POST',
  }).headers.has('Content-Type');

  if (!supportsStreamsInRequestObjects) return false;

  return fetch('data:a/a;charset=utf-8,', {
    method: 'POST',
    body: new ReadableStream(),
  }).then(() => true, () => false);
})();

// Note: supportsRequestStreamsP is a promise.
if (await supportsRequestStreamsP) {
  // …
} else {
  // …
}

…but I'm hoping we won't have to do that now we have duplex.

@yutakahirano
Copy link
Member

@jakearchibald
Copy link
Collaborator Author

Done! WebKit/standards-positions#24

@yutakahirano
Copy link
Member

Thanks!

@annevk
Copy link
Member

annevk commented Feb 22, 2023

I'm going to close this as there is test coverage for this. But let me explicitly copy @youennf here so he knows to watch out for this when WebKit gets around to the feature.

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

No branches or pull requests

3 participants