GeezerVison

Why should we be exposing this publicly? Is this a common use case? Is there a reference issue with more information?

@mscdex I think it's a much more familiar way of creating a Duplex stream for another party, instead of having to use the push/_write/_read/_destroy/_final API. I think there'd be a great benefit if more people moved in the direction of creating a stream using a symmetrical API... see #29986 for a discussion; @addaleax suggested filing a PR over there.

If we're going to make this public, I think we should be using more generic property names instead of 'client' and 'server' since it's no longer specific to tls.

@mscdex I considered exposing socket1 and socket2, which is what tls uses (the test suite uses clientSide and serverSide). I settled on additionally exposing an array called pair so that you can pick any name you want:

const [ socket1, socket2 ] = new DuplexPair().pair;

This is what TLS now does, in the patch. Do you think that's sufficient?

Some other ideas:

• make the while object iterable, so you can do e.g. const [ a, b ] = DuplexPair();
• socket1 and socket2 (what tls uses)
• upstream and downstream
• inside and outside
• a and b (for brevity when doing const { a: socket1, b: socket2 } = ...)

I think it'd be OK to expose multiple names so applications can pick one that looks most relevant.

To illustrate my thought process, I think it's important to have this be a class like Duplex or PassThrough is, because I'm playing with an idea for a SimplexPair class, which is nothing more than a PassThrough with the addition of SimplexPair#readableSide and SimplexPair#writableSide properties that are only Readable and Writable, respectively.

With this, you can do patterns that maintain encapsulation, like

function generate(){
   const stream = new SimplexPair;
   stream.write("foo");
   return stream.readableSide;
}

• socket1 and socket2 (what tls uses)

I like this because it matches port1 and port2 from new MessageChannel(). (Alternatively: stream1 and stream2.)

• make the while object iterable, so you can do e.g. const [ a, b ] = DuplexPair();

Fwiw, that’s not exclusive with any naming choices. I like it. 👍

I'd prefer something even more generic than socket1 and socket2 because this interface doesn't utilize net in any way. Maybe something as simple as side1 and side2?

Check my work on the iterable here:

function DuplexPair() {
  if (!(this instanceof DuplexPair))
    return new DuplexPair();
  const clientSide = this[0] = new DuplexSide();
  const serverSide = this[1] = new DuplexSide();
  this.length = 2;
  this[Symbol.iterator] = Array.prototype[Symbol.iterator];
  clientSide[kOtherSide] = serverSide;
  serverSide[kOtherSide] = clientSide;
}

afaict, this should behave the same as an Array, and still permit iterating over the other keys in a for..in loop.

I like side1 and side2 better than anything else, except I have this hesitation because the numbers don't line up with the array indices, and I'm slightly personally attached to serverSide/clientSide, since a lot of applications will fall into this paradigm.

Do we need the named properties at all, though?

If performance with the iterator function is a concern, you can still do

const { 0: side1, 1: side2 } = new DuplexPair;

How does this sound?

@nodejs/streams Any further thoughts on this?

CI: https://ci.nodejs.org/job/node-test-pull-request/60551/

CI: https://ci.nodejs.org/job/node-test-pull-request/60622/

CI: https://ci.nodejs.org/job/node-test-pull-request/60640/

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @mcollina.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

Landed in b32732b