Open elalish opened 2 months ago
I made a simpler emscripten EXPORT_ES6
setup to investigate the issue https://github.com/hi-ogawa/reproductions/tree/main/vitest-5704-emscripten-esm-worker
It looks like what's happening is that new URL(..., import.meta.url)
is heavily used by EXPORT_ES6
format, but Vitest started to replace it with new URL(..., self.location)
https://github.com/vitest-dev/vitest/pull/4258 for web transform (including web worker simulation provided by @vitest/web-worker
) and it's actually undefined
here, so the url becomes invalid.
A quick trick I found is to use a following plugin to prevent Vitest's transform from kicking in: https://github.com/hi-ogawa/reproductions/blob/a72885fedbf70ddb6d48294d99d26d24e79105b9/vitest-5704-emscripten-esm-worker/vitest.config.ts#L6C1-L21C5
Can you try this on your repo to see if it works?
{
name: "keep-import-meta-url",
enforce: "pre",
transform(code, id, _options) {
// prevent NormalizeURLPlugin from replacing import.meta.url with self.location
// https://github.com/vitest-dev/vitest/blob/d8304bb4fbe16285d014f63aa71ef9969865c691/packages/vitest/src/node/plugins/normalizeURL.ts#L11
// since it breaks `new URL(..., import.meta.url)` used by emscripten EXPORT_ES6 output
// https://github.com/emscripten-core/emscripten/blob/228af1a7de1672b582e1448d4573c20c5d2a5b5a/src/shell.js#L242
if (id.endsWith("/dist/lib.js")) {
return code.replace(/\bimport\.meta\.url\b/g, `String(import.meta.url)`);
}
},
}
Technically @vitest/web-worker
is probably intended to be used on a web environment (such as jsdom/happy-dom), so not sure if it should be called a bug/fixable or it's imply out of the scope to support this use case.
Personally, just testing out emscripten's EXPORT_ES6
for the first time, it's mostly working nice with Vite / Vitest in general, so it would be cool if Vitest can support it without friction.
Thanks for the investigation - I just pushed your vitest.config.js
to my above PR, but it didn't make a difference (I'm assuming this config is picked up automatically by name?).
As to use cases, indeed we filed a related bug on Emscripten a couple years back. They had also been thinking of WASM libraries as being built either for web or for Node, but not both. We convinced them that the "both" use case is really important, testing being a key reason. If we build a computational library that we intend to work on the web, it's still really convenient to test it headless, particularly on a Github CI, so Node is ideal for that.
Let me know if there's anything else I can try to help narrow this down.
If we build a computational library that we intend to work on the web,
I haven't fully understand your use case, but personally, your testing strategy might be a little too convoluted for what it achives. I'm assuming that it's a js/wasm binding of pure computation library and it's not tied to platform api (e.g. filesystem, pthread, web window api, etc..). If that's case, wouldn't it be simpler to test your Module
directly without going through web worker?
I understand your actual consumption on web sample would obviously require off-loading computation on Web worker, but I just want to make sure what exactly your objective of integrating Vitest (or any testing) there.
If you want to exercise actual web worker communication, then what @vitest/web-worker
provides is a simulation on NodeJS and that looks a bit tricky to make it work and doesn't seem worth the effort to get around as it's still a simulation. If you go with this path, then it might be better to use Vitest browser mode https://vitest.dev/guide/browser.html (though it's also possible that some Vite / Vitest transform would mess up emscripten output) or going with full browser e2e testing like playwright.
On the other hand, If the purpose is to exercise only your wasm binding (+ model io library integration e.g. gltf?), then simpler (and more direct) testing approach could be to somehow extract that logic out of your bindings/wasm/examples/worker.ts
and test directly as pure js/wasm library without WebWorker.
I get that if Vitest would somehow fix the regression, then you don't have to change any code and that's obviously better, but I just wanted to share my perspective in terms of overall testing strategy. (Also because of this perspective, it's unlikely that I'll look into further on your reproduction to investigate this specific use case unless you can minimize reproduction to point out the issue.)
Fair point, my earlier argument didn't address why I'm testing a worker. In my case it's because I'm testing ManifoldCAD.org, which uses a web worker to keep all the calculation off the main thread. It also forms a more compact API: you hand it a script of the geometry operations you want to do and it packs the result up as a GLB 3D model and hands it back. This makes for a very convenient and compact surface for end-to-end tests, testing every example on ManifoldCAD.org, but by extension, testing a huge variety of operations on the underlying Manifold library as well.
That said, at the time I built this I didn't realize the difficulties workers posed to testing on Node (it happened to just work on the version of vitest I started with, so it never occurred to me to do anything else). Is there some kind of pattern for making a worker just a lightweight wrapper for its business logic code, so that the testing can hook to the business logic only and skip the worker? Perhaps that would be a better approach.
Thanks for providing the context. As I'm very much interested in your project, I'm happy to help if I can, but I don't have a time to get into your code base and actually understand something at that moment. (Well, I'm starting to read a few bits, but I would imagine your project is pretty non trivial for me to suggest something quickly).
Please feel free to leave this issue open for now. I might be able to get back to this in the future.
Describe the bug
I have a WASM library I built as an ES module with Emscripten, so I have manifold.js and manifold.wasm in my built directory. In a worker I call it like
import Module from './built/manifold';
the offending lineThis works with
npm test
in vitest 0.33, but not in 0.34. The error I get in 0.34 is:And I believe I'm getting an equivalent error in 1.6.0:
You mention in common errors that the base path can be a problem, which this certainly seems related to, but I haven't set anything special. Even though manifold.js is calling manifold.wasm in its own directory, it can't seem to resolve the URL anymore. Is there something special I need to do to help it understand these files are both in
built/
?Reproduction
I made a PR where you can see our CI (which runs vitest in the wasm action) passes and then fails based on updating vitest.
System Info
Used Package Manager
npm
Validations