-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Consistent handling of environment variables #4296
Comments
For dynamic-public env vars, I had a usecase for it and lightly touched on it before. But the gist is that a frontend would connect to a backend URL, which can be public, but the backend URL could change for e.g. testing purposes, or it's not known in build time. But that said, it should be a userland concern, and isn't something SvelteKit/Vite has to handle IMO. |
The current recommendation (according to the FAQ) is to use a getSession hook But using Idea, part 1 (AdapterEnv) import { env } from "$app/adapter";
console.log(env.TRACKING_ID); The values inside the env object will be provided by the adapter, in case of adapter-node and the dev server it will be the The values can be used to store secrets such as connections string and are therefor only are available on the server. The "$app/adapter" import would also allow adapters to expose emulation for features in development like Cloudflare KV, but that's another topic Idea, part 2 (env Hook) To expose variables to client code we could add a in hooks.js: export function getPublicEnv(env: AdapterEnv) {
return {
API_ENDPOINT: env.API_ENDPOINT,
TRACKING_ID: env.TRACKING_ID,
};
} The getPublicEnv hook is executed only once at startup, after that the values are set and can be imported from anywhere. (no context needed) Idea, part 3 (generated exports) To access the public environment variables you'd write: import { API_ENDPOINT } from "$app/env"; Using Summary
|
Another dimension along which we're currently inconsistent: #3040. Vite loads
... |
@Rich-Harris, I'm hugely passionate about this one. Maybe it's because I was talking about it and frustrated by it even back in June of last year... I encourage you to scroll through that conversation for context about how confusing this is to someone new to the ecosystem -- the fact that Vite statically replaces Some notes, in no particular order: I'm a TypeScript-first user, so generating types based on the As far as API goes, if I could have my way, it'd look like this: import { env } from '$app/buildtime';
import { privateEnv } from '$app/buildtime';
import { env } from '$app/runtime';
import { privateEnv } from '$app/runtime'; Whatever solution we build should be able to guarantee that no properties on the declare namespace App {
interface Environment {
buildtime: {
env: Record<string, string>,
privateEnv: Record<string, string>,
}
runtime: {
env: Record<string, string>,
privateEnv: Record<string, string>,
}
} As a bonus, future feature, we may be able to have Kit validate that the expected keys appear in This isn't meant to be a complete proposal, per se, but more just to try to shuffle the conversation in a direction I think might be beneficial. Heck, it'd certainly be beneficial to me. Once the team can decide on a full design spec, I'd be happy to help implement this. |
@tcc-sejohnson I strongly agree with being clear about which variables are being exposed in the client. I started coding in the backend, so the security concerns were rarely about mistakenly exposing a private-key to the client. And, whenever I worked with the frontend I was careful to always use a proxy API or something similar. Since sveltekit has both client side and server side code it does make me very worried that I’ll expose something by accident. If it’s super clear what’s being used where I think that’s a major feature of the framework. |
Hey @Rich-Harris, Like I said, I'm pretty passionate about this one -- so here's a full design proposal to get people talking. To restate the points Rich laid out in the original comment, a solution needs to:
After thinking about it for some time, I think it should also:
Let's talk about those. Be composable to some degreeThe immediate use-case I can think of is when my app needs to know an environment-dependent URL for an api:
Whenever I need to reference this URL within my code, I often need to change behavior based on whether I'm operating in HTTP/HTTPS. Sometimes I need to know the port. Sometimes I just need the host, etc. Sure, I can just
There are other instances where I'd like composability (or just the ability to compose a JavaScript object out of a string config value without having to do so at every single line that uses the config value). More on this in the design section. 😺 Guarantee no client exposure of private variablesPretty simple. Obviously, Kit can't guarantee you don't just create a GET endpoint that returns MY_PRIVATE_VARIABLE to the frontend, but it should be able to assert that accessing a private variable from the client is invalid. Provide the ability to validate config at startup/buildtimeI've wasted more time than I probably should by forgetting to include a new environment variable in .env and then having to track down why my credentialed API call isn't working. Provide a relatively easy-to-follow execution pathThere are a lot of complicated ways to implement something like this. Let's not do one of them. DesignI propose the addition of a few new exports from the With that summary, let's follow the "execution flow" and describe in more detail how this would work.
|
@Rich-Harris can we borrrow a leaf here https://docs.astro.build/en/guides/environment-variables/ instead of letting this drag for so long . import.meta.env.WHATEVER works on Server and client side in Astro. BUT only environment variables prefixed with PUBLIC_ are available client side .note they also available server side so that code running on client and server have the same mode of extracting environment variables. I am porting a small app to astro edge functions given how long this is taking to get stamped |
Unfortunately this would only solve a very small part of the problem. Vite variables are only available at build time (not runtime), so while this may work for static sites, if the app needs any runtime variables, it would still have to try to use some messy version of Just a quick note about switching to Astro: Remember that SvelteKit is still beta software and the maintainers are being careful to balance getting to 1.0 with making good, long-term decisions about how to implement features. If you need a production-ready site right now, you should definitely use a production-ready software, as SvelteKit will experience breaking changes without warning, probably multiple times before 1.0. |
@tcc-sejohnson but do we even have the above to give unified access at Build time? If one can read environment variables from .env files in a unified manner without the concern at least of whether a module will run on just the server or server and client WRT environment variables, that will be helpful to most folks. and the conversation /suggestions so far can focus on runtime variables. All the suggestions so far speak more to dealing with runtime variables and not the static variables read from .env files. It will not only solve a small part of the problem as you said but a significant bit. There are more build time variables than run time in any application. If this were in place , i can deal with runtime variables with process.env i know at least that it is a runtime entry |
Keep in mind, I'm not a maintainer, just a humble contributor working where I can help. But look at it this way: Kit is opinionated and, where possible, it likes to provide a complete, unified solution to a problem. If we were to implement a partial solution, it could actually get in the way of a complete solution later, requiring a rewrite (and associated breaking changes). Given that we're pre-1.0 and there are no time constraints around releasing features, I'd strongly vote in favor of finding a complete solution to the problem rather than hipfiring a half-baked solution just to suit a specific use case. 🤷 |
i do also find it rather difficult to make "runtime" environment variables work. tl;dr: This works fine in DEV mode, however in order for the production deployment to work, we have to use all used environment variables already during the build time since vite replaces them with static strings -> regardless of the way we access them in the hooks.js ( What i am trying to achieve:
https://discord.com/channels/457912077277855764/988390637072162876/988397666016833596 |
This process is confusing enough that I'd be in favor of finding a way to block Vite environment variables entirely once we've implemented our solution -- from a JavaScript perspective, |
@tcc-sejohnson thank you so much for the confirmation that it should actually work. in fact i do not even need to use any .env file at all, running the node build with set environment variables works just fine without it too. the issue i ran into was a different one actually that masked itself as a process.env issue -> idk if its already one but calling the basepath of the node-build does not call |
i retract that statement, this happened because i was having an prerender true export in my index.svelte start page, which obviously doesn't need to call a hook |
Just a little note to everyone confused by the fact that dotenv is not working within Edit: |
There are three axes along which env variables vary: mode (dev/prod/etc.), visibility (client/server), build vs runtime We should review these to see which combinations work or not. I think there may be some things in Vite that need to be fixed like vitejs/vite#6626 |
We've been talking about this in the maintainers' chat and I have a proposal that I think covers all the bases we care about. First off, I don't think we need to care about A corollary is that we don't need to care about the mode axis. We only care about two axes — private/public and static/dynamic. Vite covers the public/static quadrant, but does so in a way that is frankly a bit clunky. My proposal: // env vars set at build time and statically replaced
import { PRIVATE_TOKEN } from '$app/env/private';
import { PUBLIC_BASE_URL } from '$app/env';
export function GET({ env }) {
return fetch(`${PUBLIC_BASE_URL}/stuff?theme=${env.THEME}`, {
headers: {
authorization: `Bearer ${PRIVATE_TOKEN}`
}
});
}
|
Nice to see the adapter standardization for runtime environment variables and the autocompletion improvements for the build-time environments. We write apps using the twelve-factor app methodology, therefore we create one build that deploys to all environments, so we also won't be able to use the cool autocompletion features :( Too bad that runtime is made difficult due to Cloudflare's env per request setup. |
Hmm. I'd imagined that I wonder if we could solve this another way. Since the built SvelteKit server doesn't import any of your code until the adapter calls // inside adapter code (SvelteKit users would never see this unless they were building an adapter)
const server = new Server(manifest);
const worker = {
async fetch(req, env, context) {
server.init({ env }); // calling this a second time is a noop
// ...
return server.respond(...);
}
}; ...which would make // src/hooks.js
import { env } from '$app/env/runtime';
const db = await connect(env.DATABASE_URL); // etc
// ... Adapters that don't have the same design as Cloudflare Workers could call server.init({
env: process.env
}); or whatever when they first boot up. |
(To be clear, I'm not suggesting any changes to how static variables are handled, just dynamic ones. And the names of all these modules are open to bikeshedding.) |
This comment was marked as outdated.
This comment was marked as outdated.
For everyone following along, there's now a PR open for this (thanks @tcc-sejohnson): #5663 If anyone wants to offer feedback on the design we landed on, now is the time. I think it solves the various problems pretty well — I've been trying it out locally and enjoying it a lot. |
Not sure if this is of any use since it's already shipped, but just wanted to give some feedback that I migrated a large project over to the new env variable handling, and I liked it a lot other than the I think just having |
Describe the problem
SvelteKit doesn't do anything special with environment variables; it delegates to Vite. This is good for the sorts of environment variables Vite deals in —
VITE_
prefixed values that are replaced at build time — but less good for other kinds.Environment variables can be static (known at build time) or dynamic (not known until run time), and private (only accessible to code that runs on the server) or public (accessible to code that runs on the server or the client):
.env
files and configprocess.env
is only available in Node-based environments (e.g.adapter-node
,adapter-netlify
andadapter-vercel
— though once the Vercel adapter starts building Edge Functions that will no longer be the case)It would be great if we had a consistent and idiomatic way of dealing with all of these.
Describe the proposed solution
I don't actually have any proposals to make, I just didn't want #4293 (comment) to get lost. Whatever solution we land on should
.env
/.env.*
files.env
file)$app/env/public
vs$app/env/private
or whatever)Alternatives considered
No response
Importance
would make my life easier
Additional Information
No response
The text was updated successfully, but these errors were encountered: