-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
web-worker search path regression with ES6 wasm module #5704
Comments
I made a simpler emscripten It looks like what's happening is that A quick trick I found is to use a following plugin to prevent Vitest's transform from kicking in: 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 Personally, just testing out emscripten's |
Thanks for the investigation - I just pushed your 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. |
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 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 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 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. |
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. |
Just throwing this out there: There is a build flag for emscripten:
https://emscripten.org/docs/tools_reference/settings_reference.html#use-es6-import-meta
|
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
The text was updated successfully, but these errors were encountered: