Skip to content

Strip query strings from module names when writing to disk#12824

Merged
jamesopstad merged 1 commit intomainfrom
james/additional-modules-windows
Mar 10, 2026
Merged

Strip query strings from module names when writing to disk#12824
jamesopstad merged 1 commit intomainfrom
james/additional-modules-windows

Conversation

@jamesopstad
Copy link
Copy Markdown
Contributor

@jamesopstad jamesopstad commented Mar 9, 2026

Fixes #11535.

Strip query strings from module names before writing to disk

When bundling modules with query string suffixes (e.g. .wasm?module), the ? character was included in the output filename. Since ? is not a valid filename character on Windows, this caused an ENOENT error during wrangler dev. This was particularly visible when using Prisma Client with the D1 adapter, which imports .wasm?module files.

The fix strips query strings from module names before writing them to disk, while preserving correct module resolution.


  • 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: bug fix

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


Open with Devin

@jamesopstad jamesopstad requested a review from a team as a code owner March 9, 2026 21:00
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Mar 9, 2026

🦋 Changeset detected

Latest commit: beacef5

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

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
Copy link
Copy Markdown
Contributor

workers-devprod commented Mar 9, 2026

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 9, 2026

✅ All changesets look good

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Mar 9, 2026

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

wrangler

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

commit: 695561a

Copy link
Copy Markdown
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - one question... there appears to be more than 2 possible code paths covered by these changes but only two tests. Are we missing tests for some scenarios?

Comment thread packages/wrangler/src/__tests__/deploy/formats.test.ts
@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk Mar 10, 2026
@jamesopstad
Copy link
Copy Markdown
Contributor Author

LGTM - one question... there appears to be more than 2 possible code paths covered by these changes but only two tests. Are we missing tests for some scenarios?

The change is only needed in the path that is being tested. It felt more consistent to use the cleanedPath in the other locations but the functionality shouldn't change there. I think the existing coverage is enough for these i.e. the bug is fixed and the use of cleanedPath elsewhere hasn't broken the existing tests.

@jamesopstad jamesopstad merged commit 9f93b54 into main Mar 10, 2026
52 of 59 checks passed
@jamesopstad jamesopstad deleted the james/additional-modules-windows branch March 10, 2026 10:41
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Prisma Client ENOENT

3 participants