Define experimental Cloudflare configuration with a single default export - #15713
jamesopstad wants to merge 6 commits into
Conversation
🦋 Changeset detectedLatest commit: 8291143 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 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 |
@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: |
| env[bindingName] = { | ||
| ...binding, | ||
| worker: normalizeConfigReference(binding.worker, target, "worker"), | ||
| worker: normalizeConfigReference(binding.worker, target), |
There was a problem hiding this comment.
A Container definition also has a name, so bindings.worker({ worker: container }) now resolves to a service binding for the Container name rather than failing validation. The old type discriminator prevented this; retain an equivalent Worker-shape check before converting the reference.
| worker: normalizeConfigReference(binding.worker, target), | |
| worker: | |
| isRecord(target) && | |
| typeof target.name === "string" && | |
| typeof target.compatibilityDate === "string" | |
| ? target.name | |
| : binding.worker, |
There was a problem hiding this comment.
This would be a type error and I think that's the right level of safety here. At runtime, providing a Container here is no different to providing an incorrect string.
|
I'm Bonk, and I've done a quick review of your PR. Consolidates experimental Cloudflare configuration into a single default export and updates downstream build/config consumers.
|
75cf90c to
1cc4eeb
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
| return isRecord(resolved) && typeof resolved.name === "string" | ||
| ? resolved.name | ||
| : reference; |
There was a problem hiding this comment.
🟡 Invalid references become Worker names
When binding.worker resolves to any named object, normalizeConfigReference converts it into a Worker name. The binding then passes schema validation and targets a nonexistent Worker.
Learn more
Cross-Worker bindings accept either a Worker name or a Worker definition. The resolver currently treats every resolved object with a string name as a Worker, without checking the remaining Worker shape. This includes Container definitions and arbitrary objects. The resulting string satisfies WorkerBindingSchema, so later parsing cannot detect the wrong resource type.
Example: A JavaScript config passes { name: "jobs", image: { reference: "registry/app" } } as bindings.worker({ worker: container }). Resolution changes the object to "jobs". Configuration parsing succeeds, but deployment creates a service binding to a Worker named jobs rather than rejecting the Container reference.
Recommended fix: Validate the resolved target with InputWorkerSchema before replacing it with target.name. Preserve invalid targets so the binding schema reports them, or add a targeted issue explaining that worker did not resolve to a Worker definition.
Was this helpful? React with 👍 or 👎 to provide feedback.
1cc4eeb to
159937e
Compare
4ae55ee to
af76148
Compare
af76148 to
5ee549a
Compare
5ee549a to
8291143
Compare
Define experimental Cloudflare configuration with a single default export.
Experimental
cloudflare.config.tsfiles now define settings and resources together in a default-exporteddefineConfig()call. Add a Worker underworker, add Containers to thecontainersarray, or omit both to provide settings only.A picture of a cute animal (not mandatory, but encouraged)