Generate types for all bindings across all environments #11893
Conversation
🦋 Changeset detectedLatest commit: 7a1733a 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 |
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: |
| ); | ||
| }); | ||
|
|
||
| it("should collect vars only from specified environment with --env", async () => { |
There was a problem hiding this comment.
This is something I am not too certain on. Currently main makes it so all variables are union-ized so no matter the environment you have access to all environment variable possibilities.
However, with the change above where using --env makes it so only that specific environments bindings are included, shouldn't vars do the same?
There was a problem hiding this comment.
Sorry could you clarify what you mean?
Do we currently get a union of the variables even when --env is specified and your changes here change that?
There was a problem hiding this comment.
Do we currently get a union of the variables even when
--envis specified and your changes here change that?
Correct. If you were to run this test on main it would generate the following type:
// Without a `--env` flag
interface Env {
MY_VAR: "top-level" | "staging" | "production";
STAGING_ONLY: "staging-only-value";
PROD_ONLY: "prod-only-value";
}
// With a `--env=staging` flag
interface Env {
MY_VAR: "top-level" | "staging" | "production";
STAGING_ONLY: "staging-only-value";
PROD_ONLY: "prod-only-value";
}But with this PR it generates the following:
// Without a `--env` flag
interface Env {
MY_VAR: "top-level" | "staging" | "production";
STAGING_ONLY: "staging-only-value";
PROD_ONLY: "prod-only-value";
}
// With a `--env=staging` flag
interface Env {
MY_VAR: "staging";
STAGING_ONLY: "staging-only-value";
}So variables are now no longer union-ized when providing an environment. I do feel that if you ask for types for X environment you should get X environments types. But at the same time, I think this would be a breaking change so I am inclined to revert it.
vicb
left a comment
vicb
left a comment
There was a problem hiding this comment.
Quick first pass, added some comment.
I really like the added comments 🙏
dario-piotrowicz
left a comment
dario-piotrowicz
left a comment
There was a problem hiding this comment.
I've left some comments as an initial first review, please have a look 🙂🙏
| ); | ||
| }); | ||
|
|
||
| it("should collect vars only from specified environment with --env", async () => { |
There was a problem hiding this comment.
Sorry could you clarify what you mean?
Do we currently get a union of the variables even when --env is specified and your changes here change that?
| KV_TOP: KVNamespace; | ||
| D1_TOP: D1Database; | ||
| KV_STAGING: KVNamespace; | ||
| D1_STAGING: D1Database; | ||
| KV_PROD: KVNamespace; | ||
| R2_PROD: R2Bucket; |
There was a problem hiding this comment.
if a binding is only present in an environment, could we make it optional?
e.g.
KV_TOP: KVNamespace;
D1_TOP: D1Database;
KV_STAGING?: KVNamespace;
D1_STAGING?: D1Database;
KV_PROD?: KVNamespace;
R2_PROD?: R2Bucket;
I think that that would be more correct and more type-safe 🤔
There was a problem hiding this comment.
I thought about this as well and I am still unsure what would be best here. Ideally if you'r binding a resource you should know it always exists and you can access / use it. But on the other hand, in your code how do you know what environment you're in and what resource are or are not available.
Cc @MattieTK
There was a problem hiding this comment.
After a bit of tinkering and seeing what the runtime actually offers us it's a bit more complex.
Because top-level bindings could also not exist (If you run wrangler dev --env staging you of course don't get access to any top-level bindings, only the bindings of that env) we would need to make all top-level bindings optional as well.
Going to have more of a play around with this to try and tweak it.
There was a problem hiding this comment.
ah yes, sorry my bad, I forgot that bindings are not inherited from the top-level: https://developers.cloudflare.com/workers/wrangler/configuration/#non-inheritable-keys 👍
There was a problem hiding this comment.
So I would suggest that, if possible, if a binding is present in all the environments in the config file (including the top-level) it should be non-nullable, if instead is missing somewhere it should be optional 🙂
4 participants
Fixes #10400, #9709 and #8475
Currently, if you run
wrangler typesit only generates TypeScript types for bindings (KVNamespace,D1Database, etc) defined in the top-level configuration (or a single environment when using--env). Now, by default, it collects & generates types for bindings from all environments in your configuration.This ensures your generated types include all bindings that might be used across different deployment environments (e.g., staging, production), preventing TypeScript errors when accessing environment-specific bindings.
Additionally, If the same binding name exists with different types across environments (e.g.,
CACHEis a KV namespace in one environment but an R2 bucket in another), an error will be thrown to prevent type conflicts.A picture of a cute animal (not mandatory, but encouraged)