Re: [whatwg/streams] Add an example of a transform stream that replaces template tags (#846)

domenic commented on this pull request.

I love it :D. I think the tests are fine here; it's an integration test. Maybe a comment at the top to that effect would be good.

> @@ -4572,6 +4572,67 @@ and similarly, closing or aborting the <code>writable</code> side will implicitl
 
 </div>
 
+<h3 id="example-ts-lipfuzz">A transform stream that replaces template tags</h3>
+
+It's often useful to substitute tags with variables on a stream of data, where the parts that need to be replaced are
+small compared to the overall data size. This example presents a simple way to do that. It maps strings to strings,
+replacing a template like <code>"Time: <span>{</span>{time}} Message: <span>{</span>{message}}"</code> with <code>"Time:

Why are these spans here?

> @@ -4572,6 +4572,67 @@ and similarly, closing or aborting the <code>writable</code> side will implicitl
 
 </div>
 
+<h3 id="example-ts-lipfuzz">A transform stream that replaces template tags</h3>
+
+It's often useful to substitute tags with variables on a stream of data, where the parts that need to be replaced are
+small compared to the overall data size. This example presents a simple way to do that. It maps strings to strings,
+replacing a template like <code>"Time: <span>{</span>{time}} Message: <span>{</span>{message}}"</code> with <code>"Time:

"replacing a template" doesn't seem like the right verb. "transforming"? "interpolating"?

> @@ -4572,6 +4572,67 @@ and similarly, closing or aborting the <code>writable</code> side will implicitl
 
 </div>
 
+<h3 id="example-ts-lipfuzz">A transform stream that replaces template tags</h3>
+
+It's often useful to substitute tags with variables on a stream of data, where the parts that need to be replaced are
+small compared to the overall data size. This example presents a simple way to do that. It maps strings to strings,
+replacing a template like <code>"Time: <span>{</span>{time}} Message: <span>{</span>{message}}"</code> with <code>"Time:
+15:36 Message: hello"</code> assuming that <code>{ time: "15:36", message: "hello" }</code> was passed in the
+<code>substitutions</code> parameter to <code>LipFuzzTransform(substitutions)</code>.
+
+This example also demonstrates one way to deal with a situation where a chunk contains partial data that cannot be
+transformed until more data is received. In this case, a partial template tag will be accumulated in the
+<code>partialChunk</code> variable until either the end of the tag is found or the end of the stream is reached.
+
+<pre><code class="lang-javascript">
+  function LipFuzzTransform(substitutions) {

I wonder if we should demonstrate using a class for the transformer here, and storing the partialChunk as an instance variable? Then replaceFunc could also be a method, although it'd need to be .bind()ed unfortunately.

> +        function replaceFunc(match, p1, offset) {
+          let replacement = substitutions[p1];
+          if (replacement === undefined) {
+            replacement = "";
+          }
+          lastIndex = offset + replacement.length;
+          return replacement;
+        }
+        chunk = chunk.replace(/\{\{([a-zA-Z0-9_-]+)\}\}/g, replaceFunc);
+        // Regular expression for an incomplete template at the end of a string.
+        const partialAtEndRegexp = /\{(\{([a-zA-Z0-9_-]+(\})?)?)?$/g;
+        // Avoid looking at any characters that have already been substituted.
+        partialAtEndRegexp.lastIndex = lastIndex;
+        const match = partialAtEndRegexp.exec(chunk);
+        if (match) {
+          partialChunk = chunk.substr(match.index);

substr() is "Annex B" (kinda-ish deprecated); stick to substring(), which in these cases will be equivalent I believe.

> @@ -4572,6 +4572,67 @@ and similarly, closing or aborting the <code>writable</code> side will implicitl
 
 </div>
 
+<h3 id="example-ts-lipfuzz">A transform stream that replaces template tags</h3>
+
+It's often useful to substitute tags with variables on a stream of data, where the parts that need to be replaced are
+small compared to the overall data size. This example presents a simple way to do that. It maps strings to strings,
+replacing a template like <code>"Time: <span>{</span>{time}} Message: <span>{</span>{message}}"</code> with <code>"Time:
+15:36 Message: hello"</code> assuming that <code>{ time: "15:36", message: "hello" }</code> was passed in the
+<code>substitutions</code> parameter to <code>LipFuzzTransform(substitutions)</code>.
+
+This example also demonstrates one way to deal with a situation where a chunk contains partial data that cannot be
+transformed until more data is received. In this case, a partial template tag will be accumulated in the
+<code>partialChunk</code> variable until either the end of the tag is found or the end of the stream is reached.
+
+<pre><code class="lang-javascript">
+  function LipFuzzTransform(substitutions) {

Functions should be lowercase if they're not classes. Also, great name (it took me a minute :)

> +        controller.enqueue(chunk);
+      },
+
+      flush(controller) {
+        if (partialChunk.length > 0) {
+          controller.enqueue(partialChunk);
+        }
+      }
+    });
+  }
+</code></pre>
+
+This would be used in code like:
+<pre><code class="lang-javascript">
+  fetchEvent.respondWith(new Response(fetch(url).then(response =>
+    response.body.pipeThrough(LipFuzzTransform({ userName, displayName, icon, date })))));

Give this code some room!! Also I don't think you can pass a promise to Response(). Maybe

```js
const data = { userName, displayName, icon, date };
const ts = lipFuzzTransform(data);

fetchEvent.respondWith(
  fetch(url).then(response => {
    const transformedBody = response.body.pipeThrough(ts);
    return new Response(transformedBody);
  )
);
```

> +      if (match) {
+        partialChunk = chunk.substr(match.index);
+        chunk = chunk.substr(0, match.index);
+      }
+      controller.enqueue(chunk);
+    },
+
+    flush(controller) {
+      if (partialChunk.length > 0) {
+        controller.enqueue(partialChunk);
+      }
+    }
+  });
+}
+
+const cases = [

This is fun :D

> +      flush(controller) {
+        if (partialChunk.length > 0) {
+          controller.enqueue(partialChunk);
+        }
+      }
+    });
+  }
+</code></pre>
+
+This would be used in code like:
+<pre><code class="lang-javascript">
+  fetchEvent.respondWith(new Response(fetch(url).then(response =>
+    response.body.pipeThrough(LipFuzzTransform({ userName, displayName, icon, date })))));
+</code></pre>
+
+<div class="note">For simplicity, <code>LipFuzzTransform()</code> performs unescaped text substitutions. In real

Maybe class="warning"

-- 
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/846#pullrequestreview-70964443

Received on Friday, 20 October 2017 21:14:49 UTC