Skip to content

fix: detect service worker entrypoints correctly #2396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

fix: detect service worker entrypoints correctly #2396

wants to merge 1 commit into from

Conversation

@jimmed jimmed requested a review from a team as a code owner December 12, 2022 15:26
@changeset-bot
Copy link

changeset-bot bot commented Dec 12, 2022

⚠️ No Changeset found

Latest commit: 6a9554e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

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/runs/3677216220/npm-package-wrangler-2396

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

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

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/runs/3677216220/npm-package-wrangler-2396 dev path/to/script.js
Additional artifacts:
npm install https://prerelease-registry.devprod.cloudflare.dev/runs/3677216220/npm-package-cloudflare-pages-shared-2396

@mrbbot
Copy link
Contributor

mrbbot commented Dec 12, 2022

Hey! 👋 I've just tried this PR out with the reproduction locally and it doesn't appear to fix the issue. 😕 I had a bit more of a look into this though, and I think the right fix would be to remove the format option outright. From the docs, it looks like omitting this option disables format conversions if bundleing is disabled, as is the case here.

@jfsiii
Copy link

jfsiii commented Dec 12, 2022

+1 to both comments @mrbbot

This PR didn't fix the issue for my case

However, commenting out the format line with // format: "esm", did fix the issue

@jimmed
Copy link
Author

jimmed commented Dec 13, 2022

Apologies -- I didn't have time for a proper write-up of this. In order to use the hint property, you need to run your wrangler deploy with --format=service-worker.

I've been deploying successfully using this change (via patch-package).

mrbbot added a commit that referenced this pull request Feb 20, 2023
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.

Closes #1668
Closes #2396
Ref #2737
mrbbot added a commit that referenced this pull request Feb 20, 2023
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.

Closes #1668
Closes #2396
Ref #2737
@mrbbot mrbbot closed this in 7912e63 Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants