You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have only been exposed to the Workers for a few months.
While I immediately had the feeling that Workers were a fantastic platform, I felt too much friction to get started with Workers. I can say that it took me ~6 months to get comfortable with the platform and only now I can start using all the features with confidence.
IMO there are quite a few things we could do to improve the DX.
This issue is about the name env in async fetch(request, env, ctx) or this.env in i.e. durable objects
I find env to be confusing when all the docs refer to "bindings".
It's easy to mix env with:
process.env (when nodejs_compat is used:)
the vars binding we refer to as "Environment variables"
"Variables and secrets" in the dashboard
"Build > Variables and secrets" in the dashboard too.
The documentation always refers to "bindings" (i.e. KV bindings).
I think it takes time for new comers to understand what a "binding" is - at least it took time for me. At some point you get that you need to use bindings to access a KV, assets, ... But the next problem you have to solve is how to access the bindings from your code. The answer to this question is env which I think is counter intuitive.
I think that renaming env to bindings would be consistent and intuitive:
bindings.KV.get(...)
bindings.ASSETS.fetch(request)
...
This simple change would bring consistency across:
When I was thinking about this change I thought that it would be an improvement for all bindings except maybe for the vars binding which we name "Environment variables" in the documentation
At first env.MY_VAR sounds more intuitive that bindings.MY_VAR.
However I will show that while env.MY_VAR sounds intuitive, it is actually misleading creates confusion for our users. Using bindings(.MY_VAR) would alleviate confusion with process.env (when nodejs_compat is used). I will also propose a new way to improve the API in the Follow-up section.
Issues with the current implementation
Below are a couple examples where users (/me) got confused with the current APIs
@opennextjs/cloudflare
When I started working on the cache implementation for @opennextjs/cloudflare I was still confuses with env vs process.env.
// Set the environment variables// Cloudflare suggests to not override the process.env object but instead apply the values to itfor(const[key,value]ofObject.entries(env)){if(typeofvalue==="string"){process.env[key]=value;}}
And what @conico974 wrote here is great, exactly what's need to be done.
However as we where discussing this code, Nicolas commented:
should we also merge other types ? It may be useful for people using other stuff from cloudflare like D1, Durable object ....
But as explained in the previous section, "merging" would not work as bindings (which are objects) would be converted to strings.
Here again I think that renaming env to bindings would prevent some confusion and make it clearer that process.env and bindings are not directly related concepts.
Work needed
If we decide to rename env to bindings:
Update the workers code templates in wrangler to rename env to bindings
Update WorkerEntrypoint, ... to expose this.bindings (=this.env). This should be gated by a compatibility flag as it could conflict with a user defined bindings at build time - see workers.d.ts [1]
Settle on a name to replace "Environment variables" when referring to the vars binding
import { env } from "cloudflare:test"; - expose bindings
Nice to have
Create a Binding type in the workers types. Env becomes an alias of Binding. This change is not breaking, we can keep both (and deprecated Env)
Gradually update cf projects to rename env to bindings.
[1] While adding a bindings property on the entry point classes could be a breaking change, it is very low risk. First because it would only be breaking for users defining a bindings property on their derived class. Then it would not break the runtime behavior (the bindings on the derived class would hide the base bindings property. It would only break the build which is easy to fix by either renaming the user defined bindings prop or turn off a compatibility flag. We can provide a codemod in wrangler to automatically check if users define a binding property and automatically rename it.
Follow-up
When it stills make sense to merge the vars binding into process.env in Node compatibility mode.
binding.vars would need to be set to {} when we do not defined any variable
A workerd "Environment Variables" could be an object. In that case process.env.MY_OBJ would be set to "[object Object]". This is consistent with the Node behavior, but users would be able to keep the typeof value === "string" if they prefer.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
TL;DR
I have only been exposed to the Workers for a few months.
While I immediately had the feeling that Workers were a fantastic platform, I felt too much friction to get started with Workers. I can say that it took me ~6 months to get comfortable with the platform and only now I can start using all the features with confidence.
IMO there are quite a few things we could do to improve the DX.
This issue is about the name
env
inasync fetch(request, env, ctx)
orthis.env
in i.e. durable objectsI find
env
to be confusing when all the docs refer to "bindings".It's easy to mix
env
with:process.env
(whennodejs_compat
is used:)See opennextjs/opennextjs-cloudflare#125 and details further down.
My proposal would be to rename
env
tobindings
in the fetch handler:Existing documentation URL(s)
https://developers.cloudflare.com/workers/runtime-apis/handlers/fetch/#background and many more
Details
Dashboard
The dashboard mentions "bindings"
Documentation
The documentation always refers to "bindings" (i.e. KV bindings).
I think it takes time for new comers to understand what a "binding" is - at least it took time for me. At some point you get that you need to use bindings to access a KV, assets, ... But the next problem you have to solve is how to access the bindings from your code. The answer to this question is
env
which I think is counter intuitive.I think that renaming
env
tobindings
would be consistent and intuitive:bindings.KV.get(...)
bindings.ASSETS.fetch(request)
This simple change would bring consistency across:
When I was thinking about this change I thought that it would be an improvement for all bindings except maybe for the vars binding which we name "Environment variables" in the documentation
At first
env.MY_VAR
sounds more intuitive thatbindings.MY_VAR
.However I will show that while
env.MY_VAR
sounds intuitive, it is actually misleading creates confusion for our users. Usingbindings(.MY_VAR)
would alleviate confusion withprocess.env
(whennodejs_compat
is used). I will also propose a new way to improve the API in the Follow-up section.Issues with the current implementation
Below are a couple examples where users (/me) got confused with the current APIs
@opennextjs/cloudflare
When I started working on the cache implementation for
@opennextjs/cloudflare
I was still confuses withenv
vsprocess.env
.The code I wrote at that time is akin to
and it worked!
I only realized later that this should actually not work because
process.env
is aProxy
and all the keys should be strings:This behavior is reasonable as all properties of
process.env
in Node are strings.But then why my code was (/is) working? That's because of the following line in
workers.ts
I believe that we could avoid the confusion if
env
was namedbindings
-...bindings
would make clearer that we are mixing things.@opennextjs/aws
The aws implementation of Open Next has the following code:
And what @conico974 wrote here is great, exactly what's need to be done.
However as we where discussing this code, Nicolas commented:
But as explained in the previous section, "merging" would not work as bindings (which are objects) would be converted to strings.
Here again I think that renaming
env
tobindings
would prevent some confusion and make it clearer thatprocess.env
andbindings
are not directly related concepts.Work needed
If we decide to rename
env
tobindings
:env
tobindings
WorkerEntrypoint
, ... to exposethis.bindings
(=this.env
). This should be gated by a compatibility flag as it could conflict with a user definedbindings
at build time - see workers.d.ts [1]bindings
onPlatformProxy
import { env } from "cloudflare:test";
- expose bindingsNice to have
Binding
type in the workers types.Env
becomes an alias ofBinding
. This change is not breaking, we can keep both (and deprecated Env)env
tobindings
.[1] While adding a
bindings
property on the entry point classes could be a breaking change, it is very low risk. First because it would only be breaking for users defining abindings
property on their derived class. Then it would not break the runtime behavior (thebindings
on the derived class would hide the basebindings
property. It would only break the build which is easy to fix by either renaming the user definedbindings
prop or turn off a compatibility flag. We can provide a codemod in wrangler to automatically check if users define abinding
property and automatically rename it.Follow-up
When it stills make sense to merge the vars binding into
process.env
in Node compatibility mode.As of today the best way to do this is:
We could expose the "Environment Variables" on
bindings.vars
so that the code become:Notes:
binding.vars
would need to be set to{}
when we do not defined any variableprocess.env.MY_OBJ
would be set to"[object Object]"
. This is consistent with the Node behavior, but users would be able to keep thetypeof value === "string"
if they prefer.Thoughts?
/cc @IgorMinar @petebacondarwin @dario-piotrowicz @irvinebroque @jasnell @anonrig
(moved from cloudflare/cloudflare-docs#17953)
Beta Was this translation helpful? Give feedback.
All reactions