Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add prebuilt web worker #428

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

charlesxsh
Copy link

When using workerpool in Angular or Webpack environment, usually the web worker is written in Typescript. In Webpack 5, it already can bundle the worker script automatically if using the style like mentioned in here.
In Angular, one example will be new Worker(new URL('./worker', import.meta.url));. The "worker" url infers to the worker.ts. Webpack will automatically compile/bundle the script file for us.
However, in workerpool, we can not benefit from this feature.
Thie PR aims to allow us to utilize this feature & more flexible way to use Web Worker.

The example usage is:

const worker = new Worker(new URL("./hello.worker"), import.meta.url));
pool({webWorker: worker});

In Angular, we don't need to manually deal with webpack or tsc turn hello.worker.ts to hello.worker.js and put it into the assets folder. Angular cli/Webpack will do all this for us.

@josdejong
Copy link
Owner

Thanks for your PR.

A few questions:

  1. How can I test that this works? Can you add an example that demonstrates this?
  2. I find the API that you propose quite confusing: it looks like in your example, you create a single worker and pass that to the pool, whereas normally workerpool creates multiple new Worker instances. Is your example actually working with multiple workers? Can we come up with a different API, something like pool({url, importMetaUrl}); and then internally call new Worker(url, importMetaUrl) ?

@charlesxsh
Copy link
Author

Thanks for your PR.

A few questions:

  1. How can I test that this works? Can you add an example that demonstrates this?
  2. I find the API that you propose quite confusing: it looks like in your example, you create a single worker and pass that to the pool, whereas normally workerpool creates multiple new Worker instances. Is your example actually working with multiple workers? Can we come up with a different API, something like pool({url, importMetaUrl}); and then internally call new Worker(url, importMetaUrl) ?
  1. Would you point out how to properly mock some sanity checks (for web worker)? I tried to include the unit test but failed to do that because there seems no existing mocking mechanism(for mocking web sanity check) used in the project and I am not sure add extra dependency is what we want.
  2. Good point. I should use a function that returns a web worker instead of returning an instance.

The reason cannot use something like importMetaUrl you mentioned is due to the limitation of webpack 5. I quote here:

Using a variable in the Worker constructor is not supported by webpack. For example, the following code will not work: const url = new URL('./path/to/worker.ts', import.meta.url); const worker = new Worker(url);. This is because webpack cannot analyse the syntax statically. It is important to be aware of this limitation when using Worker syntax with webpack.

This feature is mainly for more and more web applications (usually written in Typescript) that write part of the heavy modules in Rust or other programming languages, and expose the services through web worker and WASM. This change can dramatically facilitate the hybrid project development that uses Typescript and webpack,

@josdejong
Copy link
Owner

I'm not sure if it is possible to write an automated test for this. Maybe we should just test it manually in a minimal Webpack project?

Maybe we can introduce an option createWebWorker(script, workerOpts), which has the default implementation like it is now:

function createWebWorker(script, workerOpts) {
  return new Worker(script, workerOpts)
} 

Then you can use workerpool like:

pool({
  createWebWorker: (script, workerOpts) => new Worker(new URL("./hello.worker"), import.meta.url))
})

@carcigenicate
Copy link

carcigenicate commented May 2, 2024

I'm not sure if it is possible to write an automated test for this. Maybe we should just test it manually in a minimal Webpack project?

Maybe we can introduce an option createWebWorker(script, workerOpts), which has the default implementation like it is now:

function createWebWorker(script, workerOpts) {
  return new Worker(script, workerOpts)
} 

Then you can use workerpool like:

pool({
  createWebWorker: (script, workerOpts) => new Worker(new URL("./hello.worker"), import.meta.url))
})

It would be awesome if this was supported. I'm having the same issue as the OP. The proposed createWebWorker is roughly how threads handles this:

const pool = Pool(() => spawn(new Worker("./workers/multiplier")), 8 /* optional size */)

I think I'm going to have to go with threads instead of this well-used library for that reason alone unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants