Making the "queue manipulator" its own class

[domenic]

To deal with #301 per the plan in #301 (comment), 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

  const rs = new ReadableStream({
    start(enqueue, close, error, getSpaceLeft) {
      enqueue('a');
      enqueue('b');
  
      if (getSpaceLeft() > 0) {
        enqueue('c');
      }
  
      setTimeout(close, 100);
    }
  });

  we can do

  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

  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 Prototyping several changes for better support for reading bytes #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.

[tyoshino]

Addressed by #310. The class is given a name of 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, ...

Agreed