fix: set $env and $app/environment variables before analysing server nodes (#13790)

* fix, test, and changeset

* format

* fix test

* avoid analysing twice when building nodes

* return cache and reuse it since analyse is a forked process

Author: eltigerchino

.changeset/smooth-rockets-complain.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: ensure `$env` and `$app/environment` are correctly set while analysing server nodes

packages/kit/src/core/postbuild/analyse.js

@@ -10,6 +10,7 @@ import { has_server_load, resolve_route } from '../../utils/routing.js';
 import { check_feature } from '../../utils/features.js';
 import { createReadableStream } from '@sveltejs/kit/node';
 import { PageNodes } from '../../utils/page_nodes.js';
+import { build_server_nodes } from '../../exports/vite/build/build_server.js';
 
 export default forked(import.meta.url, analyse);
 
@@ -20,7 +21,9 @@ export default forked(import.meta.url, analyse);
  *   manifest_data: import('types').ManifestData;
  *   server_manifest: import('vite').Manifest;
  *   tracked_features: Record<string, string[]>;
- *   env: Record<string, string>
+ *   env: Record<string, string>;
+ *   out: string;
+ *   output_config: import('types').RecursiveRequired<import('types').ValidatedConfig['kit']['output']>;
  * }} opts
  */
 async function analyse({
@@ -29,7 +32,9 @@ async function analyse({
 	manifest_data,
 	server_manifest,
 	tracked_features,
-	env
+	env,
+	out,
+	output_config
 }) {
 	/** @type {import('@sveltejs/kit').SSRManifest} */
 	const manifest = (await import(pathToFileURL(manifest_path).href)).manifest;
@@ -58,6 +63,20 @@ async function analyse({
 	internal.set_manifest(manifest);
 	internal.set_read_implementation((file) => createReadableStream(`${server_root}/server/${file}`));
 
+	const static_exports = new Map();
+
+	// first, build server nodes without the client manifest so we can analyse it
+	await build_server_nodes(
+		out,
+		config,
+		manifest_data,
+		server_manifest,
+		null,
+		null,
+		output_config,
+		static_exports
+	);
+
 	/** @type {import('types').ServerMetadata} */
 	const metadata = {
 		nodes: [],
@@ -143,7 +162,7 @@ async function analyse({
 		});
 	}
 
-	return metadata;
+	return { metadata, static_exports };
 }
 
 /**

packages/kit/src/exports/vite/build/build_server.js

@@ -4,7 +4,8 @@ import { filter_fonts, find_deps, resolve_symlinks } from './utils.js';
 import { s } from '../../../utils/misc.js';
 import { normalizePath } from 'vite';
 import { basename, join } from 'node:path';
-import { create_static_analyser } from '../static_analysis/index.js';
+import { create_node_analyser } from '../static_analysis/index.js';
+
 
 /**
  * @param {string} out
@@ -14,8 +15,9 @@ import { create_static_analyser } from '../static_analysis/index.js';
  * @param {import('vite').Manifest | null} client_manifest
  * @param {import('vite').Rollup.OutputAsset[] | null} css
  * @param {import('types').RecursiveRequired<import('types').ValidatedConfig['kit']['output']>} output_config
+ * @param {Map<string, Record<string, any> | null>} static_exports
  */
-export async function build_server_nodes(out, kit, manifest_data, server_manifest, client_manifest, css, output_config) {
+export async function build_server_nodes(out, kit, manifest_data, server_manifest, client_manifest, css, output_config, static_exports) {
 	mkdirp(`${out}/server/nodes`);
 	mkdirp(`${out}/server/stylesheets`);
 
@@ -74,9 +76,12 @@ export async function build_server_nodes(out, kit, manifest_data, server_manifes
 		}
 	}
 
-	const { get_page_options } = create_static_analyser(async (server_node) => {
-		// Windows needs the file:// protocol for absolute path dynamic imports
-		return import(`file://${join(out, 'server', resolve_symlinks(server_manifest, server_node).chunk.file)}`);
+	const { get_page_options } = create_node_analyser({
+		resolve: (server_node) => {
+			// Windows needs the file:// protocol for absolute path dynamic imports
+			return import(`file://${join(out, 'server', resolve_symlinks(server_manifest, server_node).chunk.file)}`);
+		},
+		static_exports
 	});
 
 	for (let i = 0; i < manifest_data.nodes.length; i++) {

packages/kit/src/exports/vite/dev/index.js

@@ -19,7 +19,7 @@ import { not_found } from '../utils.js';
 import { SCHEME } from '../../../utils/url.js';
 import { check_feature } from '../../../utils/features.js';
 import { escape_html } from '../../../utils/escape.js';
-import { create_static_analyser } from '../static_analysis/index.js';
+import { create_node_analyser } from '../static_analysis/index.js';
 
 const cwd = process.cwd();
 // vite-specifc queries that we should skip handling for css urls
@@ -128,11 +128,13 @@ export async function dev(vite, vite_config, svelte_config) {
 			return;
 		}
 
-		const static_analyser = create_static_analyser(async (server_node) => {
-			const { module } = await resolve(server_node);
-			return module;
+		const node_analyser = create_node_analyser({
+			resolve: async (server_node) => {
+				const { module } = await resolve(server_node);
+				return module;
+			}
 		});
-		invalidate_page_options = static_analyser.invalidate_page_options;
+		invalidate_page_options = node_analyser.invalidate_page_options;
 
 		manifest = {
 			appDir: svelte_config.kit.appDir,
@@ -212,7 +214,7 @@ export async function dev(vite, vite_config, svelte_config) {
 						}
 
 						if (node.universal) {
-							const page_options = await static_analyser.get_page_options(node);
+							const page_options = await node_analyser.get_page_options(node);
 							if (page_options?.ssr === false) {
 								result.universal = page_options;
 							} else {

packages/kit/src/exports/vite/index.js

@@ -818,26 +818,17 @@ Tips:
 					})};\n`
 				);
 
-				// first, build server nodes without the client manifest so we can analyse it
 				log.info('Analysing routes');
 
-				await build_server_nodes(
-					out,
-					kit,
-					manifest_data,
-					server_manifest,
-					null,
-					null,
-					svelte_config.output
-				);
-
-				const metadata = await analyse({
+				const { metadata, static_exports } = await analyse({
 					hash: kit.router.type === 'hash',
 					manifest_path,
 					manifest_data,
 					server_manifest,
 					tracked_features,
-					env: { ...env.private, ...env.public }
+					env: { ...env.private, ...env.public },
+					out,
+					output_config: svelte_config.output
 				});
 
 				log.info('Building app');
@@ -990,7 +981,8 @@ Tips:
 					server_manifest,
 					client_manifest,
 					css,
-					svelte_config.kit.output
+					svelte_config.kit.output,
+					static_exports
 				);
 
 				// ...and prerender

packages/kit/src/exports/vite/static_analysis/index.js

@@ -181,12 +181,12 @@ export function get_name(node) {
 }
 
 /**
- * @param {(server_node: string) => Promise<Record<string, any>>} resolve
+ * @param {{
+ *   resolve: (file: string) => Promise<Record<string, any>>;
+ *   static_exports?: Map<string, Record<string, any> | null>;
+ * }} opts
  */
-export function create_static_analyser(resolve) {
-	/** @type {Map<string, Record<string, any> | null>} */
-	const static_exports = new Map();
-
+export function create_node_analyser({ resolve, static_exports = new Map() }) {
 	/**
 	 * Computes the final page options for a node (if possible). Otherwise, returns `null`.
 	 * @param {import('types').PageNode} node

packages/kit/test/apps/basics/src/routes/node-analysis/+page.js

@@ -0,0 +1 @@
+// this universal file causes the server node to be evaluated during static analysis

packages/kit/test/apps/basics/src/routes/node-analysis/+page.server.js

@@ -0,0 +1,9 @@
+// this module is evaluated when building the server nodes to determine its page options
+// Therefore, we need to ensure $env, etc. still works during this process so that it doesn't throw errors
+// when it's evaluated as `undefined`
+
+import { env } from '$env/dynamic/public';
+
+if (!env.PUBLIC_DYNAMIC) {
+	throw Error('this should not happen');
+}