[wrangler] Preserve inferred zone in Miniflare worker options#13792
[wrangler] Preserve inferred zone in Miniflare worker options#13792mihaip wants to merge 1 commit intocloudflare:mainfrom
Conversation
🦋 Changeset detectedLatest commit: bdab3d9 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 |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
| dev: { | ||
| ip: "localhost", | ||
| port: undefined, | ||
| local_protocol: "http", | ||
| upstream_protocol: "http", | ||
| enable_containers: false, | ||
| ...overrides.dev, | ||
| }, | ||
| ...overrides, | ||
| } as unknown as Config; |
There was a problem hiding this comment.
🟡 makeConfig spread ordering causes dev defaults to be silently overwritten
In the makeConfig helper, when overrides.dev is provided, the dev defaults (ip, port, local_protocol, upstream_protocol, enable_containers) are first correctly merged via ...overrides.dev inside the dev object (line 33), but then immediately overwritten by ...overrides at the top level (line 35). Because overrides contains the key dev: { host: "example.workers.dev" }, the top-level spread replaces the entire merged dev with just { host: "example.workers.dev" }, discarding all defaults.
The first test passes only by coincidence—getMiniflareZone only reads config.dev.host and returns early. But any future test that supplies overrides.dev and relies on the default dev properties (e.g., enable_containers) further down in unstable_getMiniflareWorkerOptions (packages/wrangler/src/api/integrations/platform/index.ts:522) will silently get undefined values instead of the intended defaults.
| dev: { | |
| ip: "localhost", | |
| port: undefined, | |
| local_protocol: "http", | |
| upstream_protocol: "http", | |
| enable_containers: false, | |
| ...overrides.dev, | |
| }, | |
| ...overrides, | |
| } as unknown as Config; | |
| dev: { | |
| ip: "localhost", | |
| port: undefined, | |
| local_protocol: "http", | |
| upstream_protocol: "http", | |
| enable_containers: false, | |
| ...overrides.dev, | |
| }, | |
| ...overrides, | |
| dev: { | |
| ip: "localhost", | |
| port: undefined, | |
| local_protocol: "http", | |
| upstream_protocol: "http", | |
| enable_containers: false, | |
| ...overrides.dev, | |
| }, | |
| } as unknown as Config; |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Can you add a test (and probably some handling in the code) for the case where the host cannot be inferred? e.g. routes: "*/*".
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
Fixes #13791.
Summary
@cloudflare/vite-pluginusesunstable_getMiniflareWorkerOptions()to build the local Miniflare worker config. That helper was not forwarding the local dev zone, so Miniflare fell back to<worker>.example.comfor theCF-Workerheader.This makes the helper reuse Wrangler's existing host inference logic and pass the resulting
zonethrough inworkerOptions, so the Vite local-dev path matches the existingwrangler dev --localpath.That existing path already:
dev.origin.hostnameashost ?? getInferredHost(routes, config.configPath)inresolveDevConfig()zoneinLocalRuntimeControllerThis change mirrors that behavior for callers of
unstable_getMiniflareWorkerOptions().Tests
dev.host