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

process.env is prefered over import.meta.env #115

Open
juliusmarminge opened this issue Jan 25, 2024 · 5 comments
Open

process.env is prefered over import.meta.env #115

juliusmarminge opened this issue Jan 25, 2024 · 5 comments

Comments

@juliusmarminge
Copy link

juliusmarminge commented Jan 25, 2024

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:

std-env/src/env.ts

Lines 5 to 10 in 7528b13

const _getEnv = (useShim?: boolean) =>
globalThis.process?.env ||
import.meta.env ||
globalThis.Deno?.env.toObject() ||
globalThis.__env__ ||
(useShim ? _envShim : globalThis);

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
Copy link
Author

cc @pi0 any thoughts?

@juliusmarminge
Copy link
Author

Opened one at astro too withastro/roadmap#853

@pi0 pi0 changed the title "bug": env not there for stuff like astro process.env is prefered over import.meta.env Jan 25, 2024
@pi0
Copy link
Member

pi0 commented Jan 25, 2024

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?

@juliusmarminge
Copy link
Author

What do you think about merging them?

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

@pi0
Copy link
Member

pi0 commented Jan 25, 2024

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 👍🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants