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

Different behaviour from native Chrome ReadableStream #10

Open
t-mullen opened this issue Dec 17, 2017 · 2 comments
Open

Different behaviour from native Chrome ReadableStream #10

t-mullen opened this issue Dec 17, 2017 · 2 comments

Comments

@t-mullen
Copy link

t-mullen commented Dec 17, 2017

The ReadableStream polyfill has different behaviour from Chrome's native ReadableStream when used as the first argument in a Response object from a ServiceWorker. The polyfill doesn't seem to work at all.

I'm working on a minimal way to reproduce the issue, but is there any obvious differences between the APIs that might explain this?

@t-mullen t-mullen changed the title Different behaviour from native Chrome ReadableStream in SW Different behaviour from native Chrome ReadableStream Dec 17, 2017
@t-mullen
Copy link
Author

t-mullen commented Dec 17, 2017

It's not restricted to serviceworkers. The polyfill isn't working in the simplest Response case:

<html>
<head>
    <meta charset="UTF-8">
    <meta name="viewport"
          content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0">
    <meta http-equiv="X-UA-Compatible" content="ie=edge">
    <title>Simple Stream Pump</title>
</head>
<body>
<h1>Simple Stream Pump</h1>
<p>The left tortoise is the original, the right one is created using a custom stream.</p>
<img src="tortoise.png" width="150" height="84" alt="Tortoise Original">
<img src="tortoise.png" width="150" height="84" alt="Tortoise Copy" id="target">
<script src="polyfill.min.js"></script>
<script>
  const image = document.getElementById('target');

  // Fetch the original image
  fetch('./tortoise.png')
  // Retrieve its body as ReadableStream
  .then(response => {
    const reader = response.body.getReader();

    return new ReadableStream({
      start(controller) {
        return pump();

        function pump() {
          return reader.read().then(({ done, value }) => {
            // When no more data needs to be consumed, close the stream
            if (done) {
              controller.close();
              return;
            }

            // Enqueue the next data chunk into our target stream
            controller.enqueue(value);
            return pump();
          });
        }
      }
    })
  })
  .then(stream => new Response(stream))
  .then(response => response.blob())
  .then(blob => URL.createObjectURL(blob))
  .then(url => console.log(image.src = url))
  .catch(err => console.error(err));
</script>
</body>
</html>

@MattiasBuelens
Copy link

MattiasBuelens commented Dec 26, 2017

This is because of this line in the polyfill. Even though Chrome has native support for ReadableStream, the polyfill always replaces the global ReadableStream with its own version. However, Chrome's native Response class does not know what to do with this polyfilled readable stream instance.

It's quite annoying to deal with polyfilled and native stream implementations. Chrome supports ReadableStream and WritableStream, but not yet TransformStream. Edge also supports ReadableStream and WritableStream, but their ReadableStream implementation is incomplete (it's not constructable and it's missing pipeTo and pipeThrough).

I feel like the polyfill should have a better interop story for native streams. Perhaps it could have a non-standard ReadableStream.toNative(stream) function that constructs a native ReadableStream (if it exists) from a (potentionally) polyfilled one?

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

No branches or pull requests

2 participants