Update for comlink 4.x

[wellcaffeinated]

I've updated this for 4.x

I've also modified the api very slightly to make a bit more sense.

All the tests pass but more should be added.

README updated to reflect the new way of importing.

If anyone wants to use this before it gets merged, I created a "dist" branch for that purpose.

yarn add https://github.com/wellcaffeinated/comlink-loader#dist

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[wellcaffeinated]

(I signed the CLA)

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[googlebot]

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

[developit]

This PR appears to change the behavior to always allow multi-instancing. ?multi was not the default because it breaks export parity, which makes typings harder and this model more difficult to understand.

[wellcaffeinated]

Not quite. I did change the api slightly, partly because it didn't make sense to me to return a singleton when using the keyword new. But mostly because i didn't think the old api could do what I wanted to create multiple instances of workers. (I don't understand how import { MyClass } from 'comlink-loader!./my-worker would give you the class if the loader returns a function f()...). Looking at it again... it may be able to do what i want...?

Anyways this is what i came up with, that lets you use singletons and create new workers at the same time.

New api example

// my-worker.js

export class MyClass {
  constructor(){ ... }
}

export function helper(){ ... }

// consumer
import factory from 'comlink-loader!my-worker'
// singleton
let worker = factory()
let myClass = await new worker.MyClass()
let sameWorker = factory() // === instance

// run helper function
worker.helper()

// a different consumer
import factory from 'comlink-loader!my-worker'
let worker = factory() // same instance as before

// create multiple threads
Promise.map(Array.from('1234'), ( id ) => {
  // using the "new" keyword here to create new threads for this consumer
  let thread = new factory()

  // run the helpers... each runs on a parallel thread
  return thread.helper()
}).then( results => {
  // do something with results
})

[developit]

Can't ship this with a github dependency tbh.

[wellcaffeinated]

No problem. The reason for that fork was to get around a WASM bug in worker-loader. It's still not merged but I can change it in the PR

[developit]

@wellcaffeinated your example is how ?multi worked. new is just sugar, the function gets invoked and its return value is the result of the new expression if there is a return value.

I'd love to merge pieces of this PR! Just not sure I can merge the worker-loader fork or API change. The reason the API was set up this way was to achieve type parity for the simple (singleton) use-case.

One option would be to invert the boolean value. Instead of:

import { Foo } from 'comlink-loader!foo';
new Foo();    // implicitly creates worker on first use
import createFooWorker from 'comlink-loader?multi!foo';
new (createFooWorker().Foo)()

The API could be inverted:

import { Foo } from 'comlink-loader?singleton!foo';
new Foo();  // implicitly creates worker on first use
import createWorker from 'comlink-loader!foo';
new (createWorker().Foo)();

Thoughts? The latter would be very similar to what you have here, but retains the ability to do optional workers and consistent types.

[wellcaffeinated]

I guess I don't really understand the concern about "types". Nor do I understand the motivation behind a new returning a singleton. Isn't that counter-intuitive?

I mean, this isn't my repo, so you can implement it how you like, but since it's an update to a different major version of comlink, which itself has a different API, if there's a time to update comlink-loader's api, this would be it.

also, I hope i don't come off as being difficult. I just think this is an awesome project and intend to use this a lot in my projects, so I care about its implementation. :)

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[developit]

Hey @wellcaffeinated - apologies for dropping the ball on this. I separately updated to Comlink 4, and looking at the diff I think we're pretty close to these changes. I'm going to merge this PR, but I would actually like to change the behavior of the default (singleton) option so that it supports your import { MyClass } from 'comlink-loader!./other' example.

[wellcaffeinated]

@developit Cool!
so where does that land in the new api? does it follow what i wrote in my earlier comment #12 (comment)

It would be great to have examples of all the ways to use this in the README :)