Conversation
🦋 Changeset detectedLatest commit: e53c7b5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
|
I'm Bonk, and I've done a quick review of your PR. Adds named-only module Worker support in local development.
|
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/codemods
@cloudflare/config
@cloudflare/containers-shared
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-plugin
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
|
packages/wrangler/src/deployment-bundle/apply-middleware.ts:R32-38 Export changes break watched rebuilds After a running Worker adds or removes its default export, Wrangler computes Example: Start Recommended fix: Derive facade eligibility from each build's current export metadata, or restart/reconfigure the bundle when the entrypoint's export shape changes. Add watch-mode tests for both default-to-named-only and named-only-to-default edits. |
|
|
||
| Allow `wrangler dev` to start module Workers without a default export | ||
|
|
||
| Wrangler now skips default-entrypoint middleware for Workers that only export named entrypoints. This avoids generating a middleware facade with an invalid default import. |
There was a problem hiding this comment.
Changeset exposes internal implementation
The second paragraph discusses the middleware facade and generated imports. REVIEW.md requires changesets to describe user-facing impact instead of internal implementation.
There was a problem hiding this comment.
Fixed! Amended that commit.
The middleware facade only wraps the default entrypoint and re-exports named entrypoints unchanged, so skipping it for named-only modules preserves existing behavior while avoiding an invalid default import. The regression test waits for the Worker runtime to reload and fails on build errors because worker.ready only indicates that the proxy server started and can resolve after a recoverable bundling failure.
Move console-prefix from request middleware to an esbuild inject. The console-prefix module was previously loaded through the middleware facade only for its import-time side effect. Its default export was a passthrough that did not alter request handling. This is a noop refactor for default exports because injecting the module directly still applies the same patch. This is a fix for named-only workers, which were never able to load the middleware previously anyway.
Re-evaluate entrypoint exports and restart the watcher when middleware facade eligibility changes. Restarting updates esbuild's immutable entrypoint graph without sacrificing incremental rebuilds for ordinary edits.
226a196 to
e53c7b5
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
Good catch! I pushed a new commit to address that. I went with the restart-on-changes approach because that seems to play better with the watchers and incremental builds. Added both of those tests. |
| exportShapeChanged = | ||
| shouldApplyMiddlewareLoaderFacade(entry) !== | ||
| shouldApplyMiddlewareLoaderFacade({ | ||
| format: entry.format, | ||
| exports: currentExports, | ||
| }); |
There was a problem hiding this comment.
🟡 Named entrypoint edits keep stale names
When a rebuild changes named exports without crossing the default-export boundary, exportShapeChanged remains false. entry.exports retains removed names and omits new RPC entrypoints in the reloaded runtime.
Learn more
Miniflare receives the runtime entrypoint list from buildSourceOptions. This plugin compares only whether the middleware facade applies, not whether the export names changed. A successful rebuild from export class Api {} to export class Admin {} therefore updates the bundle source but leaves entry.exports as ["Api"]. The runtime reload advertises an entrypoint that no longer exists and omits Admin.
Example: Start a named-only Worker exporting Api, then rename that class to Admin. The rebuilt source exports Admin, but service bindings still receive Api as the configured entrypoint name.
Recommended fix: Compare the complete discovered export list with entry.exports and refresh the bundler state whenever it changes. Keep the middleware-facade predicate only for deciding whether facade regeneration is necessary, not for deciding whether runtime export metadata must change.
Was this helpful? React with 👍 or 👎 to provide feedback.
| targetConsumer, | ||
| testScheduled, | ||
| plugins: [ | ||
| exportShapePlugin, |
There was a problem hiding this comment.
🟡 Alternate rebuilds keep stale exports
When no_bundle or a custom build changes exports, exportShapePlugin never refreshes entry.exports. Custom builds can select the stale facade, while either mode exposes stale runtime entrypoints.
Learn more
The new plugin is installed only when build calls bundleWorker. A normal no_bundle build bypasses that call, and runCustomBuild calls bundleWorker without the plugin. Both watch paths reuse the exports discovered before watching started. The new middleware decision now depends on that stale list, and Miniflare also consumes the same list.
Example: A custom build initially emits a default Worker, then emits only CounterService after an input edit. Wrangler still treats the output as default-exporting, generates a facade with a default import, and the rebuild fails instead of loading CounterService.
Recommended fix: Re-run export discovery after each successful custom build and each watched no_bundle entrypoint change. Recreate the facade when its applicability changes, and always publish the refreshed Entry.exports with the rebuilt bundle.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "wrangler": patch | ||
| --- | ||
|
|
||
| Allow `wrangler dev` to start module Workers without a default export | ||
|
|
||
| Previously, `wrangler dev` failed to start module Workers that exported only named entrypoints, such as RPC services, unless an unused default export was added. These Workers now start normally in local development without that workaround. |
| // When multiple workers are running we need some way to disambiguate logs | ||
| // between them. This patch only has a side effect, so inject it independently | ||
| // of the middleware facade, which may be skipped for named-only Workers. | ||
| if (getFlag("MULTIWORKER")) { | ||
| middlewareToLoad.push({ | ||
| name: "patch-console-prefix", | ||
| path: "templates/middleware/middleware-patch-console-prefix.ts", | ||
| supports: ["modules", "service-worker"], | ||
| config: { | ||
| prefix: chalk.blue(`[${entry.name}]`), | ||
| }, | ||
| }); | ||
| const name = "patch-console-prefix"; | ||
| inject.push( | ||
| path.resolve( | ||
| getBasePath(), | ||
| "templates/middleware/middleware-patch-console-prefix.ts" | ||
| ) | ||
| ); | ||
| middlewareConfig[name] = { | ||
| prefix: chalk.blue(`[${entry.name}]`), | ||
| }; |
There was a problem hiding this comment.
| return !( | ||
| entry.format === "modules" && | ||
| entry.exports.length > 0 && | ||
| !entry.exports.includes("default") | ||
| ); | ||
| } |
The middleware facade only wraps the default entrypoint and re-exports named entrypoints unchanged, so skipping it for named-only modules preserves existing behavior while avoiding an invalid default import.
The regression test waits for the Worker runtime to reload and fails on build errors because worker.ready only indicates that the proxy server started and can resolve after a recoverable bundling failure.
Followup to #15518
A picture of a cute animal (not mandatory, but encouraged)