Implement MessageChannel/MessagePort

[jasnell]

This is a initial sketch of a MessageChannel/MessagePort impl. Why? Thank you for asking.. two reasons:

1. Node.js compat. There are a number of node.js ecosystem modules/frameworks that use MessageChannel.
2. WinterTC conformance. MessageChannel/MessagePort are being added to the WinterTC minimum common API.

This is a work in progress. PR will remain draft until all of the design/impl questions are answered but I'm opening this to start getting review.

const { port1, port2 } = new MessageChannel();
port1.onmessage = (event) => console.log(event.data);
port2.postMessage('hello');

Or when used with JSRPC... JSRPC support deferred to later

// ...
   const { port1, port2 } = new MessageChannel();
   port1.postMessage('hello');
   port1.onmessage = console.log;
   await env.RPCThing.setup(port2);

// then on the other side...

export default {
  async setup(port) {
    port.onmessage = (event) => console.log(event.data);
    port.postMessage('world');
  }
}

There are a range of open questions here that need to be figured out before this lands:

1. Currently, when sending a port over rpc, the local version of that port is still usable. This is because jsrpc does not necessarily imply transfer semantics. So in the above jsrpc example, port2 is still usable locally and we actually have a one-to-many relationship now from port1 to port2 and port1 to the remote port on the rpc remote. That feels a bit weird.

4. I've no idea if the underlying capnp rpc mechanism here is entirely correct. Is the output gate usage correct? Do we need a registerPendingEvent in here somewhere, etc. Specifically, should both sides of the jsrpc be kept alive so long as the message ports are connected and not closed? There are a range of behavioral things we should consider here. Simply... how do we expect things to behave where there are active ports spread across the rpc connection?

5. Right now the message cloning does not support the extended types like ReadableStream and WritableStream. It probably should? But, should that also be supported if we're not cloning across a jsrpc boundary and everything is just local. Specifically, what should the cloning behavior be in the local-to-local case vs. the local-to-remote-rpc case?

6. This does not implement the transfer list at all, is there a way to do so reasonably?

This still needs a lot more tests, so the PR will remain draft until the tests are expanded and more of these questions are answered.

[github-actions]

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

[anonrig]

I think we should have a subset of MessagePort web-platform tests that validate this. If not, I'm afraid we're going to introduce yet another implementation that is not fully spec compliant.

[jasnell]

I think we should have a subset of MessagePort web-platform tests that validate this. If not, I'm afraid we're going to introduce yet another implementation that is not fully spec compliant.

That's a given. We won't be able to run the full web-platform test suite since it requires web workers in many cases. Before this is taken out of draft the tests will be greatly expanded and there will be more verifications that we are as close to spec conformance as possible.

That said, this won't be fully spec compliant out of the box. Transfer lists will not be supported and the behavior will likely be closer to what Node.js has implemented than to what browsers implement.

[kentonv]

I had started reviewing some code details but then at some point realized that unfortunately the overall design here might have a fatal flaw -- I don't think you can send a new MesasgePort across an existing MessagePort as designed, but I believe this is a requirement. See my first comment thread below... this might require a totally different design.

So... You can probably ignore the implementation comments since I suspect much of the implementation will have to change to solve this anyway...

[anonrig]

Can we put this behind an experimental flag or include the web platform tests, before landing it?

[jasnell]

See the last commit in the set for discussion of web platform tests. We don't want to block on that indefinitely as we have a customer that needs this as soon as possible and the minimum of what we need is what they need to support their use of via undici. As for putting this behind an experimental, same. We will need this to be available outside of experimental.

[anonrig]

See the last commit in the set for discussion of web platform tests. We don't want to block on that indefinitely as we have a customer that needs this as soon as possible and the minimum of what we need is what they need to support their use of via undici. As for putting this behind an experimental, same. We will need this to be available outside of experimental.

If we're going to add web platform tests, which will require a compat flag to fix any WPT bug, I'm not sure if we should land this as is.

Regardless, of WPT, I think we put it behind a compat flag. This will likely break people since this is a global variable. And any application that checks if MessagePort includes will behave differently.

[danlapid]

Does undici work now that we have this?

[jasnell]

Does undici work now that we have this?

That'll be the next validation point. We need to have the customer test it...

FWIW, undici does not actually use MessagePort directly, which is fun. It does implement it's own version of MessageEvent which checks for the existence of MessagePort when figuring out the webidl conversion to ensure that the ports property is implemented correctly, so I believe this should actually work just fine with undici... which does not ACTUALLY need it lol.

Also, keep in mind that there might be other reasons undici might not work, this is just the one we know for sure based on what the customer has asked for.

[jasnell]

If we're going to add web platform tests, which will require a compat flag to fix any WPT bug, I'm not sure if we should land this as is.

-1 on this. We can use a pedantic_wpt* flag later to fix up variances as needed. For now the customer need is going to trump that.

I will look at adding a compat flag for the global exposure.

[anonrig]

Thank you

[jasnell]

While this should otherwise be ready to land, I want to hold off just a bit until we get confirmation from the customer that it's going to meet the need they have. (I also think I might need to tweak the internal branch to get internal tests working but I'm going to hold off on that until I get confirmation on the customer question).