fix: include components dynamically imported in the universal load function when inlining CSS (#13723)

* add test

* map client to server stylesheets using rollup bundle data

* changeset

* avoid duplicate filenames

* skip bundle mapping if inline style threshold is not enabled

* optimise mapping and skip if no inline threshold

Author: eltigerchino

.changeset/spicy-cougars-wave.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: correctly inline stylesheets of components dynamically imported in a universal load function if they are below the configured inlineStyleThreshold

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

@@ -74,6 +74,7 @@ async function analyse({
 		server_manifest,
 		null,
 		null,
+		null,
 		output_config,
 		static_exports
 	);

packages/kit/src/exports/public.d.ts

@@ -468,7 +468,7 @@ export interface KitConfig {
 		errorTemplate?: string;
 	};
 	/**
-	 * Inline CSS inside a `<style>` block at the head of the HTML. This option is a number that specifies the maximum length of a CSS file in UTF-16 code units, as specified by the [String.length](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/length) property, to be inlined. All CSS files needed for the page and smaller than this value are merged and inlined in a `<style>` block.
+	 * Inline CSS inside a `<style>` block at the head of the HTML. This option is a number that specifies the maximum length of a CSS file in UTF-16 code units, as specified by the [String.length](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/length) property, to be inlined. All CSS files needed for the page that are smaller than this value are merged and inlined in a `<style>` block.
 	 *
 	 * > [!NOTE] This results in fewer initial requests and can improve your [First Contentful Paint](https://web.dev/first-contentful-paint) score. However, it generates larger HTML output and reduces the effectiveness of browser caches. Use it advisedly.
 	 * @default 0

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

@@ -13,66 +13,49 @@ import { create_node_analyser } from '../static_analysis/index.js';
  * @param {import('types').ManifestData} manifest_data
  * @param {import('vite').Manifest} server_manifest
  * @param {import('vite').Manifest | null} client_manifest
- * @param {import('vite').Rollup.OutputAsset[] | null} css
+ * @param {import('vite').Rollup.OutputBundle | null} server_bundle
+ * @param {import('vite').Rollup.RollupOutput['output'] | null} client_bundle
  * @param {import('types').RecursiveRequired<import('types').ValidatedConfig['kit']['output']>} output_config
  * @param {Map<string, { page_options: Record<string, any> | null, children: string[] }>} static_exports
  */
-export async function build_server_nodes(out, kit, manifest_data, server_manifest, client_manifest, css, output_config, static_exports) {
+export async function build_server_nodes(out, kit, manifest_data, server_manifest, client_manifest, server_bundle, client_bundle, output_config, static_exports) {
 	mkdirp(`${out}/server/nodes`);
 	mkdirp(`${out}/server/stylesheets`);
 
 	/** @type {Map<string, string>} */
-	const stylesheet_lookup = new Map();
-
-	if (css) {
-		/** @type {Set<string>} */
-		const client_stylesheets = new Set();
-		for (const key in client_manifest) {
-			client_manifest[key].css?.forEach((filename) => {
-				client_stylesheets.add(filename);
-			});
-		}
+	const stylesheets_to_inline = new Map();
 
-		/** @type {Map<number, string[]>} */
-		const server_stylesheets = new Map();
-		manifest_data.nodes.forEach((node, i) => {
-			if (!node.component || !server_manifest[node.component]) return;
+	if (server_bundle && client_bundle && kit.inlineStyleThreshold > 0) {
+		const client = get_stylesheets(client_bundle);
 
-			const { stylesheets } = find_deps(server_manifest, node.component, false);
+		const server_chunks = Object.values(server_bundle);
+		const server = get_stylesheets(server_chunks);
 
-			if (stylesheets.length) {
-				server_stylesheets.set(i, stylesheets);
-			}
-		});
-
-		for (const asset of css) {
-			// ignore dynamically imported stylesheets since we don't need to inline those
-			if (!client_stylesheets.has(asset.fileName) || asset.source.length >= kit.inlineStyleThreshold) {
+		// map server stylesheet name to the client stylesheet name
+		for (const [id, client_stylesheet] of client.stylesheets_used) {
+			const server_stylesheet = server.stylesheets_used.get(id);
+			if (!server_stylesheet) {
 				continue;
 			}
+			client_stylesheet.forEach((file, i) => {
+				stylesheets_to_inline.set(file, server_stylesheet[i]);
+			}) 
+		}
 
-			// We know that the names for entry points are numbers.
-			const [index] = basename(asset.fileName).split('.');
-			// There can also be other CSS files from shared components
-			// for example, which we need to ignore here.
-			if (isNaN(+index)) continue;
-
-			const file = `${out}/server/stylesheets/${index}.js`;
-
-			// we need to inline the server stylesheet instead of the client one
-			// so that asset paths are correct on document load
-			const filenames = server_stylesheets.get(+index);
-
-			if (!filenames) {
-				throw new Error('This should never happen, but if it does, it means we failed to find the server stylesheet for a node.');
+		// filter out stylesheets that should not be inlined
+		for (const [fileName, content] of client.stylesheet_content) {
+			if (content.length >= kit.inlineStyleThreshold) {
+				stylesheets_to_inline.delete(fileName);
 			}
+		}
 
-			const sources = filenames.map((filename) => {
-				return fs.readFileSync(`${out}/server/${filename}`, 'utf-8');
-			});
-			fs.writeFileSync(file, `// ${filenames.join(', ')}\nexport default ${s(sources.join('\n'))};`);
-
-			stylesheet_lookup.set(asset.fileName, index);
+		// map server stylesheet source to the client stylesheet name
+		for (const [client_file, server_file] of stylesheets_to_inline) {
+			const source = server.stylesheet_content.get(server_file);
+			if (!source) {
+				throw new Error(`Server stylesheet source not found for client stylesheet ${client_file}`);
+			}
+			stylesheets_to_inline.set(client_file, source);
 		}
 	}
 
@@ -139,8 +122,9 @@ export async function build_server_nodes(out, kit, manifest_data, server_manifes
 			const entry_path = `${normalizePath(kit.outDir)}/generated/client-optimized/nodes/${i}.js`;
 			const entry = find_deps(client_manifest, entry_path, true);
 
-			// eagerly load stylesheets and fonts imported by the SSR-ed page to avoid FOUC.
-			// If it is not used during SSR, it can be lazily loaded in the browser.
+			// eagerly load client stylesheets and fonts imported by the SSR-ed page to avoid FOUC.
+			// However, if it is not used during SSR (not present in the server manifest),
+			// then it can be lazily loaded in the browser.
 
 			/** @type {import('types').AssetDependencies | undefined} */
 			let component;
@@ -155,26 +139,26 @@ export async function build_server_nodes(out, kit, manifest_data, server_manifes
 			}
 
 			/** @type {Set<string>} */
-			const css_used_by_server = new Set();
+			const eager_css = new Set();
 			/** @type {Set<string>} */
-			const assets_used_by_server = new Set();
+			const eager_assets = new Set();
 
-			entry.stylesheet_map.forEach((value, key) => {
-				// pages and layouts are named as node indexes in the client manifest
-				// so we need to use the original filename when checking against the server manifest
-				if (key === entry_path) {
-					key = node.component ?? key;
+			entry.stylesheet_map.forEach((value, filepath) => {
+				// pages and layouts are renamed to node indexes when optimised for the client
+				// so we use the original filename instead to check against the server manifest
+				if (filepath === entry_path) {
+					filepath = node.component ?? filepath;
 				}
 
-				if (component?.stylesheet_map.has(key) || universal?.stylesheet_map.has(key)) {
-					value.css.forEach(file => css_used_by_server.add(file));
-					value.assets.forEach(file => assets_used_by_server.add(file));
+				if (component?.stylesheet_map.has(filepath) || universal?.stylesheet_map.has(filepath)) {
+					value.css.forEach(file => eager_css.add(file));
+					value.assets.forEach(file => eager_assets.add(file));
 				}
 			});
 
 			imported = entry.imports;
-			stylesheets = Array.from(css_used_by_server);
-			fonts = filter_fonts(Array.from(assets_used_by_server));
+			stylesheets = Array.from(eager_css);
+			fonts = filter_fonts(Array.from(eager_assets));
 		}
 
 		exports.push(
@@ -184,19 +168,26 @@ export async function build_server_nodes(out, kit, manifest_data, server_manifes
 		);
 
 		/** @type {string[]} */
-		const styles = [];
-
-		stylesheets.forEach((file) => {
-			if (stylesheet_lookup.has(file)) {
-				const index = stylesheet_lookup.get(file);
-				const name = `stylesheet_${index}`;
-				imports.push(`import ${name} from '../stylesheets/${index}.js';`);
-				styles.push(`\t${s(file)}: ${name}`);
+		const inline_styles = [];
+
+		stylesheets.forEach((file, i) => {
+			if (stylesheets_to_inline.has(file)) {
+				const filename = basename(file);
+				const dest = `${out}/server/stylesheets/${filename}.js`;
+				const source = stylesheets_to_inline.get(file);
+				if (!source) {
+					throw new Error(`Server stylesheet source not found for client stylesheet ${file}`);
+				}
+				fs.writeFileSync(dest, `// ${filename}\nexport default ${s(source)};`);
+
+				const name = `stylesheet_${i}`;
+				imports.push(`import ${name} from '../stylesheets/${filename}.js';`);
+				inline_styles.push(`\t${s(file)}: ${name}`);
 			}
 		});
 
-		if (styles.length > 0) {
-			exports.push(`export const inline_styles = () => ({\n${styles.join(',\n')}\n});`);
+		if (inline_styles.length > 0) {
+			exports.push(`export const inline_styles = () => ({\n${inline_styles.join(',\n')}\n});`);
 		}
 
 		fs.writeFileSync(
@@ -205,3 +196,37 @@ export async function build_server_nodes(out, kit, manifest_data, server_manifes
 		);
 	}
 }
+
+/**
+ * @param {(import('vite').Rollup.OutputAsset | import('vite').Rollup.OutputChunk)[]} chunks 
+ */
+function get_stylesheets(chunks) {
+	/**
+	 * A map of module IDs and the stylesheets they use.
+	 * @type {Map<string, string[]>}
+	 */
+	const stylesheets_used = new Map();
+
+	/**
+	 * A map of stylesheet names and their content.
+	 * @type {Map<string, string>}
+	 */
+	const stylesheet_content = new Map();
+
+	for (const chunk of chunks) {
+		if (chunk.type === 'asset') {
+			if (chunk.fileName.endsWith('.css')) {
+				stylesheet_content.set(chunk.fileName, chunk.source.toString());
+			}
+			continue;
+		}
+
+		if (chunk.viteMetadata?.importedCss.size) {
+			const css = Array.from(chunk.viteMetadata.importedCss);
+			for (const id of chunk.moduleIds) {
+				stylesheets_used.set(id, css );
+			}
+		}
+	}
+	return { stylesheets_used, stylesheet_content };
+}

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

@@ -787,7 +787,7 @@ Tips:
 		 */
 		writeBundle: {
 			sequential: true,
-			async handler(_options) {
+			async handler(_options, bundle) {
 				if (secondary_build_started) return; // only run this once
 
 				const verbose = vite_config.logLevel === 'info';
@@ -843,7 +843,7 @@ Tips:
 
 				secondary_build_started = true;
 
-				const { output } = /** @type {import('vite').Rollup.RollupOutput} */ (
+				const { output: client_chunks } = /** @type {import('vite').Rollup.RollupOutput} */ (
 					await vite.build({
 						configFile: vite_config.configFile,
 						// CLI args
@@ -886,7 +886,7 @@ Tips:
 						imports: [...start.imports, ...app.imports],
 						stylesheets: [...start.stylesheets, ...app.stylesheets],
 						fonts: [...start.fonts, ...app.fonts],
-						uses_env_dynamic_public: output.some(
+						uses_env_dynamic_public: client_chunks.some(
 							(chunk) => chunk.type === 'chunk' && chunk.modules[env_dynamic_public]
 						)
 					};
@@ -935,14 +935,14 @@ Tips:
 						imports: start.imports,
 						stylesheets: start.stylesheets,
 						fonts: start.fonts,
-						uses_env_dynamic_public: output.some(
+						uses_env_dynamic_public: client_chunks.some(
 							(chunk) => chunk.type === 'chunk' && chunk.modules[env_dynamic_public]
 						)
 					};
 
 					if (svelte_config.kit.output.bundleStrategy === 'inline') {
 						const style = /** @type {import('rollup').OutputAsset} */ (
-							output.find(
+							client_chunks.find(
 								(chunk) =>
 									chunk.type === 'asset' &&
 									chunk.names.length === 1 &&
@@ -957,11 +957,6 @@ Tips:
 					}
 				}
 
-				const css = output.filter(
-					/** @type {(value: any) => value is import('vite').Rollup.OutputAsset} */
-					(value) => value.type === 'asset' && value.fileName.endsWith('.css')
-				);
-
 				// regenerate manifest now that we have client entry...
 				fs.writeFileSync(
 					manifest_path,
@@ -980,7 +975,8 @@ Tips:
 					manifest_data,
 					server_manifest,
 					client_manifest,
-					css,
+					bundle,
+					client_chunks,
 					svelte_config.kit.output,
 					static_exports
 				);

packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/+page.js

@@ -0,0 +1,5 @@
+export async function load() {
+	return {
+		Thing: (await import('./Thing.svelte')).default
+	};
+}

packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/+page.svelte

@@ -0,0 +1,5 @@
+<script>
+	export let data;
+</script>
+
+<svelte:component this={data.Thing} />

packages/kit/test/apps/options/source/pages/inline-assets/dynamic-import/Thing.svelte

@@ -0,0 +1,8 @@
+<p>I'm dynamically imported</p>
+
+<style>
+	p {
+		color: blue;
+		font-size: 20px;
+	}
+</style>

packages/kit/test/apps/options/test/test.js

@@ -107,7 +107,7 @@ test.describe('assets path', () => {
 		await page.goto('/path-base/');
 		const href = await page.locator('link[rel="icon"]').getAttribute('href');
 
-		const response = await request.get(href);
+		const response = await request.get(href ?? '');
 		expect(response.status()).toBe(200);
 	});
 });
@@ -133,15 +133,15 @@ test.describe('CSP', () => {
 	test('ensure CSP header in stream response', async ({ page, javaScriptEnabled }) => {
 		if (!javaScriptEnabled) return;
 		const response = await page.goto('/path-base/csp-with-stream');
-		expect(response.headers()['content-security-policy']).toMatch(
+		expect(response?.headers()['content-security-policy']).toMatch(
 			/require-trusted-types-for 'script'/
 		);
 		expect(await page.textContent('h2')).toBe('Moo Deng!');
 	});
 
 	test("quotes 'script'", async ({ page }) => {
 		const response = await page.goto('/path-base');
-		expect(response.headers()['content-security-policy']).toMatch(
+		expect(response?.headers()['content-security-policy']).toMatch(
 			/require-trusted-types-for 'script'/
 		);
 	});
@@ -305,19 +305,32 @@ if (!process.env.DEV) {
 	});
 
 	test.describe('inlineStyleThreshold', () => {
-		test('loads asset', async ({ page }) => {
+		test('loads assets', async ({ page }) => {
 			let fontLoaded = false;
-
 			page.on('response', (response) => {
 				if (response.url().endsWith('.woff2') || response.url().endsWith('.woff')) {
 					fontLoaded = response.ok();
 				}
 			});
-
 			await page.goto('/path-base/inline-assets');
-
 			expect(fontLoaded).toBeTruthy();
 		});
+
+		test('includes components dynamically imported in universal load', async ({
+			page,
+			get_computed_style
+		}) => {
+			let loaded_css = false;
+			page.on('response', (response) => {
+				if (response.url().endsWith('.css')) {
+					loaded_css = true;
+				}
+			});
+			await page.goto('/path-base/inline-assets/dynamic-import');
+			await expect(page.locator('p')).toHaveText("I'm dynamically imported");
+			expect(loaded_css).toBe(false);
+			expect(await get_computed_style('p', 'color')).toEqual('rgb(0, 0, 255)');
+		});
 	});
 }