fix: keep the @font-face rules inlined during development through hydration

.changeset/keep-dev-styles-on-hydration.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: let Vite's client take over the styles inlined during development instead of replacing them on hydration, which re-created their font faces

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

@@ -136,9 +136,7 @@ export function generate_manifest(
 					/** @type {Set<EnvironmentModuleNode>} */
 					const deps = new Set();
 
-					for (const module_node of module_nodes) {
-						await find_deps(vite_dev_server, module_node, deps);
-					}
+					await find_deps(vite_dev_server, module_nodes, deps);
 
 					/** @type {Record<string, string>} */
 					const styles = {};
@@ -150,7 +148,8 @@ export function generate_manifest(
 								: dep.url + '?inline';
 							try {
 								const mod = await runner.import(inlineCssUrl);
-								styles[dep.url] = mod.default;
+								// keyed by the module's id, which Vite's client uses too when it resolves the same file
+								styles[dep.id ?? dep.url] = mod.default;
 							} catch {
 								// this can happen with dynamically imported modules, I think
 								// because the Vite module graph doesn't distinguish between
@@ -266,44 +265,58 @@ async function resolve(vite, vite_dev_server, runner, id) {
 }
 
 /**
+ * Adds `nodes` and the modules they import, directly or not, to `deps`: static imports depth first,
+ * which puts their stylesheets in the order the browser evaluates them, then the modules imported
+ * dynamically, whose order at run time is only approximated
  * @param {ViteDevServer} vite
- * @param {EnvironmentModuleNode} node
+ * @param {EnvironmentModuleNode[]} nodes
  * @param {Set<EnvironmentModuleNode>} deps
  */
-async function find_deps(vite, node, deps) {
+async function find_deps(vite, nodes, deps) {
+	/** @type {Map<EnvironmentModuleNode, { static_imports: EnvironmentModuleNode[], dynamic_imports: EnvironmentModuleNode[] }>} */
+	const imports = new Map();
+
+	/** @param {string[]} [urls] */
+	async function resolve_urls(urls = []) {
+		const nodes = await Promise.all(
+			urls.map((url) => vite.environments.ssr.moduleGraph.getModuleByUrl(url))
+		);
+		return nodes.filter((node) => node !== undefined);
+	}
+
 	// since `transformResult.deps` contains URLs instead of `ModuleNode`s, this process is asynchronous.
 	// instead of using `await`, we resolve all branches in parallel.
-	/** @type {Promise<void>[]} */
-	const branches = [];
-
 	/** @param {EnvironmentModuleNode} node */
-	async function add(node) {
-		if (!deps.has(node)) {
-			deps.add(node);
-			await find_deps(vite, node, deps);
-		}
-	}
+	async function discover(node) {
+		if (imports.has(node)) return;
 
-	/** @param {string} url */
-	async function add_by_url(url) {
-		const node = await vite.environments.ssr.moduleGraph.getModuleByUrl(url);
+		/** @type {{ static_imports: EnvironmentModuleNode[], dynamic_imports: EnvironmentModuleNode[] }} */
+		const found = { static_imports: Array.from(node.importedModules), dynamic_imports: [] };
+		imports.set(node, found);
 
-		if (node) {
-			await add(node);
+		if (node.transformResult) {
+			found.static_imports = await resolve_urls(node.transformResult.deps);
+			found.dynamic_imports = await resolve_urls(node.transformResult.dynamicDeps);
 		}
+
+		await Promise.all([...found.static_imports, ...found.dynamic_imports].map(discover));
 	}
 
-	if (node.transformResult) {
-		if (node.transformResult.deps) {
-			node.transformResult.deps.forEach((url) => branches.push(add_by_url(url)));
-		}
+	/** @type {EnvironmentModuleNode[]} */
+	const dynamic_imports = [];
 
-		if (node.transformResult.dynamicDeps) {
-			node.transformResult.dynamicDeps.forEach((url) => branches.push(add_by_url(url)));
-		}
-	} else {
-		node.importedModules.forEach((node) => branches.push(add(node)));
+	/** @param {EnvironmentModuleNode} node */
+	function add(node) {
+		if (deps.has(node)) return;
+		deps.add(node);
+
+		const found = imports.get(node);
+		found?.static_imports.forEach(add);
+		dynamic_imports.push(...(found?.dynamic_imports ?? []));
 	}
 
-	await Promise.all(branches);
+	await Promise.all(nodes.map(discover));
+	nodes.forEach(add);
+	// the list grows as dynamically imported modules are added
+	for (const node of dynamic_imports) add(node);
 }

packages/kit/src/runtime/client/client.js

@@ -898,12 +898,6 @@ async function initialize(result, target, should_hydrate) {
 		nav
 	};
 
-	// Removes the style node we used to avoid FOUC during development
-	if (__SVELTEKIT_DEV__) {
-		const style = document.querySelector('style[data-sveltekit]');
-		if (style) style.remove();
-	}
-
 	apply_navigation_result(result);
 
 	// TODO treeshake `hydrate` in csr mode

packages/kit/src/runtime/server/page/render.js

@@ -279,18 +279,29 @@ export async function render_response({
 	/** @param {string} path */
 	const prefixed = (path) => client_path(path, { base, assets });
 
-	const style = client?.inline
-		? client.inline?.style
-		: Array.from(inline_styles.values()).join('\n');
-
-	if (style) {
-		// We always inline all styles to avoid FOUC during development.
-		// Once that's accomplished, we find and remove the style node using the
-		// `data-sveltekit` attribute once CSR kicks in
-		const attributes = __SVELTEKIT_DEV__ ? ['data-sveltekit'] : [];
+	/**
+	 * @param {string} style
+	 * @param {string[]} attributes
+	 */
+	const add_style = async (style, attributes) => {
 		if (csp.style_needs_nonce) attributes.push(`nonce="${csp.nonce}"`);
 		await csp.add_style(style);
 		head.add_style(style, attributes);
+	};
+
+	if (__SVELTEKIT_DEV__) {
+		// We always inline all styles to avoid FOUC during development. Each one is marked with its
+		// module's id, so that Vite's client takes it over, instead of adding a copy, when it runs a
+		// module with the same id
+		for (const [id, style] of inline_styles) {
+			await add_style(style, [`data-vite-dev-id="${escape_html(id, true)}"`]);
+		}
+	} else {
+		const style = client?.inline
+			? client.inline?.style
+			: Array.from(inline_styles.values()).join('\n');
+
+		if (style) await add_style(style, []);
 	}
 
 	/**

packages/kit/test/apps/basics/src/routes/css/order/+page.js

@@ -0,0 +1,6 @@
+import './cycle.js';
+import './after-cycle.css';
+
+export async function load() {
+	await import('./late.css');
+}

packages/kit/test/apps/basics/src/routes/css/order/+page.svelte

@@ -0,0 +1,8 @@
+<script>
+	import './nested.js';
+	import './override.css';
+</script>
+
+<p class="nested">this text is blue</p>
+<p class="cycle">this text is blue</p>
+<p class="late">this text is blue</p>

packages/kit/test/apps/basics/src/routes/css/order/after-cycle.css

@@ -0,0 +1,3 @@
+.cycle {
+	color: blue;
+}

packages/kit/test/apps/basics/src/routes/css/order/base.css

@@ -0,0 +1,4 @@
+.nested,
+.late {
+	color: red;
+}

packages/kit/test/apps/basics/src/routes/css/order/before-cycle.css

@@ -0,0 +1,3 @@
+.cycle {
+	color: red;
+}

packages/kit/test/apps/basics/src/routes/css/order/cycle.js

@@ -0,0 +1,2 @@
+import './+page.js';
+import './before-cycle.css';

packages/kit/test/apps/basics/src/routes/css/order/late.css

@@ -0,0 +1,3 @@
+.late {
+	color: blue;
+}

packages/kit/test/apps/basics/src/routes/css/order/nested.js

@@ -0,0 +1 @@
+import './base.css';

packages/kit/test/apps/basics/src/routes/css/order/override.css

@@ -0,0 +1,3 @@
+.nested {
+	color: blue;
+}

packages/kit/test/apps/basics/test/playwright/client.test.js

@@ -2818,3 +2818,41 @@ test.describe('routing', () => {
 		await expect(page).toHaveURL((url) => url.pathname === '/routing');
 	});
 });
+
+test.describe('CSS', () => {
+	test('lets Vite take over the styles inlined during development', async ({ page }) => {
+		test.skip(!process.env.DEV, 'styles are only inlined during development');
+
+		// hold back the stylesheet's module, so that hydration waits until the inlined style is found
+		/** @type {import('@playwright/test').Route[]} */
+		const held = [];
+		await page.route(
+			(url) => url.pathname.endsWith('/css/_styles.css'),
+			(route) => {
+				held.push(route);
+			}
+		);
+
+		await page.goto('/css', { waitUntil: 'commit', wait_for_started: false });
+
+		// `color: green` is only in `_styles.css`
+		const inlined = await page.waitForFunction(() =>
+			Array.from(document.querySelectorAll('style')).find((style) =>
+				style.textContent?.includes('color: green')
+			)
+		);
+		await expect.poll(() => held.length).toBe(1);
+		await held[0].continue();
+		await page.locator('body.started').waitFor();
+
+		expect(await inlined.evaluate((style) => style?.isConnected)).toBe(true);
+		expect(
+			await page.evaluate(
+				() =>
+					Array.from(document.querySelectorAll('style')).filter((style) =>
+						style.textContent?.includes('color: green')
+					).length
+			)
+		).toBe(1);
+	});
+});

packages/kit/test/apps/basics/test/playwright/cross-platform/test.js

@@ -53,6 +53,23 @@ test.describe('CSS', () => {
 		check_styles(get_computed_style);
 	});
 
+	test('applies styles in the order they are imported', async ({
+		page,
+		get_computed_style,
+		javaScriptEnabled
+	}) => {
+		await page.goto('/css/order');
+		await expect(page.locator('.nested')).toBeVisible();
+
+		expect(await get_computed_style('.nested', 'color')).toBe('rgb(0, 0, 255)');
+		expect(await get_computed_style('.cycle', 'color')).toBe('rgb(0, 0, 255)');
+
+		// a built page doesn't link the styles that `load` imports, until the client loads them
+		if (process.env.DEV || javaScriptEnabled) {
+			expect(await get_computed_style('.late', 'color')).toBe('rgb(0, 0, 255)');
+		}
+	});
+
 	test('loads styles on routes with encoded characters', async ({ page, get_computed_style }) => {
 		await page.goto('/css/encöded');
 		expect(await get_computed_style('h1', 'color')).toBe('rgb(128, 0, 128)');

packages/kit/test/apps/basics/test/playwright/test.js

@@ -741,7 +741,7 @@ test.describe('Page options', () => {
 			expect(await page.textContent('h1')).toBe('content was rendered');
 		} else {
 			expect(await page.$('h1')).toBeNull();
-			expect(await page.$('style[data-sveltekit]')).toBeNull();
+			expect(await page.$('style[data-vite-dev-id]')).toBeNull();
 		}
 	});
 

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

@@ -172,18 +172,11 @@ test.describe('assets path', () => {
 });
 
 test.describe('inlineStyleThreshold', () => {
-	test('inlines CSS', async ({ page, javaScriptEnabled }) => {
+	test('inlines CSS', async ({ page }) => {
 		await page.goto('/path-base/base/');
 		if (process.env.DEV) {
-			const ssr_style = await page.$('style[data-sveltekit]');
-
-			if (javaScriptEnabled) {
-				// <style data-sveltekit> is removed upon hydration
-				expect(ssr_style).toBeNull();
-			} else {
-				expect(ssr_style).not.toBeNull();
-			}
-
+			// Vite's client takes over the inlined styles on hydration, so they stay
+			expect(await page.$('style[data-vite-dev-id]')).not.toBeNull();
 			expect(await page.$('link[rel="stylesheet"]')).toBeNull();
 		} else {
 			expect(await page.$('style')).not.toBeNull();