vitejs / vite

Next generation frontend tooling. It's fast!
http://vite.dev
MIT License
68.57k stars 6.19k forks source link

Simplify environment variable handling #11685

Open manucorporat opened 1 year ago

manucorporat commented 1 year ago

Description

Today, variable environment handling in Vite is made of the following moving parts:

The current state of the art, relies on import.meta.env and process.env (a Node-only API) to correctly (and securely) integrate environment variables in a vite-powered application.

Variables starting with VITE_ will be included in import.meta.env in both client and server (SSR) build, while variable without the prefix will be included also be included in process.env only in the server build.

I want to argue that while the docs could be improved, the API is inheritely confusing, we have seen developers at qwik include their secrets in a environment variable prefixed with VITE_ because they thought import.meta.env was the only API available to access variables.

Suggested solution

  1. Simplify the decision making: process.env is deprecated and never recommended
  2. Rename VITE_ to PUBLIC_, so it states clearly the intention of the prefix, VITE while on-brand, it does not contain semantic.
  3. Change the behaviour of import.meta.env for SSR build:
    • During SSR build, import.meta.env will include ALL the environment variables, with and without the PUBLIC_ prefix
    • During browser build: import.meta.env will include ONLY the environment variables with the PUBLIC_ prefix (like it does currently)

This way, there is no need for process.env in modern source code, and the cognitive load is lower for developers since there is a single recommended API to get the variables import.meta.env and a clear intention for variables supposed to be public and therefore included in the browser build.

Alternative

No response

Additional context

No response

Validations

dominikg commented 1 year ago

additional information:

sveltekit offers a different way to import environment variables that ensures private env isn't leaked to the client build. https://kit.svelte.dev/docs/server-only-modules#private-environment-variables

I made an experiment a while ago where a vite plugin is used to offer similar protections in a more generic way, albeit with slightly different DX (in kit you can import env, in that plugin you have to import each var)

https://github.com/svitejs/vite-plugin-env-import/tree/main/packages/vite-plugin-env-import

benmccann commented 1 year ago

I'd get rid of the import.meta.env built-ins because they don't work in library mode. If you build a library using import.meta.env and then distribute it, it will not work with other bundlers. I built esm-env as a replacement we use in SvelteKit which works both for libraries and apps

I agree with the VITE_ to PUBLIC_ rename

process.env can be nice because it's set dynamically at runtime whereas import.meta.env is replaced dynamically at build time. We need both as there are cases where one or the other must be preferred. SvelteKit made this dynamic vs static usage more explicit, which we may be able to do in Vite as well with an approach similar to the one Dominik shared

manucorporat commented 1 year ago

I understand it gets replaces at build time when building the library, no?, process.env is a node-only idiom too. Sveltekit solution is interesting, are those server-only files commited? one of the nice things about the .env files is that it's well-known to not be committed

WyvernDrexx commented 1 year ago

We can show the depreciation warning to Vite users to not use process.env still we can copy the envs into both process.env and import.meta.env .

manucorporat commented 1 year ago

The problem with not using process.env is that it's the only way TODAY to keep secret environment variables private!

manucorporat commented 1 year ago

btw @benmccann in Qwik, we also have a similar solution for isServer, isBrowser:

import {isServer} from '@builder.io/qwik/build';

i was thinking more of custom environment variables, Sveltekit solution with modules is very interesting though

WyvernDrexx commented 1 year ago

I think to remove the confusion on which envs gets loaded in which environment, we can instead add CLIENT_ prefix, which will be loaded only on the client build, while SERVER_ prefix will be loaded only on server builds. AND, the PUBLIC_ prefix will be loaded on both client and server.

benmccann commented 1 year ago

Yes, good point about process.env being Node-only. The way we handle that in SvelteKit is by allowing the adapter for each hosting platform to pass in the environment variables. E.g. here's where we do it in our Vercel adapter:

https://github.com/sveltejs/kit/blob/b6c238a7f137b49e4f24746696e592103e1c3aed/packages/adapter-vercel/files/serverless.js#L11

I'm not sure how that would work outside a meta framework. Perhaps Vite could define a function like getEnv that each framework would need to provide an implementation of or something. But I'm not sure there's a whole lot of benefit of standardizing that across frameworks and it seems just as well that this is something that's implemented by each framework.

The other interesting thing SvelteKit does is make sure you don't access any private env variables from public-facing code. You can only reference them if server is in the directory or file name. Potentially that could be refactored out into a Vite plugin which could be shared across projects.

achaphiv commented 1 year ago

I find the VITE_ prefixing to be quite annoying.

In my case, I wanted to show the git sha1/branch to easily tell what version is deployed. By default this required re-exporting the CI environment variables.

export VITE_VERCEL_GITHUB_COMMIT_REF=$VERCEL_GITHUB_COMMIT_REF
export VITE_VERCEL_GITHUB_COMMIT_SHA=$VERCEL_GITHUB_COMMIT_SHA

Changing the prefix to PUBLIC_ doesn't really help in this case. It's still a lot of error-prone copy/pasting.

Additionally, I would sometimes introduce a import.meta.env.ABC in a typescript file, but forget to update the build script. So it would silently fail.

My current solution is to centralize all client-only environment variables to one file.

client-env-vars.ts

export const VERCEL_GITHUB_COMMIT_REF = process.env.VERCEL_GITHUB_COMMIT_REF
export const VERCEL_GITHUB_COMMIT_SHA = process.env.VERCEL_GITHUB_COMMIT_SHA

Components have to go through this file to use environment variables.

DebugPage.vue

import { VERCEL_GITHUB_COMMIT_REF, VERCEL_GITHUB_COMMIT_SHA } from '../client-env-vars'

Vite directly imports this file and replaces the process.env.* via define:

vite.config.ts

import * as envVars from './client-env-vars'

const define: Record<string, string | undefined> = {}
for (const [key, value] of Object.entries(envVars)) {
  define[`process.env.${key}`] = JSON.stringify(value)
}

export default defineConfig({
  define
})

I don't know what the best solution is, but I think ideally:

vimfn commented 8 months ago

I appreciate the diverse perspectives. I would like to add I find the convenience of @t3-oss/env-nextjs or @t3-oss/env-core very commendable as well. It ensures type-safety for all environment variables.