fix: only disallow dynamic env access when prerendering (#11436)

* only disallow dynamic env access when prerendering * changeset * add test * sigh --------- Co-authored-by: Rich Harris <rich.harris@vercel.com>
sveltejs · Dec 21, 2023 · 851176e · 851176e
1 parent d4eeb26
commit 851176e
Show file tree

Hide file tree

Showing 11 changed files with 59 additions and 18 deletions.
diff --git a/.changeset/perfect-eyes-brush.md b/.changeset/perfect-eyes-brush.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: allow dynamic env access when building but not prerendering
diff --git a/packages/kit/src/core/postbuild/analyse.js b/packages/kit/src/core/postbuild/analyse.js
@@ -39,7 +39,7 @@ async function analyse({ manifest_path, env }) {
 
 	// configure `import { building } from '$app/environment'` —
 	// essential we do this before analysing the code
-	internal.set_building(true);
+	internal.set_building();
 
 	// set env, in case it's used in initialisation
 	const { publicPrefix: public_prefix, privatePrefix: private_prefix } = config.env;

diff --git a/packages/kit/src/core/postbuild/fallback.js b/packages/kit/src/core/postbuild/fallback.js
@@ -30,7 +30,7 @@ async function generate_fallback({ manifest_path, env }) {
 	/** @type {import('@sveltejs/kit').SSRManifest} */
 	const manifest = (await import(pathToFileURL(manifest_path).href)).manifest;
 
-	set_building(true);
+	set_building();
 
 	const server = new Server(manifest);
 	await server.init({ env });

diff --git a/packages/kit/src/core/postbuild/prerender.js b/packages/kit/src/core/postbuild/prerender.js
@@ -36,7 +36,8 @@ async function prerender({ out, manifest_path, metadata, verbose, env }) {
 
 	// configure `import { building } from '$app/environment'` —
 	// essential we do this before analysing the code
-	internal.set_building(true);
+	internal.set_building();
+	internal.set_prerendering();
 
 	/**
 	 * @template {{message: string}} T
@@ -98,9 +99,6 @@ async function prerender({ out, manifest_path, metadata, verbose, env }) {
 	/** @type {Map<string, string>} */
 	const saved = new Map();
 
-	const server = new Server(manifest);
-	await server.init({ env });
-
 	const handle_http_error = normalise_error_handler(
 		log,
 		config.prerender.handleHttpError,
@@ -413,16 +411,27 @@ async function prerender({ out, manifest_path, metadata, verbose, env }) {
 		}
 	}
 
+	let has_prerenderable_routes = false;
+
+	for (const value of prerender_map.values()) {
+		if (value) {
+			has_prerenderable_routes = true;
+			break;
+		}
+	}
+
 	if (
-		config.prerender.entries.length > 1 ||
-		config.prerender.entries[0] !== '*' ||
-		route_level_entries.length > 0 ||
-		prerender_map.size > 0
+		(config.prerender.entries.length === 0 && route_level_entries.length === 0) ||
+		!has_prerenderable_routes
 	) {
-		// Only log if we're actually going to do something to not confuse users
-		log.info('Prerendering');
+		return { prerendered, prerender_map };
 	}
 
+	log.info('Prerendering');
+
+	const server = new Server(manifest);
+	await server.init({ env });
+
 	for (const entry of config.prerender.entries) {
 		if (entry === '*') {
 			for (const [id, prerender] of prerender_map) {

diff --git a/packages/kit/src/core/sync/write_server.js b/packages/kit/src/core/sync/write_server.js
@@ -26,7 +26,7 @@ const server_template = ({
 	error_page
 }) => `
 import root from '../root.${isSvelte5Plus() ? 'js' : 'svelte'}';
-import { set_building } from '__sveltekit/environment';
+import { set_building, set_prerendering } from '__sveltekit/environment';
 import { set_assets } from '__sveltekit/paths';
 import { set_private_env, set_public_env, set_safe_public_env } from '${runtime_directory}/shared-server.js';
 
@@ -63,7 +63,7 @@ export function get_hooks() {
 	return ${hooks ? `import(${s(hooks)})` : '{}'};
 }
 
-export { set_assets, set_building, set_private_env, set_public_env, set_safe_public_env };
+export { set_assets, set_building, set_prerendering, set_private_env, set_public_env, set_safe_public_env };
 `;
 
 // TODO need to re-run this whenever src/app.html or src/error.html are

diff --git a/packages/kit/src/exports/vite/index.js b/packages/kit/src/exports/vite/index.js
@@ -442,10 +442,15 @@ function kit({ svelte_config }) {
 					return dedent`
 						export const version = ${s(version.name)};
 						export let building = false;
+						export let prerendering = false;
 
 						export function set_building() {
 							building = true;
 						}
+
+						export function set_prerendering() {
+							prerendering = true;
+						}
 					`;
 				}
 			}

diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js
@@ -3,7 +3,7 @@ import { set_private_env, set_public_env, set_safe_public_env } from '../shared-
 import { options, get_hooks } from '__SERVER__/internal.js';
 import { DEV } from 'esm-env';
 import { filter_private_env, filter_public_env } from '../../utils/env.js';
-import { building } from '../app/environment.js';
+import { prerendering } from '__sveltekit/environment';
 
 /** @type {ProxyHandler<{ type: 'public' | 'private' }>} */
 const prerender_env_handler = {
@@ -47,8 +47,12 @@ export class Server {
 		const private_env = filter_private_env(env, prefixes);
 		const public_env = filter_public_env(env, prefixes);
 
-		set_private_env(building ? new Proxy({ type: 'private' }, prerender_env_handler) : private_env);
-		set_public_env(building ? new Proxy({ type: 'public' }, prerender_env_handler) : public_env);
+		set_private_env(
+			prerendering ? new Proxy({ type: 'private' }, prerender_env_handler) : private_env
+		);
+		set_public_env(
+			prerendering ? new Proxy({ type: 'public' }, prerender_env_handler) : public_env
+		);
 		set_safe_public_env(public_env);
 
 		if (!this.#options.hooks) {

diff --git a/packages/kit/src/types/ambient.d.ts b/packages/kit/src/types/ambient.d.ts
@@ -86,11 +86,16 @@ declare module '__sveltekit/environment' {
 	 * SvelteKit analyses your app during the `build` step by running it. During this process, `building` is `true`. This also applies during prerendering.
 	 */
 	export const building: boolean;
+	/**
+	 * True during prerendering, false otherwise.
+	 */
+	export const prerendering: boolean;
 	/**
 	 * The value of `config.kit.version.name`.
 	 */
 	export const version: string;
 	export function set_building(): void;
+	export function set_prerendering(): void;
 }
 
 /** Internal version of $app/paths */

diff --git a/packages/kit/src/types/internal.d.ts b/packages/kit/src/types/internal.d.ts
@@ -27,8 +27,9 @@ export interface ServerModule {
 }
 
 export interface ServerInternalModule {
-	set_building(building: boolean): void;
 	set_assets(path: string): void;
+	set_building(): void;
+	set_prerendering(): void;
 	set_private_env(environment: Record<string, string>): void;
 	set_public_env(environment: Record<string, string>): void;
 	set_safe_public_env(environment: Record<string, string>): void;

diff --git a/packages/kit/test/apps/options/source/hooks.server.js b/packages/kit/test/apps/options/source/hooks.server.js
@@ -1,3 +1,10 @@
+import { env } from '$env/dynamic/private';
+
+// this verifies that dynamic env vars can be read during analysis phase
+// (it would fail if this app contained prerendered routes)
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
+const FOO = env.FOO;
+
 /** @type {import('@sveltejs/kit').Handle} */
 export function handle({ event, resolve }) {
 	return resolve(event, {

diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts
@@ -2185,11 +2185,16 @@ declare module '__sveltekit/environment' {
 	 * SvelteKit analyses your app during the `build` step by running it. During this process, `building` is `true`. This also applies during prerendering.
 	 */
 	export const building: boolean;
+	/**
+	 * True during prerendering, false otherwise.
+	 */
+	export const prerendering: boolean;
 	/**
 	 * The value of `config.kit.version.name`.
 	 */
 	export const version: string;
 	export function set_building(): void;
+	export function set_prerendering(): void;
 }
 
 /** Internal version of $app/paths */