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

fix: do not freeze process.env in dev #12585

Merged
merged 10 commits into from
Dec 9, 2024
5 changes: 5 additions & 0 deletions .changeset/nasty-parrots-cry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes a case where `process.env` would be frozen despite changes made to environment variables in development
1 change: 0 additions & 1 deletion packages/astro/src/container/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ function createManifest(
i18n: manifest?.i18n,
checkOrigin: false,
middleware: manifest?.middleware ?? middlewareInstance,
envGetSecretEnabled: false,
key: createKey(),
};
}
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/core/app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ export type SSRManifest = {
i18n: SSRManifestI18n | undefined;
middleware?: () => Promise<AstroMiddlewareInstance> | AstroMiddlewareInstance;
checkOrigin: boolean;
envGetSecretEnabled: boolean;
};

export type SSRManifestI18n = {
Expand Down
3 changes: 0 additions & 3 deletions packages/astro/src/core/base-pipeline.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { setGetEnv } from '../env/runtime.js';
import { createI18nMiddleware } from '../i18n/middleware.js';
import type { ComponentInstance } from '../types/astro.js';
import type { MiddlewareHandler, RewritePayload } from '../types/public/common.js';
Expand All @@ -10,8 +9,6 @@ import type {
SSRResult,
} from '../types/public/internal.js';
import { createOriginCheckMiddleware } from './app/middlewares.js';
import { AstroError } from './errors/errors.js';
import { AstroErrorData } from './errors/index.js';
import type { Logger } from './logger/core.js';
import { NOOP_MIDDLEWARE_FN } from './middleware/noop-middleware.js';
import { sequence } from './middleware/sequence.js';
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,5 @@ function createBuildManifest(
checkOrigin:
(settings.config.security?.checkOrigin && settings.buildOutput === 'server') ?? false,
key,
envGetSecretEnabled: false,
};
}
3 changes: 0 additions & 3 deletions packages/astro/src/core/build/plugins/plugin-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,5 @@ function buildManifest(
(settings.config.security?.checkOrigin && settings.buildOutput === 'server') ?? false,
serverIslandNameMap: Array.from(settings.serverIslandNameMap),
key: encodedKey,
envGetSecretEnabled:
(unwrapSupportKind(settings.adapter?.supportedAstroFeatures.envGetSecret) ??
'unsupported') !== 'unsupported',
};
}
6 changes: 3 additions & 3 deletions packages/astro/src/env/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import type { ValidationResultInvalid } from './validators.js';
export { validateEnvVariable, getEnvFieldType } from './validators.js';

export type GetEnv = (key: string) => string | undefined;
type OnSetGetEnv = (reset: boolean) => void;
type OnSetGetEnv = () => void;

let _getEnv: GetEnv = (key) => process.env[key];

export function setGetEnv(fn: GetEnv, reset = false) {
export function setGetEnv(fn: GetEnv) {
_getEnv = fn;

_onSetGetEnv(reset);
_onSetGetEnv();
}

let _onSetGetEnv: OnSetGetEnv = () => {};
Expand Down
21 changes: 16 additions & 5 deletions packages/astro/src/env/vite-plugin-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,24 @@ interface AstroEnvPluginParams {

export function astroEnv({ settings, mode, sync }: AstroEnvPluginParams): Plugin {
const { schema, validateSecrets } = settings.config.env;
let isDev: boolean;

let templates: { client: string; server: string; internal: string } | null = null;

return {
name: 'astro-env-plugin',
enforce: 'pre',
config(_, { command }) {
isDev = command !== 'build';
},
buildStart() {
const loadedEnv = loadEnv(mode, fileURLToPath(settings.config.root), '');
for (const [key, value] of Object.entries(loadedEnv)) {
if (value !== undefined) {
process.env[key] = value;

if (!isDev) {
for (const [key, value] of Object.entries(loadedEnv)) {
if (value !== undefined) {
process.env[key] = value;
}
}
}

Expand All @@ -42,7 +49,7 @@ export function astroEnv({ settings, mode, sync }: AstroEnvPluginParams): Plugin
});

templates = {
...getTemplates(schema, validatedVariables),
...getTemplates(schema, validatedVariables, isDev ? loadedEnv : null),
internal: `export const schema = ${JSON.stringify(schema)};`,
};
},
Expand Down Expand Up @@ -122,6 +129,7 @@ function validatePublicVariables({
function getTemplates(
schema: EnvSchema,
validatedVariables: ReturnType<typeof validatePublicVariables>,
loadedEnv: Record<string, string> | null,
) {
let client = '';
let server = readFileSync(MODULE_TEMPLATE_URL, 'utf-8');
Expand All @@ -142,10 +150,13 @@ function getTemplates(
}

server += `export let ${key} = _internalGetSecret(${JSON.stringify(key)});\n`;
onSetGetEnv += `${key} = reset ? undefined : _internalGetSecret(${JSON.stringify(key)});\n`;
onSetGetEnv += `${key} = _internalGetSecret(${JSON.stringify(key)});\n`;
}

server = server.replace('// @@ON_SET_GET_ENV@@', onSetGetEnv);
if (loadedEnv) {
server = server.replace('// @@GET_ENV@@', `return (${JSON.stringify(loadedEnv)})[key]`);
}

return {
client,
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/vite-plugin-astro-server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest
i18n: i18nManifest,
checkOrigin:
(settings.config.security?.checkOrigin && settings.buildOutput === 'server') ?? false,
envGetSecretEnabled: false,
key: hasEnvironmentKey() ? getEnvironmentKey() : createKey(),
middleware() {
return {
Expand Down
14 changes: 8 additions & 6 deletions packages/astro/templates/env.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,19 @@
import { schema } from 'virtual:astro:env/internal';
import {
createInvalidVariablesError,
getEnv,
getEnv as _getEnv,
getEnvFieldType,
setOnSetGetEnv,
validateEnvVariable,
} from 'astro/env/runtime';

const getEnv = (key) => {
// @@GET_ENV@@
return _getEnv(key);
florian-lefebvre marked this conversation as resolved.
Show resolved Hide resolved
}

export const getSecret = (key) => {
return getEnv(key);
return getEnv(key)
};

const _internalGetSecret = (key) => {
Expand All @@ -25,9 +30,6 @@ const _internalGetSecret = (key) => {
throw createInvalidVariablesError(key, type, result);
};

// used while generating the virtual module
// biome-ignore lint/correctness/noUnusedFunctionParameters: `reset` is used by the generated code
// biome-ignore lint/correctness/noUnusedVariables: `reset` is used by the generated code
setOnSetGetEnv((reset) => {
setOnSetGetEnv(() => {
// @@ON_SET_GET_ENV@@
});
Loading