Fix pthread worker options usage with Vite bundler

[jamsinclair]

Overview

An attempt at solving #22394.

I had previously opened an attempt at #22834, however, it got a bit messy with the commits. I have opened a new PR with a cleaner context which should be easier to focus on the reasoning and necessary changes.

The Problem

When code using threads is compiled with Emscripten and then bundled with Vite it does not bundle and throws an error.

This is due to a variable reference for worker options being used during worker construction. Vite needs inline static worker options to bundle the code successfully.

e.g.

const workerOptions = { type: 'module',  name: 'foo'  } ;

// ❌ this will break vite build as the worker options are dynamic
new Worker(new URL('./my-worker.js', import.meta.url), workerOptions); 

// ✅ The literal worker option values can be statically analysed by Vite 
// and bundled correctly
new Worker(new URL('./my-worker.js', import.meta.url), {
  type: 'module',
  name: 'foo'
}); 

The Solution

Emscripten needs to output literal static values instead of using the workerOptions variable.

[jamsinclair]

Previously @sbc100 you had kindly suggested making use of a macro.

{{{
globalThis.WORKER_OPTIONS = {
   ...
};
null;
}}}

This would have been a great solution, however, because we use a dynamic PThread.nextWorkerID reference I couldn't think of a way to make it work. Let me know if you have any other ideas.

[sbc100]

I'm pretty sure we can still find a way to avoid these 4-fold duplication. I'll see if I can some up with something

[jamsinclair]

Is it possible to capture a raw string and then output that in these templates? Is there a particular templating framework being used? 🤔

[sbc100]

Rather than checking for this specific output I wonder if we can instead test directly that it works with vite.

I recently added some vite tests to test_browser.py so I think we could add them there. Then we could test that it actually runs though vite and executed.

I also wonder if its enough to test this in just rollup, since IIUC vite uses rollup?

[jamsinclair]

Awesome! I'll look into adding some tests there 👀

For this particular Vite issue, rollup would not be enough to test. The special logic around these workers is implemented on the Vite side.

[sbc100]

Good to know. Thanks for working on this!

As a followup question. Is there some way we can test this stuff using node, or do we really need to test the output in a browser? I test to prefer non-browser-tests since they are much easy to work with, but maybe this is a case where we really need the browser?

[jamsinclair]

For the particular issue concerning the PR, I think the browser is the only way.

Vite is primarily a bundler for the browser, although, as it is built on top of rollup it can be customised and you could bundle a JS file only and test with node (providing the bundled code is Node compatible).

I think the documentation for bundling a library could possibly be used by some tests and then run with node. I could try to create a PoC, if you like?

[sbc100]

That would be most welcome.

BTW, are you a vite contributor/maintainer?

[sbc100]

Thanks for working on this! I'm excited to get more test coverage here.

[jamsinclair]

We need to tell Vite to use the 'es' format for workers, as our test case is outputting with ES6 enabled.

[jamsinclair]

We need to compile for only browser based environments, otherwise Vite runs into issues with bundling.

Perhaps it could be worth moving -sEnvironment to the default args array below.