feat(wasm): add SSR support for .wasm?init#21102
Conversation
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
upsuper
changed the title
upsuper
changed the title
sapphi-red
left a comment
sapphi-red
left a comment
There was a problem hiding this comment.
Thanks, but this isn't working for build.
When I run vite build --ssr src/app.js in playground/ssr-wasm, I expect the wasm files to be copied (or inlined depending on the option) to dist. But that didn't happen.
sapphi-red
added
p2-nice-to-have
|
@sapphi-red Thanks for your review! I have looked into the issue, and now build should be working correctly. The test should now be covering the build case properly as well, and I've add some tests to verify the build result. Also I have done some manual testing locally and confirmed that it works the way I'd expect. |
|
Hi @sapphi-red, it has been a week since the last update of this PR, would you mind taking another look at it? Is there anything I can do to help moving it forward? |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@sapphi-red I updated the PR to handle wasm init specially via replacing the asset placeholder with a different placeholder, and replace it as part of |
sapphi-red
left a comment
sapphi-red
left a comment
There was a problem hiding this comment.
I'll take a look soon. In the meantime, would you rebase the PR on the latest main branch? The tests may fail without the _VITE_TEST_JS_PLUGIN=1 env var, but don't worry about that.
sapphi-red
changed the title
.wasm?init
I assume that the current CI failures are from this. I checked |
|
@upsuper Would you enable "Allow edits from maintainers" so that I and other can add changes? |
I'd like to, but unfortunately I can't, because this is forked under an org I use to park all my forks, and GitHub doesn't support "Allow edits from maintainers" for a repo owned by an org. I've instead invited you as an admin of my fork project. Hope that helps. |
|
Thanks! I was able to add the changes. |
|
@upsuper Could you grant me access to this repository so I can make some updates? |
Thanks! I was able to add the changes. |
Related to vitejs/vite#21566 This is because vitejs/vite#21102 will refactor the logic around `wasmHelperPlugin`, and `nativeWasmHelperPlugin` is not aligned with the `fileToUrl` behavior. So I plan to remove it for now and re-port it when it’s needed again later.
sapphi-red
left a comment
sapphi-red
left a comment
There was a problem hiding this comment.
Thanks for your patience 💚
3 participants
This PR intends to add WASM support for Vitest and SSR. It should fix #8882 as well as vitest-dev/vitest#6723.
Implementation
The main changes are in the wasm plugin, that provide different helper code based on whether the consumer is server or client. As I'm new to this project, it's not clear to me whether this is the right approach.
I also looked into
vite-plugin-wasmfor how it handles SSR and vitest. What it does is:This approach doesn't feel great to me.
Testing
There are two tests I added:
ssr-wasmwhich serves SSR with WASM loaded.wasme2e test that checks that WASM can be imported and run within tests.It's not clear to me whether those tests are in the right location. Please let me know if there are better places for them.
Regarding Vitest
Checking
consumeractually doesn't seem to be a reliable way to work in Vitest. Vitest'senvironmentcan specifyviteEnvironment. Within its builtin environments,jsdomandhappy-domspecifiesclientas their vite environment which, on the Vite side, usesconsumer: 'client', even though the tests are still running in Node.js.I'm going to leave it there given that Vitest's default environment is
node, so this mechanism should just work most of the time, and also because I failed to find a way to add test for this behavior.I tried adding
// @vitest-environment jsdomto a spec file, but esbuild complains aboutwhich is probably because jsdom doesn't have the proper implementation for it currently. So I'm not going to dig deeper on this rabbit hole for now.
One trick I found is that some plugins like
vite-plugin-solidimplicitly changes the test environment, which could potentially cause confusion.If someone wants to address this part in the future, one possible approach is from
vite-plugin-wasmthat goes through plugins to findvitest, then combine it with the consumer check to decide which way the file is passed and obtained. It's hacky but I can confirm it works locally.