Re: [streams] Making the "queue manipulator" its own class (#309)

OK, so for those watching by email, I accidentally pressed enter on an incomplete version of this issue. I edited the OP to be complete. I'll re-post the complete OP in this comment so you get an email with all that content, then I'll delete this comment:

---

To deal with #301 per the plan in https://github.com/whatwg/streams/issues/301#issuecomment-83939205, where in the current set of functions (enqueue, close, error) grows an additional getter "spaceLeftGetter", I'm planning a bit of a change the signature and wiring. Would love thoughts.

The naive extension is of course something like (enqueue, close, error, getSpaceLeft). But at this point the argument list is starting to feel overloaded. I think there's a better solution.

The basic idea is: Instead of creating separate functions stream@[[enqueue]], stream@[[close]], and stream@[[error]] for each stream (plus a stream@[[getSpaceLeft]]), we create a _class_, called `ReadableStreamQueueManipulator`, which has `enqueue`, `close`, and `error` methods (plus a `spaceLeft` getter). We then create an instance of this class when creating the stream, and hand it out in calls to the underlying source's `start` and `pull` methods.

This has a number of advantages:

- Saves on memory, as we don't need to create four separate closures per class. Instead there's just the three methods/one getter, shared on the prototype of `ReadableStreamQueueManipulator`.
- Makes the signature a bit less crazy. Instead of writing stuff like

  ```js
  const rs = new ReadableStream({
    start(enqueue, close, error, getSpaceLeft) {
      enqueue('a');
      enqueue('b');

      if (getSpaceLeft() > 0) {
        enqueue('c');
      }

      setTimeout(close, 100);
    }
  });
  ```

  we can do

  ```js
  const rs = new ReadableStream({
    start(queue) {
      queue.enqueue('a');
      queue.enqueue('b');

      if (queue.spaceLeft > 0) {
        queue.enqueue('c');
      }

      setTimeout(() => queue.close(), 100);
    }
  });
  ```

- Better allows for future extensibility. (Even if we never add new methods or properties to `ReadableStreamQueueManipulator`, any other stream classes that want to follow this pattern would probably benefit from their interface also being a single queue manipulator object with custom methods. Maybe `ReadableByteStream` would benefit, indeed!)

It also has a couple drawbacks:

- A bit more typing. `setTimeout(close, 100)` vs. `setTimeout(() => queue.close(), 100)`. Or `ws.onend = close` vs. `ws.onend = queue.close.bind(queue)`.
- They're now methods, instead of closures, which is less convenient. `ws.onend = queue.close` will fail. As will

  ```js
  const rs = new ReadableStream({
    start({ enqueue, close }) {
      // oops, destructuring un-bound the methods, so enqueue() will fail.
    }
  });
  ```

It invites a few naming decisions:

- I named it `ReadableStreamQueueManipulator` because it exposes an interface for manipulating the stream's queue. You could also try to conceptualize it as the queue itself---and I think that's what most authors will do, as I illustrate in the naming of my variables in the above sample code. But at least the current spec doesn't give a very clean separation. `enqueue`/`close`/`error` manipulate a lot of state that belongs to the stream, not to any queue. I think `ReadableStreamQueueManipulator` is fine for now (especially since it's barely author-exposed)...
- On the other hand, you don't really "close" or "error" a queue. You do that to the stream. Maybe it's just `ReadableStreamManipulator`? `ReadableStreamController`?
- We _could_ pun on the writable stream interface with names like `write`/`close`/`abort` instead of `enqueue`/`close`/`error`. However, I think this is probably not a good idea, because of all the stuff @tyoshino explored in #287. The readable stream's queue is actually quite different from a writable stream, e.g. there's no notion of an enqueue succeeding (whereas a write might succeed or fail, depending on the hardware/network/etc.); all operations are sync instead of async; etc.



---
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/issues/309#issuecomment-87852335