std-env icon indicating copy to clipboard operation
std-env copied to clipboard

`process.env` is prefered over `import.meta.env`

Open juliusmarminge opened this issue 2 years ago • 5 comments

Environment

node 20.11 std-env 3.7

Reproduction

not really relevant

Describe the bug

So I'm not sure if I'd call this a bug per-se, maybe more of an observation to open up a discussion how we could solve this.

Astro is a bit (if you ask me) dumb in the way that they have a process.env on the server but they don't populate it with stuff from your env file. That stuff goes on import.meta.env. This means that the following logs undefined:

import { process } from "std-env";
console.log(process.env.SOME_ENV_FROM_ENVFILE);

This is cause (like I said), process does exists, so std-env chooses it as the env object: https://github.com/unjs/std-env/blob/7528b1324ab48dea93c0739b0b046e0d5607fcf5/src/env.ts#L5-L10

Any ideas how to solve this in a nice way? I think this is just a stupid behavior from Astro but any workaround we can do to support this would be great. We recently moved to std-env in uploadthing to have a single source of truth for accessing environment variables but then I stumbled on this edge-case...

Additional context

No response

Logs

process.env {
  MANPATH: '/opt/homebrew/share/man::',
  COREPACK_ROOT: '/Users/julius/Library/Application Support/fnm/node-versions/v20.11.0/installation/lib/node_modules/corepack',
  BUNCH_OF_STUFF: "THAT I REMOVED",
  NODE_ENV: 'development'
}
import.meta.env {
  BASE_URL: '/',
  MODE: 'development',
  DEV: true,
  PROD: false,
  SSR: true,
  SITE: undefined,
  ASSETS_PREFIX: undefined,
  UPLOADTHING_SECRET: 'sk_live_xxxxxx' // <-- this is coming from .env.local
}

juliusmarminge avatar Jan 25 '24 14:01 juliusmarminge

cc @pi0 any thoughts?

juliusmarminge avatar Jan 25 '24 14:01 juliusmarminge

Opened one at astro too https://github.com/withastro/roadmap/issues/853

juliusmarminge avatar Jan 25 '24 14:01 juliusmarminge

Thanks for explaining the issue! Well this is interesting. I will followup your upstream issue.

It is i guess more of a question that for std-env we shall prefer import.meta.env over process.env or not. From other discussions and also considering the breaking changes risk it can bring to the ecosystem (at least UnJs/Nitro are safe. We have both and they are the same) it is probably not something we should do and strange enough about Vite side..

Also as mentioned in astro's discussion, if Astro (or even Vite) needs a special logic, i am open to it.

Do you think we should still track it here for std-env too?

pi0 avatar Jan 25 '24 15:01 pi0

What do you think about merging them?

So like Object.assign({}, process.env, import.meta.env) (with safeguards since one might not exist?

juliusmarminge avatar Jan 25 '24 16:01 juliusmarminge

We can...But it also costs. Also makes another issue. If there is a later change to process.env or import.meta.env objects, we do lose them (imagine in runtime a code setting process.env.FOOBAR -- I know many places do this)

One other thing we can do is that inside proxy, we iterate over sources and check each if exists and key exists in it. I think it is an acceptable compromise and middle-ground to fix the issue. Basically _getEnv(shim) to be replaced with _getEnvVar(shim, key) :D Feel free to make a PR if you like to try this 👍🏼

pi0 avatar Jan 25 '24 16:01 pi0