Disable format conversion when detecting format

[mrbbot]

What this PR solves

Wrangler automatically detects whether your code is a modules or service-worker format Worker based on the presence of a default export. This check currently works by building your entrypoint with esbuild and looking at the output metafile.

Previously, we were passing format: "esm" to esbuild when performing this check, which enables format conversion. This may introduce export default into the built code, even if it wasn't there to start with, resulting in incorrect format detections.

This change removes format: "esm" which disables format conversion when bundling is disabled: https://esbuild.github.io/api/#format.

We may want to use a package like es-module-lexer in the future, but this issue is affecting users, and this is probably the simplest fix.

How to test:

Run wrangler dev worker.js with the following:

addEventListener("fetch", (event) => {
  event.respondWith(new Response(typeof module));
});

Before this PR, you'd see When using module syntax, the 'fetch' event handler should be declared as an exported function on the root module as opposed to using the global addEventListener(). in the console, indicating Wrangler thought this was a modules-format Worker, and uploaded it as such. Attempting to access this in the browser results in Uncaught Error: Handler does not export a fetch() function..

After this PR, the Worker should be deployed as a service-worker, and attempting to access it in the browser will result in a 200 OK response.

Associated docs issues/PR:

N/A

Author has included the following, where applicable:

• Tests
• Changeset

Reviewer has performed the following, where applicable:

• Checked for inclusion of relevant tests
• Checked for inclusion of a relevant changeset
• Checked for creation of associated docs updates
• Manually pulled down the changes and spot-tested

Fixes #1668
Supersedes #2396
Supersedes #2737

[changeset-bot]

🦋 Changeset detected

Latest commit: 0271af5

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

This PR includes changesets to release 1 package

Name	Type
wrangler	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]

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4224956213/npm-package-wrangler-2766

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/2766/npm-package-wrangler-2766

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4224956213/npm-package-wrangler-2766 dev path/to/script.js

Additional artifacts:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/4224956213/npm-package-cloudflare-pages-shared-2766

Note that these links will no longer work once the GitHub Actions artifact expires.

[codecov]

Codecov Report

Merging #2766 (da20e04) into main (a9adfbe) will increase coverage by 5.89%.
The diff coverage is n/a.

❗ Current head da20e04 differs from pull request most recent head 0271af5. Consider uploading reports for the commit 0271af5 to get more accurate results

@@            Coverage Diff             @@
##             main    #2766      +/-   ##
==========================================
+ Coverage   68.12%   74.01%   +5.89%     
==========================================
  Files         166      166              
  Lines       10161    10161              
  Branches     2705     2705              
==========================================
+ Hits         6922     7521     +599     
+ Misses       3239     2640     -599     

Impacted Files	Coverage Δ
packages/wrangler/src/entry.ts	98.37% <ø> (+30.08%)	⬆️
packages/wrangler/src/user/user.ts	72.31% <0.00%> (+0.26%)	⬆️
packages/wrangler/src/index.ts	83.98% <0.00%> (+0.39%)	⬆️
packages/wrangler/src/config/validation-helpers.ts	97.97% <0.00%> (+0.50%)	⬆️
packages/wrangler/src/config/validation.ts	90.88% <0.00%> (+3.70%)	⬆️
packages/wrangler/src/git-client.ts	43.75% <0.00%> (+4.16%)	⬆️
...ackages/wrangler/src/__tests__/helpers/mock-bin.ts	100.00% <0.00%> (+5.26%)	⬆️
...ckages/wrangler/src/__tests__/helpers/msw/index.ts	100.00% <0.00%> (+5.55%)	⬆️
packages/wrangler/src/publish/index.ts	94.87% <0.00%> (+10.25%)	⬆️
packages/wrangler/src/bundle.ts	93.68% <0.00%> (+10.67%)	⬆️
... and 16 more

[petebacondarwin]

What is the situation if the file is supposed to be a service-worker format but contains an import statement?

[mrbbot]

What is the situation if the file is supposed to be a service-worker format but contains an import statement?

This shouldn't matter, as the detection is only looking for exports, but I could see how an import might affect this, especially given what esbuild's doing with format conversion. I've added a test for it, which passes without additional changes.