Add wrapChain function

[CvX]

This PR adds support for the method chaining:

await nock('http://localhost')
  .intercept('/notes', 'GET')
  .reply(200, [
    { id: 1, title: 'hello' },
  ]);

await (new Thing()
  .prop
  .method()
  .value = 2
);

I've added this functionality in a separate wrapChain() function, because while almost fully back-compatible with the wrap() function, it has a breaking change:

Messages are passed between contexts only after you await/call then().
For example this works with wrap():

const thing = Comlink.wrap(this.port1);
Comlink.expose((f) => f(), port2);

it("works with wrap", function(done) {
  thing(done);
});

But with the new implementation, thing(done) isn't enough to actually call a function.

An example of changed wrap() test:

  it("will wrap marked parameter values, simple function", async function() {
    const thing = Comlink.wrapChain(this.port1);
    Comlink.expose(async function(f) {
-     await f();
+     return await f();
    }, this.port2);
-   await new Promise(async resolve => {
-     thing(Comlink.proxy(_ => resolve()));
-   });
+   expect(await thing(Comlink.proxy(_ => 1))).to.equal(1);
  });

If this breaking change is acceptable (with a major version bump), I can update this PR and merge those two wrap functions.

[danwenzel]

This is great!

@surma - Got time to take a look?

[CvX]

Here's the version with the breaking change to wrap() instead of introducing the wrapChain():
master...CvX:wrap-chain-breaking

[ryanto]

Woohoo! 🎉

Any thoughts on this PR? It would help us with embermap/ember-cli-fastboot-testing#61

[surma]

Hey, apologies for being so quiet. I’m currently a bit swamped, but I’ll put this on the top of my priority list for when I have a bit of downtime. Please be patient! Thank you :)

[surma]

Okay, I finally had some time to look at this.

I definitely see the value of supporting chain-style APIs, especially with their increasing popularity. However, it’s a significant code increase (1K, ~300B gzip’d) that and I don’t think I want every Comlink user to pay.

I could, however, see providing your wrapChain() (or something similar) in a separate module, similar to node-adapter.ts.

What do you think?

[CvX]

Would the breaking change version (master...CvX:wrap-chain-breaking) be out of the question?

It does preserve compatibility with the usual cases (await new Proxy() await proxy.method() proxied().then(…))

What it breaks, are only cases without then. I don’t have the data, but I guess that would affect relatively small portion of conlink users? It would require a major version bump and a clear path of upgrade though.

But if that isn’t a viable option, a separate module would do.

[surma]

As I said, my concerns are mostly about file size, not necessarily breaking changes. A separate module is definitely my preferred way forward.

[CvX]

Hm, the byte size difference between the gzipped umd/comlink.min.js on the wrap-chain-breaking branch and the master is currently 88 bytes (and with a minimal amount of code golf I was able to reduce that to 77).

I'd say a downside of a separate module is an increased API surface. You'd have to decide which comlink "flavor" to use in each case.
E.g. when you begin with a simple call await thing(), and you want to expand it later to await thing().otherThing(). You'd have to import the chain module and change the wrap() call. It adds friction (in theory, not sure how comlink-heavy real world projects are 😅)

[danwenzel]

Hi @surma - Any updates on this one?

[surma]

Apologies. I’ll get back to this soon. Chrome Dev Summit is happening and, uh, it’s stressful 😅 Please don’t give up on me and keep pinging if you think I forgot.

[danwenzel]

Hi @surma - I hope you're recovered from the Dev Summit! Just pinging you again, per your request 😃

[danwenzel]

Hey @surma - Checking in again. Any updates on this one?