-
Notifications
You must be signed in to change notification settings - Fork 716
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
fix: render a helpful build error if a Service Worker mode Worker has imports #6872
Conversation
… imports A common mistake is to forget to export from the entry-point of a Worker, which causes Wrangler to infer that we are in "Service Worker" mode. In this mode, imports to external modules are not allowed. Currently this only fails at runtime, because our esbuild step converts these imports to an internal `__require()` call that throws an error. The error message is misleading and does not help the user identify the cause of the problem. This is particularly tricky where the external imports are added by a library or our own node.js polyfills. Fixes #6648
🦋 Changeset detectedLatest commit: f3ce6ee The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
Are you sure we can't make it actually work? I think require()s work in the runtime.
(Happy to be wrong) |
If esm is an issue, making that cjs might also do it |
packages/wrangler/src/deployment-bundle/esbuild-plugins/hybrid-nodejs-compat.ts
Outdated
Show resolved
Hide resolved
For posterity, I was wrong, can't do imports/requires in a Service Worker |
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/11130701217/npm-package-wrangler-6872 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/6872/npm-package-wrangler-6872 Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11130701217/npm-package-wrangler-6872 dev path/to/script.js Additional artifacts:npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11130701217/npm-package-create-cloudflare-6872 --no-auto-update npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11130701217/npm-package-cloudflare-kv-asset-handler-6872 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11130701217/npm-package-miniflare-6872 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11130701217/npm-package-cloudflare-pages-shared-6872 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11130701217/npm-package-cloudflare-vitest-pool-workers-6872 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11130701217/npm-package-cloudflare-workers-editor-shared-6872 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11130701217/npm-package-cloudflare-workers-shared-6872 Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
packages/wrangler/src/deployment-bundle/esbuild-plugins/cloudflare-internal.ts
Show resolved
Hide resolved
packages/wrangler/src/deployment-bundle/esbuild-plugins/cloudflare-internal.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌 👏 🎉
A common mistake is to forget to export from the entry-point of a Worker, which causes
Wrangler to infer that we are in "Service Worker" mode.
In this mode, imports to external modules are not allowed.
Currently this only fails at runtime, because our esbuild step converts these imports to an internal
__require()
call that throws an error.The error message is misleading and does not help the user identify the cause of the problem.
This is particularly tricky where the external imports are added by a library or our own node.js polyfills.
Fixes #6648
What this PR solves / how to test
Fixes #[insert GH or internal issue number(s)].
Author has addressed the following