Re: [whatwg/streams] Changes to Example 8.9: Fix code issue, add const, add text decode step (#924)

ricea commented on this pull request.

Thanks for spotting the problems and providing fixes.

> @@ -5692,14 +5692,27 @@ useful when there is instance data to track.
 The class would be used in code like:
 
 <pre><code class="lang-javascript">
+  function textDecoderStream() {

I would much rather use the TextDecoderStream class that is planned to be added to the encoding standard (https://github.com/whatwg/encoding/issues/72), even though it isn't available yet. It avoids spending a lot of space on this mostly-unrelated topic, and avoids data corruption when characters are split between chunk boundaries.

>    const data = { userName, displayName, icon, date };
   const ts = new TransformStream(new LipFuzzTransformer(data));
+  const { url } = fetchEvent.request;

Might be better not to have a `url` intermediate value, but just extract the value directly in the call to fetch().

>  
   fetchEvent.respondWith(
     fetch(url).then(response => {
-      const transformedBody = response.body.pipeThrough(ts);
+      // Decode the binary-encoded response to string
+      const decodedBody = response.body.pipeThrough(textDecoderStream());

It's better to not create a `decodedBody` variable, but just chain the pipes like this

```javascript
const transformedBody = response.body
  .pipeThrough(new TextDecoderStream())
  .pipeThrough(ts);
```
I don't think it's necessary to comment each operation separately, but you can if you think it is clearer.


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/924#pullrequestreview-118327884

Received on Tuesday, 8 May 2018 11:52:26 UTC