[vitest-pool-workers] Support require("./x.wasm?module") in CommonJS deps#14189
Open
tahmid-23 wants to merge 2 commits into
Open
[vitest-pool-workers] Support require("./x.wasm?module") in CommonJS deps#14189tahmid-23 wants to merge 2 commits into
tahmid-23 wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: f4c2a37 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
workers-devprod
requested review from
a team and
emily-shen
and removed request for
a team
Contributor
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…deps
Previously, only literal `await import("./x.wasm?module")` specifiers were
rewritten through the static analysis path added in cloudflare#11094. CommonJS
dependencies that use `require("./x.wasm?module")` reach the module-fallback
service at runtime, where the `?module` suffix went unhandled. The fallback
either failed with `No such module "<abs>/x.wasm?module"` or, when a
CompiledWasm rule was configured, attempted to evaluate the WebAssembly bytes
as JavaScript.
These `require()`s work in deployed workers because esbuild's bundler
statically rewrites them into ES dynamic imports. vitest-pool-workers'
Vite-based pipeline doesn't do that rewrite and instead defers to the
module-fallback at runtime.
The module-fallback now strips `?module` from the resolved target and
synthesizes a CommonJS wrapper that re-`require`s the underlying `.wasm` by
absolute path, exposing it on `default` to match what workerd produces for
CompiledWasm modules.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Per .changeset/README.md, changeset titles shouldn't use `fix:`/`feat:`/etc prefixes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tahmid-23
force-pushed
the
fix/cjs-wasm-module-fallback
branch
from
6a6dff5 to
f4c2a37
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, only literal
await import("./x.wasm?module")specifiers were rewritten through the static analysis path added in #11094. CommonJS dependencies that userequire("./x.wasm?module")reach the module-fallback service at runtime, where the?modulesuffix went unhandled. The fallback either failed withNo such module "<abs>/x.wasm?module"or, when a CompiledWasm rule was configured, attempted to evaluate the WebAssembly bytes as JavaScript.These
require()s work in deployed workers because esbuild's bundler statically rewrites them into ES dynamic imports. vitest-pool-workers' Vite-based pipeline doesn't do that rewrite and instead defers to the module-fallback at runtime.The module-fallback now strips
?modulefrom the resolved target and synthesizes a CommonJS wrapper that re-requires the underlying.wasmby absolute path, exposing it ondefaultto match what workerd produces for CompiledWasm modules.Prisma 7 generates such
require()statements onwasm?moduleimports. When running vitest under its default configuration, it prefers to load js-like extensions before ts-like extensions. Hence, this issue currently exists while running Prisma 7 with vitest-pool-workers, with the workaround being to reorder the extensions to prefer ts-like extensions first. This fix aims to make this workaround unnecessary (and support other possible packages thatrequire()awasm?moduleimport.It is possible to construct code, somewhat pathologically, that uses a
requirestatement that the fallback server does not handle because we do not strip the?modulesuffix fromspecifier. However, the example that I tried wouldn't actually resolve the imports when actually running the worker. Hence, I did not choose to strip the suffix fromspecifier, since I do not believe it is necessary for imports that actually resolve on workerd.A picture of a cute animal (not mandatory, but encouraged)