Maintain backwards compatibility with wire format from 4.3.0

[Nemikolh]

Hi 👋

This PR adds backwards compatibility with the wire format from 4.3.0 (or lower). This is something that we need at StackBlitz in order to keep using the upstream version of comlink (which we would love to! ❤️ ).

Why?

Both Messages and WireValues have a type property. Until [email protected] those values used to overlap. In version 4.3.1, those values were changed to strings.

At StackBlitz, we have limited control over one kind of apps (which might be forever locked with [email protected]) that consume our comlink interface. When they consume this interface they initiate the communication by doing a Comlink.wrap eventually followed by method calls, sending clone-able data or MessagePorts. Those MessagePorts are eventually Comlink.wraped on the other end.

This PR propose changes to make those communication use cases working with at least 4.3.0 (maybe other older versions might work as well but it hasn't been tested and I haven't added test for them in this PR).

How?

Let's look at an example. Supposing we have foo.com that does:

import * as Comlink from '[email protected]';

const iframe = createIframe('bar.com');
const ep = Comlink.windowEndpoint(iframe.contentWindow);
const a = Comlink.wrap(ep);

And on bar.com, an API is exposed:

import * as Comlink from 'comlink@latest';

const api = {
     ...
};

Comlink.expose(api, Comlink.windowEndpoint(self.parent));

When we then do on foo.com:

a.foo();

This will essentially do this:

// Will trigger:
postMessage({ id: '<uuid>', type: 2 /* APPLY */, ... }) 

// The other end see that it's using the legacy format
const isLegacy = typeof type === 'number'

// It then match in the switch case for the correct value
switch (type) {

    ...
   case LegacyMessageType.APPLY: <-- Match!

}

// It make the function call and then serialize back to the legacy format:
const [wireValue, transferables] = toWireValue(returnValue, isLegacy);

// And finally sends back the response:
postMessage({ id, type: 0 /* RAW */, value })

Proxy (and more generally MessagePorts) are supported too:

a.registerCallback(Comlink.proxy(() => console.log('hello world!'));

For proxy the flow is similar but we "mark" port as using the legacy format:

markLegacyPort(value.value);

// When the proxy is deserialized it eventually calls `createProxy` which check 
// whether or not this port was marked:
const isLegacy = legacyEndpoints.has(ep);

EDIT: Updated description to reflect the current state of the PR (see edit history to see the description matching the earlier commits)

[MrMadClown]

This would enable communication between 4.3.0 and the new latest.

But latest communicating with 4.3.1/4.4.0/4.4.1 would not work.

Wouldn't keeping the strings, and adding an additional mapping for the integers be better? Since it would then support all versions, at least with one way communication.

[Nemikolh]

@MrMadClown Very good point!

So to summarize your proposal, we would allow this type of communication:

// 4.3.0                         ------------------>           latest
const a = Comlink.wrap(port)                            Comlink.expose(stuff, port)

// For instance this
a.foo()

// Will trigger:
postMessage({ id: '<uuid>', type: 2 /* APPLY */, ... }) 

// The other end, notice that type is an integer, maps it to a string (or just go through the switch case)
// and make sure to return a wire value using an integer instead of using a string, so:
postMessage({ id, type: 0 /* RAW */, value })

// Need to do something for:
a.bar(Comlink.proxy(() => console.log('hello world !'));

All good. And this wouldn't work:

// 4.3.0                     <-----------------           latest
Comlink.expose(foo, port)                         const b = Comlink.wrap(port)

// For instance this
b.foo()

// Will trigger:
postMessage({ id: '<uuid>', type: "APPLY", ... }) 

// "APPLY" does not match any possible values, so it break

Communication with 4.3.1, 4.4.0 and 4.4.1 would be unmodified. @d3lm I think this should be enough for our us. Will update this PR with a proposed solution.

[Nemikolh]

Note that this code does not care whether instances of Test are exposed to an endpoint that need to talk 4.3.0 format or the latest format.

The trick is that when port is being received here, we've "marked" it so that if any code path tries to wrap it, the code will automatically work.

If that trick does not work because the port is nested inside an object, you can always resort to passing Comlink.wrap(port, undefined, true) manually which would force the proxy created to use the 4.3.0 format.

[d3lm]

Would it make sense to call the V430* stuff maybe Legacy*? I am not sure if legacy is really better or if it's better to be more specific.

[Nemikolh]

This is provided for cases where prior knowledge has not been obtained. For instance if instead of doing this:

a.foo(Comlink.transfer(port, [port])

The caller does:

a.foo(Comlink.transfer({ prop: port }, [port]))

In this case if inside foo we do:

foo({ prop: port }) {
  const remote = Comlink.wrap(port);
  // Oopsie this will break! It won't use the legacy format.
  remote.oopsie();
}

In this scenario because we don't traverse the object { prop: port } we don't mark port as legacy. We could traverse them but this might have a huge overhead and so I opted for not doing it.

Instead the implementer can do:

foo({ prop: port }) {
  const remote = Comlink.wrap(port, undefined, true);
  // Ok fine this works :)
  remote.oopsie();
}

[Nemikolh]

If we want to add the traversal then only markLegacyPort needs to be updated

[d3lm]

Is there a chance we can move forward with this PR and get a review on this? It'd be really nice if we could maintain backwards compatibility to some degree.

[d3lm]

I don’t mean to nag, but it'd be really nice if we could move forward with this somehow because it's pretty important for us. Is there anything else we can provide to get this moving?

[benjamind]

I don't have much time right now to look deeply into this I'm afraid.

I am hesitant to move ahead with legacy protocol support without some plan in place for when legacy support will be removed. It's quite a bit of complication and maintaining that going forward seems more hassle than it's worth for a library that isn't in major active development.

@surma your thoughts?

[defunctzombie]

My understanding from using this library is that the wire protocol is an internal implementation detail and not something that's part of the public interface.

[Nemikolh]

@defunctzombie It might seems like it because it isn't exposed in the public TypeScript API of this library. However the endpoint used (whether it's a MessageChannel, a window or self) only speaks the wire format. Making it the only public API from a practical perspective.

For instance you could have two esm modules that could talk to another as long as they use the same wire format. One of them could explicitely use comlink while the other would only send and parse manually crafted messages (so it would literally not import comlink at all).

The goal for this PR was to partially revert a breaking change in the wire format that was introduced as part of the 4.3.1 patch such that communication would be possible again at least in one direction (from 4.3.0 to newer versions).

If you want to convince yourself, here's a playground:

https://stackblitz.com/edit/node-zofu4b?file=index.html

[defunctzombie]

For instance you could have two esm modules that could talk to another as long as they use the same wire format. One of them could explicitely use comlink while the other would only send and parse manually crafted messages (so it would literally not import comlink at all).

Sure - but I'm wondering if thats outside the goals of the project. This is moving into the realm of things like protobuf and similar. I am not speaking for the project maintainers - only as a user myself. I had imagined it was more focused on providing a nicer RPC interface for the workers it spawns - which also come from the same release of the application.

I can see your point though that if you assemble an application from a few components, two components might use different versions of comlink and the ask is that within a major series they are compatible with each other. That's a reasonable ask if the maintainers consider that the wire interface is something that is semi-public in this manner. It would be equally reasonable to say only the JS/TS interface is public and everything else is internal implementation details that work only with the exact same version talking to the same version.

We can see what the maintainers say about their initial intent with any wire interface.