Skip to content

[vitest-pool-workers] Support Workerd's new module registry - #15117

Draft
edmundhung wants to merge 1 commit into
mainfrom
edmundhung/vitest-pool-new-module-registry
Draft

[vitest-pool-workers] Support Workerd's new module registry#15117
edmundhung wants to merge 1 commit into
mainfrom
edmundhung/vitest-pool-new-module-registry

Conversation

@edmundhung

@edmundhung edmundhung commented Aug 10, 2026

Copy link
Copy Markdown
Member

Close #15066

Support Workerd's V2 module fallback protocol in @cloudflare/vitest-pool-workers when the new module registry is enabled, while leaving the legacy V1 handler unchanged for compatibility dates or configurations that select legacy_module_registry.

The V2 path preserves URL-based import.meta behavior, returns CommonJS dependencies as native CommonJS modules with named exports, and updates the Vitest module runner patches for runtime module URLs. It also includes an isolated, flag-selectable workaround for Workerd's current redirected-module identity mismatch; the canonical redirect implementation remains separate so the workaround can be removed cleanly after the runtime is fixed.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: this follows Workerd's existing compatibility-date and compatibility-flag selection automatically and adds no new vitest-pool-workers configuration API.

A picture of a cute animal (not mandatory, but encouraged)

@changeset-bot

changeset-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a38861c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cloudflare/vitest-pool-workers Patch

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

@github-actions

Copy link
Copy Markdown
Contributor

✅ All changesets look good

@ask-bonk

ask-bonk Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

I've done a thorough review. Let me now consolidate my assessment.

The code is well-structured and well-documented. The V1 path is untouched. The V2 path is properly gated behind protocol detection. Tests are comprehensive.

Let me reconsider one genuine concern I noticed: handleV2ModuleFallbackError and error reporting for internal imports. The console.error message says Failed to ${request.type} ..., which could produce Failed to internal "..."—minor wording, not actionable.

Let me look for a real bug in the redirect workaround around stale cache entries. The v2ResolvedPaths map is per-vite-server and never cleared. If a file's resolution changes during a watch/HMR session (e.g., a symlink or package.json changes), the cache would return a stale filePath. However, module URLs are content-identity in the new registry and vite server restarts create a new WeakMap key. This is a minor edge case—likely acceptable for the workaround. Not worth flagging as actionable given the workaround's temporary nature.

Let me verify one final actual concern: in resolveV2, the forced-type early return happens before maybeGetTargetFilePath, but the .wasm?module stripping for require happens first. For an import of x.wasm?module, target keeps ?module, then maybeGetTargetFilePath("...x.wasm?module")isFile false, extensions not probed (not require), not a dir → returns undefined → falls to viteResolve. In V1, the same. Consistent.

I've examined the logic paths thoroughly. Let me check the one thing that genuinely could be a bug: whether the esModuleLexer.parse on CommonJS-transpiled or non-ESM content in rewriteV2ModuleSpecifiers could corrupt. But it's only called when "esModule" in module.contents, so contents are ESM. Safe.

The implementation is solid, well-tested, and the V1 path is preserved. The identified concerns (stale cache in a temporary workaround, error message wording for internal) are minor and not actionable per the "don't nitpick" guidance.

Let me confirm I'm reviewing the right PR and there are no compat/logic bugs I'm missing by checking the worker patch runExternalModule URL construction once more, since that's runtime-critical.

The runExternalModule patch (worker/index.ts:297-304): for an id not starting with / and not a URL scheme, it wraps as new URL(\/bundle/${id}`, "file:///").href. This matches the /bundle/stripping ingetV2RequestSpecifier`. Consistent design.

I've completed a thorough review. The changes are correct, the V1 legacy path is untouched for backward compatibility, protocol selection is driven by miniflare's parser, and tests cover the key behaviors. The only observations are minor/non-actionable (temporary-workaround cache lifetime, internal in an error string).

LGTM

github run

@pkg-pr-new

pkg-pr-new Bot commented Aug 10, 2026

Copy link
Copy Markdown
@cloudflare/autoconfig

npm i https://pkg.pr.new/@cloudflare/autoconfig@15117

@cloudflare/build-output-utils

npm i https://pkg.pr.new/@cloudflare/build-output-utils@15117

@cloudflare/config

npm i https://pkg.pr.new/@cloudflare/config@15117

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@15117

@cloudflare/deploy-helpers

npm i https://pkg.pr.new/@cloudflare/deploy-helpers@15117

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@15117

miniflare

npm i https://pkg.pr.new/miniflare@15117

@cloudflare/pages-functions

npm i https://pkg.pr.new/@cloudflare/pages-functions@15117

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@15117

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@15117

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@15117

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@15117

@cloudflare/workers-auth

npm i https://pkg.pr.new/@cloudflare/workers-auth@15117

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@15117

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@15117

wrangler

npm i https://pkg.pr.new/wrangler@15117

commit: a38861c

@edmundhung
edmundhung force-pushed the edmundhung/vitest-pool-new-module-registry branch from e644528 to a38861c Compare August 10, 2026 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

Support new module registry in vitest-pool-workers

2 participants