-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Load configuration at runtime instead of build time #813
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
Conversation
bc88929
to
69c509d
Compare
Instead of renaming the variables with There will be import adjustments, but it will be more intuitive. If secrets are made dynamic, then the docker images will stay "env agnostic" and can be built once. |
By no means an expert on Vite/Svelte, but here is my understanding: The problem is that Vite doesn't load environment variables unless they're prefixed with It does seem like it should be possible to manually load variables at build time to get around this, but I couldn't find a place to load them early enough that they got injected into |
I am no expert at all either, but both the documentation and this tutorial don't have anything particularly special: https://learn.svelte.dev/tutorial/env-dynamic-private So wouldn't expect renames, but changing the usage places and imports. |
089b9b1
to
abbd3ab
Compare
You are correct, I must have had something else miss-configured when I thought the prefix was required. Fixed! |
Admittedly, I couldn't quite get it to work myself either, so I feel your pain. (I tried updating just the mongodb vars, and with the dynamic vars it builds and runs locally just fine. |
TL;DR: You only need to fix the As a follow up to my previous comment, I actually made it work with relatively few changes. The only problem really was the bloody Only annoying thing was (which could just be me doing things wrong) that I expected the |
Just to clarify, did you need to change that in this PR? Or does it work for you as is? |
I had to do the following changes: In
And then I updated the import { MONGODB_URL, MONGODB_DB_NAME, MONGODB_DIRECT_CONNECTION } from "$env/static/private";
import { env } from "$env/dynamic/private";
// ...
const mongoUrl = env.MONGODB_URL || MONGODB_URL; // I expected vars to work like this already!
if (!mongoUrl) // throw ...
const client = new MongoClient(mongoUrl, {
directConnection: (env.MONGODB_DIRECT_CONNECTION ?? MONGODB_DIRECT_CONNECTION) === "true",
});
// ...
const db = client.db(
(env.MONGODB_DB_NAME ?? MONGODB_DB_NAME) + (import.meta.env.MODE === "test" ? "-test" : "")
);
// ... First part made it build ok. (I really don't understand why the code is actually executed at build time) |
This line isn't in this PR.
This is unnecessary with the |
Coming back to this with better understanding: The code gets executed at build time as per the Code Analysis discussed here: The Not throwing or attempting to connect at build time makes it build fine. But whether the correct environment values are used is a different matter. The $env module is not explained in great detail, but to summarise:
All of this makes it hard to provide env vars at runtime. To resolve all of this, I replaced all of the env var usages with try {
const { env: dynamicEnv } = await import("$env/dynamic/private");
const staticEnv = await import("$env/static/private");
// Iterate these objects as PUBLIC_ vars are split out already
Object.keys(staticEnv ?? {}).forEach(k => dynamicEnv[k] ??= process.env[k] ?? staticEnv[k])
// ^^^ SAME for public vars
} catch () {} Separately, I didn't want to update the usages EVERYWHERE in code, so I used destructuring right after the import statement: import { env } from "$env/dynamic/private";
const {
COOKIE_NAME,
EXPOSE_API,
MESSAGES_BEFORE_LOGIN
} = env; |
80feec4
to
07d2587
Compare
Moved the database check to runtime only. The one outstanding issue I've encountered with this PR is that it doesn't currently make it possible to use a dynamic |
I've abandoned my attempt to make Happy to provide my notes if anyone else wants to attempt but for now this PR just leaves |
This will make it possible to self contained docker images which can then be used in simple docker/kubernetes deployments.
After the dynamic env changes this remains the only thing that must be build time configured. Making this possible to change at runtime is very difficult and requires (at least) a major svelte version upgrade.
Hey! Will close this PR as we handled it with #1092. Sorry for the double work, we needed to figure it out for our infra on HuggingChat. 😅 Appreciate the work on this |
This will make it possible to publish self contained docker images which can then be used in simple docker/kubernetes deployments.
This is a breaking change,
in particular private environment variables must now have theusers will need to make sure environment variables are available at runtime.VITE_
prefix, andFixes #425