Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 BUG: process.env.NODE_ENV is not set to production by default #7886

Open
brillout opened this issue Jan 23, 2025 · 11 comments
Open

🐛 BUG: process.env.NODE_ENV is not set to production by default #7886

brillout opened this issue Jan 23, 2025 · 11 comments
Labels
bug Something that isn't working

Comments

@brillout
Copy link

Which Cloudflare product(s) does this pertain to?

Wrangler

What versions are you using?

3.102.0 [Wrangler]

What operating system and version are you using?

Ubuntu 24.04.1

Please provide a link to a minimal reproduction

No response

Describe the Bug

If I set the following:

# wrangler.toml

[vars]
NODE_ENV = "production"

Then I expect wrangler to replace process.env.NODE_ENV with 'production'. It's a ubiquitous JavaScript convention.

I also tried the following, but it doesn't work either.

# wrangler.toml

define = { "process.env.NODE_ENV" = "production" }

I can't dig why it doesn't work as it doesn't seem possible to inspect the worker code as per #7885.

Please provide any relevant error logs

No response

@brillout brillout added the bug Something that isn't working label Jan 23, 2025
@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Jan 23, 2025
@vicb
Copy link
Contributor

vicb commented Jan 24, 2025

# wrangler.toml

[vars]
NODE_ENV = "production"

Makes NODE_ENV available on env (as in fetch(request, env, ctx)) - it is not the same as process.env

See https://developers.cloudflare.com/workers/runtime-apis/nodejs/process/#processenv for details

# wrangler.toml

define = { "process.env.NODE_ENV" = "production" }

I am not sure what define is used for, maybe @petebacondarwin knows?

@brillout
Copy link
Author

brillout commented Jan 24, 2025

Is it even possible to set the process.env.NODE_ENV value then?

In my specific case, Vike shows a warning if it isn't 'production':

[vike][Warning] The environment is set to be a development environment by process.env.NODE_ENV==="undefined", is contradictory because the environment seems to be a production environment (Vite isn't loaded), see https://vike.dev/NODE_ENV

This is where Vike reads process.env.NODE_ENV: https://github.com/vikejs/vike/blob/bb8341b7144f1e9a1e9fa47a916042cb94dbd76a/vike/utils/assertSetup.ts#L141.

@vicb
Copy link
Contributor

vicb commented Jan 24, 2025

Is it even possible to set the process.env.NODE_ENV value then?

You can have process.env.NODE_ENV = env.NODE_ENV; in your code.
Note that as env in only available in the request context, top-level code might see process.env.NODE_ENV === undefined

Ideally we should support defining process.env.NODE_ENV in wrangler.json, I'm not sure if it is the intent of the define entry in the configuration file but if it is it looks like it doesn't work

@brillout
Copy link
Author

Yes, ideally wrangler should automatically set process.env.NODE_ENV (regardess of the Node.js compat flag). Many libraries need this, for example React: https://github.com/facebook/react/blob/01ab35a9a731dec69995fbd28f3ac7eaad11e183/packages/react/npm/index.js.

You can have process.env.NODE_ENV = env.NODE_ENV; in your code.

👍 We'll try that in our vike-cloudflare integration.

@vicb
Copy link
Contributor

vicb commented Jan 24, 2025

Yes, ideally wrangler should automatically set process.env.NODE_ENV

There are internal discussion around this - stay tuned.

I was looking at out code and what we are supposed to do is to set process.env.NODE_ENV as build time (without you having to add define in wrangler). This obviously doesn't work well...

@brillout
Copy link
Author

Maybe it's an issue on our side though, is there any way I can have a look at the generated worker code? That would help a ton. Even better would be able to modify it and then run this modified version.

@vicb
Copy link
Contributor

vicb commented Jan 24, 2025

No, it's a few issues on wrangler side.

We have this code:

...(process.env.NODE_ENV && {
define: {
...(defineNavigatorUserAgent
? { "navigator.userAgent": `"Cloudflare-Workers"` }
: {}),
// use process.env["NODE_ENV" + ""] so that esbuild doesn't replace it
// when we do a build of wrangler. (re: https://github.com/cloudflare/workers-sdk/issues/1477)
"process.env.NODE_ENV": `"${process.env["NODE_ENV" + ""]}"`,
...(nodejsCompatMode === "legacy" ? { global: "globalThis" } : {}),
...define,
},
}),

There are a few issues here:

  1. We check process.env.NODE_ENV while it should be process.env["NODE_ENV" + ""]
  2. This faulty check is used to guard the whole define while it should only be used for process.env.NODE_ENV
  3. Lastly process.env.WHATEVER will be replaced by something like cloudflare_default2.env.WHATEVER by the hybrid plygin / unenv so even if 1 and are fixed it would not be applied.
  4. The code is not tested

I believe 1 and 2 could be fixed via:

// use process.env["NODE_ENV" + ""] so that esbuild doesn't replace it 
// when we do a build of wrangler. (re: https://github.com/cloudflare/workers-sdk/issues/1477) 
const runtimeNodeEnv = process.env["NODE_ENV" + ""];

const buildOptions = {
  // ...
  define: { 
    ...(defineNavigatorUserAgent ? { "navigator.userAgent": `"Cloudflare-Workers"` } : {}),
    ...(runtimeNodeEnv ? {"process.env.NODE_ENV": `"${runtimeNodeEnv}"`} : {}),
    ...(nodejsCompatMode === "legacy" ? { global: "globalThis" } : {}), 
    ...define, 
  }, 

Not sure how to fix 3.

@penalosa or @petebacondarwin would you have time to take a look or find someone that can?

@vicb vicb changed the title 🐛 BUG: process.env.NODE_ENV is null but should be 'production' instead. 🐛 BUG: wrangler define is broken - fails to replace process.env.NODE_ENV Jan 24, 2025
@vicb vicb changed the title 🐛 BUG: wrangler define is broken - fails to replace process.env.NODE_ENV 🐛 BUG: 🚨 wrangler define is broken 🚨 - fails to replace process.env.NODE_ENV Jan 24, 2025
brillout added a commit to vikejs/vike-cloudflare that referenced this issue Jan 24, 2025
@brillout
Copy link
Author

I can confirm it's a wrangler bug: it replaces process.env.NODE_ENV with the string "undefined". Workaround: vikejs/vike-cloudflare@e99537b.

@penalosa
Copy link
Contributor

@brillout you should be able to get Wrangler to statically replace process.env.NODE_ENV in your worker by providing the NODE_ENV environment variable. I agree that not replacing with production by default is perhaps unexpected, but currently Wrangler leans towards being more explicit here.

You can also use the define config, if you'd prefer (note the extra quotes, as it's a JSON encoded value):

[define]
"process.env.NODE_ENV" = "'production'"

@vicb I don't think (3) is accurate—this seems to work for me even with nodejs_compat turned on

@penalosa penalosa changed the title 🐛 BUG: 🚨 wrangler define is broken 🚨 - fails to replace process.env.NODE_ENV 🐛 BUG: process.env.NODE_ENV is not set to production by default Jan 24, 2025
@vicb
Copy link
Contributor

vicb commented Jan 24, 2025

@penalosa do you use the global or import process from "node:process"; - I think the later will fail, not sure about the former.

@vicb
Copy link
Contributor

vicb commented Jan 24, 2025

I can confirm that the global works but not the import 🚨

So I think we need to fix 1 - 4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working
Projects
Status: Untriaged
Development

No branches or pull requests

3 participants