feat(astro): address astro env rfc feedback (#11213)

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>

Author: florian-lefebvre

.changeset/clever-jars-trade.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Removes the `PUBLIC_` prefix constraint for `astro:env` public variables

.changeset/dirty-rabbits-act.md

@@ -0,0 +1,15 @@
+---
+'astro': patch
+---
+
+**BREAKING CHANGE to the experimental `astro:env` feature only**
+
+Server secrets specified in the schema must now be imported from `astro:env/server`. Using `getSecret()` is no longer required to use these environment variables in your schema:
+
+```diff
+- import { getSecret } from 'astro:env/server'
+- const API_SECRET = getSecret("API_SECRET")
++ import { API_SECRET } from 'astro:env/server'
+```
+
+Note that using `getSecret()` with these keys is still possible, but no longer involves any special handling and the raw value will be returned, just like retrieving secrets not specified in your schema.

packages/astro/src/@types/astro.ts

@@ -2070,17 +2070,17 @@ export interface AstroUserConfig {
 		 *
 		 * ```astro
 		 * ---
-		 * import { PUBLIC_APP_ID } from "astro:env/client"
-		 * import { PUBLIC_API_URL, getSecret } from "astro:env/server"
-		 * const API_TOKEN = getSecret("API_TOKEN")
+		 * import { APP_ID } from "astro:env/client"
+		 * import { API_URL, API_TOKEN, getSecret } from "astro:env/server"
+		 * const NODE_ENV = getSecret("NODE_ENV")
 		 *
-		 * const data = await fetch(`${PUBLIC_API_URL}/users`, {
+		 * const data = await fetch(`${API_URL}/users`, {
 		 * 	method: "POST",
 		 * 	headers: {
 		 * 		"Content-Type": "application/json",
 		 * 		"Authorization": `Bearer ${API_TOKEN}`
 		 * 	},
-		 * 	body: JSON.stringify({ appId: PUBLIC_APP_ID })
+		 * 	body: JSON.stringify({ appId: APP_ID, nodeEnv: NODE_ENV })
 		 * })
 		 * ---
 		 * ```
@@ -2095,37 +2095,36 @@ export interface AstroUserConfig {
 		 *     experimental: {
 		 *         env: {
 		 *             schema: {
-		 *                 PUBLIC_API_URL: envField.string({ context: "client", access: "public", optional: true }),
-		 *                 PUBLIC_PORT: envField.number({ context: "server", access: "public", default: 4321 }),
+		 *                 API_URL: envField.string({ context: "client", access: "public", optional: true }),
+		 *                 PORT: envField.number({ context: "server", access: "public", default: 4321 }),
 		 *                 API_SECRET: envField.string({ context: "server", access: "secret" }),
 		 *             }
 		 *         }
 		 *     }
 		 * })
 		 * ```
 		 *
-		 * There are currently three data types supported: strings, numbers and booleans.
+		 * There are currently four data types supported: strings, numbers, booleans and enums.
 		 *
 		 * There are three kinds of environment variables, determined by the combination of `context` (client or server) and `access` (secret or public) settings defined in your [`env.schema`](#experimentalenvschema):
 		 *
 		 * - **Public client variables**: These variables end up in both your final client and server bundles, and can be accessed from both client and server through the `astro:env/client` module:
 		 *
 		 *     ```js
-		 *     import { PUBLIC_API_URL } from "astro:env/client"
+		 *     import { API_URL } from "astro:env/client"
 		 *     ```
 		 *
 		 * - **Public server variables**: These variables end up in your final server bundle and can be accessed on the server through the `astro:env/server` module:
 		 *
 		 *     ```js
-		 *     import { PUBLIC_PORT } from "astro:env/server"
+		 *     import { PORT } from "astro:env/server"
 		 *     ```
 		 *
-		 * - **Secret server variables**: These variables are not part of your final bundle and can be accessed on the server through the `getSecret()` helper function available from the `astro:env/server` module:
+		 * - **Secret server variables**: These variables are not part of your final bundle and can be accessed on the server through the `astro:env/server` module. The `getSecret()` helper function can be used to retrieve secrets not specified in the schema:
 		 *
 		 *     ```js
-		 *     import { getSecret } from "astro:env/server"
+		 *     import { API_SECRET, getSecret } from "astro:env/server"
 		 *
-		 *     const API_SECRET = getSecret("API_SECRET") // typed
 		 *     const SECRET_NOT_IN_SCHEMA = getSecret("SECRET_NOT_IN_SCHEMA") // string | undefined
 		 *     ```
 		 *
@@ -2152,8 +2151,8 @@ export interface AstroUserConfig {
 			 *   experimental: {
 			 *     env: {
 			 *       schema: {
-			 *         PUBLIC_API_URL: envField.string({ context: "client", access: "public", optional: true }),
-			 *         PUBLIC_PORT: envField.number({ context: "server", access: "public", default: 4321 }),
+			 *         API_URL: envField.string({ context: "client", access: "public", optional: true }),
+			 *         PORT: envField.number({ context: "server", access: "public", default: 4321 }),
 			 *         API_SECRET: envField.string({ context: "server", access: "secret" }),
 			 *       }
 			 *     }

packages/astro/src/core/base-pipeline.ts

@@ -66,7 +66,7 @@ export abstract class Pipeline {
 		if (callSetGetEnv && manifest.experimentalEnvGetSecretEnabled) {
 			setGetEnv(() => {
 				throw new AstroError(AstroErrorData.EnvUnsupportedGetSecret);
-			});
+			}, true);
 		}
 	}
 

packages/astro/src/env/constants.ts

@@ -5,7 +5,6 @@ export const VIRTUAL_MODULES_IDS = {
 };
 export const VIRTUAL_MODULES_IDS_VALUES = new Set(Object.values(VIRTUAL_MODULES_IDS));
 
-export const PUBLIC_PREFIX = 'PUBLIC_';
 export const ENV_TYPES_FILE = 'env.d.ts';
 
 const PKG_BASE = new URL('../../', import.meta.url);

packages/astro/src/env/runtime.ts

@@ -5,8 +5,16 @@ export type GetEnv = (key: string) => string | undefined;
 
 let _getEnv: GetEnv = (key) => process.env[key];
 
-export function setGetEnv(fn: GetEnv) {
+export function setGetEnv(fn: GetEnv, reset = false) {
 	_getEnv = fn;
+
+	_onSetGetEnv(reset);
+}
+
+let _onSetGetEnv = (reset: boolean) => {};
+
+export function setOnSetGetEnv(fn: typeof _onSetGetEnv) {
+	_onSetGetEnv = fn;
 }
 
 export function getEnv(...args: Parameters<GetEnv>) {

packages/astro/src/env/schema.ts

@@ -1,5 +1,4 @@
 import { z } from 'zod';
-import { PUBLIC_PREFIX } from './constants.js';
 
 const StringSchema = z.object({
 	type: z.literal('string'),
@@ -84,29 +83,12 @@ const EnvFieldMetadata = z.union([
 
 const KEY_REGEX = /^[A-Z_]+$/;
 
-export const EnvSchema = z
-	.record(
-		z.string().regex(KEY_REGEX, {
-			message: 'A valid variable name can only contain uppercase letters and underscores.',
-		}),
-		z.intersection(EnvFieldMetadata, EnvFieldType)
-	)
-	.superRefine((schema, ctx) => {
-		for (const [key, value] of Object.entries(schema)) {
-			if (key.startsWith(PUBLIC_PREFIX) && value.access !== 'public') {
-				ctx.addIssue({
-					code: z.ZodIssueCode.custom,
-					message: `An environment variable whose name is prefixed by "${PUBLIC_PREFIX}" must be public.`,
-				});
-			}
-			if (value.access === 'public' && !key.startsWith(PUBLIC_PREFIX)) {
-				ctx.addIssue({
-					code: z.ZodIssueCode.custom,
-					message: `An environment variable that is public must have a name prefixed by "${PUBLIC_PREFIX}".`,
-				});
-			}
-		}
-	});
+export const EnvSchema = z.record(
+	z.string().regex(KEY_REGEX, {
+		message: 'A valid variable name can only contain uppercase letters and underscores.',
+	}),
+	z.intersection(EnvFieldMetadata, EnvFieldType)
+);
 
 // https://www.totaltypescript.com/concepts/the-prettify-helper
 type Prettify<T> = {

packages/astro/src/env/vite-plugin-env.ts

@@ -67,9 +67,8 @@ export function astroEnv({
 				fs,
 				content: getDts({
 					fs,
-					clientPublic: clientTemplates.types,
-					serverPublic: serverTemplates.types.public,
-					serverSecret: serverTemplates.types.secret,
+					client: clientTemplates.types,
+					server: serverTemplates.types,
 				}),
 			});
 		},
@@ -154,22 +153,17 @@ function validatePublicVariables({
 }
 
 function getDts({
-	clientPublic,
-	serverPublic,
-	serverSecret,
+	client,
+	server,
 	fs,
 }: {
-	clientPublic: string;
-	serverPublic: string;
-	serverSecret: string;
+	client: string;
+	server: string;
 	fs: typeof fsMod;
 }) {
 	const template = fs.readFileSync(TYPES_TEMPLATE_URL, 'utf-8');
 
-	return template
-		.replace('// @@CLIENT@@', clientPublic)
-		.replace('// @@SERVER@@', serverPublic)
-		.replace('// @@SECRET_VALUES@@', serverSecret);
+	return template.replace('// @@CLIENT@@', client).replace('// @@SERVER@@', server);
 }
 
 function getClientTemplates({
@@ -201,27 +195,28 @@ function getServerTemplates({
 	fs: typeof fsMod;
 }) {
 	let module = fs.readFileSync(MODULE_TEMPLATE_URL, 'utf-8');
-	let publicTypes = '';
-	let secretTypes = '';
+	let types = '';
+	let onSetGetEnv = '';
 
 	for (const { key, type, value } of validatedVariables.filter((e) => e.context === 'server')) {
 		module += `export const ${key} = ${JSON.stringify(value)};`;
-		publicTypes += `export const ${key}: ${type};	\n`;
+		types += `export const ${key}: ${type};	\n`;
 	}
 
 	for (const [key, options] of Object.entries(schema)) {
 		if (!(options.context === 'server' && options.access === 'secret')) {
 			continue;
 		}
 
-		secretTypes += `${key}: ${getEnvFieldType(options)};		\n`;
+		types += `export const ${key}: ${getEnvFieldType(options)};		\n`;
+		module += `export let ${key} = _internalGetSecret(${JSON.stringify(key)});\n`;
+		onSetGetEnv += `${key} = reset ? undefined : _internalGetSecret(${JSON.stringify(key)});\n`;
 	}
 
+	module = module.replace('// @@ON_SET_GET_ENV@@', onSetGetEnv);
+
 	return {
 		module,
-		types: {
-			public: publicTypes,
-			secret: secretTypes,
-		},
+		types,
 	};
 }

packages/astro/templates/env/module.mjs

@@ -1,18 +1,27 @@
 import { schema } from 'virtual:astro:env/internal';
-import { createInvalidVariableError, getEnv, validateEnvVariable } from 'astro/env/runtime';
+import {
+	createInvalidVariableError,
+	getEnv,
+	validateEnvVariable,
+	setOnSetGetEnv,
+} from 'astro/env/runtime';
 
 export const getSecret = (key) => {
+	return getEnv(key);
+};
+
+const _internalGetSecret = (key) => {
 	const rawVariable = getEnv(key);
 	const variable = rawVariable === '' ? undefined : rawVariable;
 	const options = schema[key];
 
-	if (!options) {
-		return variable;
-	}
-
 	const result = validateEnvVariable(variable, options);
 	if (result.ok) {
 		return result.value;
 	}
 	throw createInvalidVariableError(key, result.type);
 };
+
+setOnSetGetEnv((reset) => {
+	// @@ON_SET_GET_ENV@@
+});

packages/astro/templates/env/types.d.ts

@@ -5,16 +5,5 @@ declare module 'astro:env/client' {
 declare module 'astro:env/server' {
 	// @@SERVER@@
 
-	type SecretValues = {
-		// @@SECRET_VALUES@@
-	};
-
-	type SecretValue = keyof SecretValues;
-
-	type Loose<T> = T | (string & {});
-	type Strictify<T extends string> = T extends `${infer _}` ? T : never;
-
-	export const getSecret: <TKey extends Loose<SecretValue>>(
-		key: TKey
-	) => TKey extends Strictify<SecretValue> ? SecretValues[TKey] : string | undefined;
+	export const getSecret: (key: string) => string | undefined;
 }

packages/astro/test/fixtures/astro-env-server-fail/astro.config.mjs

@@ -5,7 +5,7 @@ export default defineConfig({
 	experimental: {
 		env: {
 			schema: {
-				PUBLIC_FOO: envField.string({ context: "server", access: "public", optional: true, default: "ABC" }),
+				FOO: envField.string({ context: "server", access: "public", optional: true, default: "ABC" }),
 			}
 		}
 	}

packages/astro/test/fixtures/astro-env-server-fail/src/pages/index.astro

@@ -1,3 +1,3 @@
 <script>
-    import { PUBLIC_FOO } from "astro:env/server"
+    import { FOO } from "astro:env/server"
 </script>

packages/astro/test/fixtures/astro-env-server-secret/src/pages/index.astro

@@ -1,7 +1,6 @@
 ---
-import { getSecret } from "astro:env/server"
+import { getSecret, KNOWN_SECRET } from "astro:env/server"
 
-const KNOWN_SECRET = getSecret("KNOWN_SECRET")
 const UNKNOWN_SECRET = getSecret("UNKNOWN_SECRET")
 ---
 

packages/astro/test/fixtures/astro-env/astro.config.mjs

@@ -5,9 +5,9 @@ export default defineConfig({
 	experimental: {
 		env: {
 			schema: {
-				PUBLIC_FOO: envField.string({ context: "client", access: "public", optional: true, default: "ABC" }),
-				PUBLIC_BAR: envField.string({ context: "client", access: "public", optional: true, default: "DEF" }),
-				PUBLIC_BAZ: envField.string({ context: "server", access: "public", optional: true, default: "GHI" }),
+				FOO: envField.string({ context: "client", access: "public", optional: true, default: "ABC" }),
+				BAR: envField.string({ context: "client", access: "public", optional: true, default: "DEF" }),
+				BAZ: envField.string({ context: "server", access: "public", optional: true, default: "GHI" }),
 			}
 		}
 	}

packages/astro/test/fixtures/astro-env/src/pages/index.astro

@@ -1,15 +1,15 @@
 ---
-import { PUBLIC_FOO } from "astro:env/client"
-import { PUBLIC_BAZ } from "astro:env/server"
+import { FOO } from "astro:env/client"
+import { BAZ } from "astro:env/server"
 
-console.log({ PUBLIC_BAZ })
+console.log({ BAZ })
 ---
 
-<div id="server-rendered">{PUBLIC_FOO}</div>
+<div id="server-rendered">{FOO}</div>
 <div id="client-rendered"></div>
 
 <script>
-    import { PUBLIC_BAR } from "astro:env/client"
+    import { BAR } from "astro:env/client"
 
-    document.getElementById("client-rendered").innerText = PUBLIC_BAR
+    document.getElementById("client-rendered").innerText = BAR
 </script>

packages/astro/test/units/config/config-validate.test.js

@@ -4,7 +4,6 @@ import stripAnsi from 'strip-ansi';
 import { z } from 'zod';
 import { validateConfig } from '../../../dist/core/config/config.js';
 import { formatConfigErrorMessage } from '../../../dist/core/messages.js';
-import { envField } from '../../../dist/env/config.js';
 
 describe('Config Validation', () => {
 	it('empty user config is valid', async () => {
@@ -368,46 +367,5 @@ describe('Config Validation', () => {
 				).catch((err) => err)
 			);
 		});
-
-		it('Should not allow client variables without a PUBLIC_ prefix', async () => {
-			const configError = await validateConfig(
-				{
-					experimental: {
-						env: {
-							schema: {
-								FOO: envField.string({ context: 'client', access: 'public' }),
-							},
-						},
-					},
-				},
-				process.cwd()
-			).catch((err) => err);
-			assert.equal(configError instanceof z.ZodError, true);
-			assert.equal(
-				configError.errors[0].message,
-				'An environment variable that is public must have a name prefixed by "PUBLIC_".'
-			);
-		});
-
-		it('Should not allow non client variables with a PUBLIC_ prefix', async () => {
-			const configError = await validateConfig(
-				{
-					experimental: {
-						env: {
-							schema: {
-								FOO: envField.string({ context: 'server', access: 'public' }),
-							},
-						},
-					},
-				},
-				process.cwd()
-			).catch((err) => err);
-			assert.equal(configError instanceof z.ZodError, true);
-			console.log(configError);
-			assert.equal(
-				configError.errors[0].message,
-				'An environment variable that is public must have a name prefixed by "PUBLIC_".'
-			);
-		});
 	});
 });