Fix CSS deduping and missing chunks (#7218)

7 files changed, 69 insertions, 28 deletions

diff --git a/.changeset/eighty-gifts-cheer.md b/.changeset/eighty-gifts-cheer.md
new file mode 100644
index 000000000..41ca0d7f6
--- /dev/null
+++ b/.changeset/eighty-gifts-cheer.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fix CSS deduping and missing chunks
diff --git a/packages/astro/src/core/build/internal.ts b/packages/astro/src/core/build/internal.ts
index 18d28ef74..d1e2404d6 100644
--- a/packages/astro/src/core/build/internal.ts
+++ b/packages/astro/src/core/build/internal.ts
@@ -8,10 +8,13 @@ import type { PageBuildData, StylesheetAsset, ViteID } from './types';
 
 export interface BuildInternals {
 	/**
-	 * The module ids of all CSS chunks, used to deduplicate CSS assets between
-	 * SSR build and client build in vite-plugin-css.
+	 * Each CSS module is named with a chunk id derived from the Astro pages they
+	 * are used in by default. It's easy to crawl this relation in the SSR build as
+	 * the Astro pages are the entrypoint, but not for the client build as hydratable
+	 * components are the entrypoint instead. This map is used as a cache from the SSR
+	 * build so the client can pick up the same information and use the same chunk ids.
 	 */
-	cssChunkModuleIds: Set<string>;
+	cssModuleToChunkIdMap: Map<string, string>;
 
 	// A mapping of hoisted script ids back to the exact hoisted scripts it references
 	hoistedScriptIdToHoistedMap: Map<string, Set<string>>;
@@ -92,7 +95,7 @@ export function createBuildInternals(): BuildInternals {
 	const hoistedScriptIdToPagesMap = new Map<string, Set<string>>();
 
 	return {
-		cssChunkModuleIds: new Set(),
+		cssModuleToChunkIdMap: new Map(),
 		hoistedScriptIdToHoistedMap,
 		hoistedScriptIdToPagesMap,
 		entrySpecifierToBundleMap: new Map<string, string>(),
diff --git a/packages/astro/src/core/build/plugins/plugin-css.ts b/packages/astro/src/core/build/plugins/plugin-css.ts
index a5bdb70f1..1cec97223 100644
--- a/packages/astro/src/core/build/plugins/plugin-css.ts
+++ b/packages/astro/src/core/build/plugins/plugin-css.ts
@@ -64,20 +64,7 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
 	const cssBuildPlugin: VitePlugin = {
 		name: 'astro:rollup-plugin-build-css',
 
-		transform(_, id) {
-			// In the SSR build, styles that are bundled are tracked in `internals.cssChunkModuleIds`.
-			// In the client build, if we're also bundling the same style, return an empty string to
-			// deduplicate the final CSS output.
-			if (options.target === 'client' && internals.cssChunkModuleIds.has(id)) {
-				return '';
-			}
-		},
-
 		outputOptions(outputOptions) {
-			// Skip in client builds as its module graph doesn't have reference to Astro pages
-			// to be able to chunk based on it's related top-level pages.
-			if (options.target === 'client') return;
-
 			const assetFileNames = outputOptions.assetFileNames;
 			const namingIncludesHash = assetFileNames?.toString().includes('[hash]');
 			const createNameForParentPages = namingIncludesHash
@@ -89,16 +76,31 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
 					// For CSS, create a hash of all of the pages that use it.
 					// This causes CSS to be built into shared chunks when used by multiple pages.
 					if (isBuildableCSSRequest(id)) {
+						// For client builds that has hydrated components as entrypoints, there's no way
+						// to crawl up and find the pages that use it. So we lookup the cache during SSR
+						// build (that has the pages information) to derive the same chunk id so they
+						// match up on build, making sure both builds has the CSS deduped.
+						// NOTE: Components that are only used with `client:only` may not exist in the cache
+						// and that's okay. We can use Rollup's default chunk strategy instead as these CSS
+						// are outside of the SSR build scope, which no dedupe is needed.
+						if (options.target === 'client') {
+							return internals.cssModuleToChunkIdMap.get(id)!;
+						}
+
 						for (const [pageInfo] of walkParentInfos(id, {
 							getModuleInfo: meta.getModuleInfo,
 						})) {
 							if (new URL(pageInfo.id, 'file://').searchParams.has(PROPAGATED_ASSET_FLAG)) {
 								// Split delayed assets to separate modules
 								// so they can be injected where needed
-								return createNameHash(id, [id]);
+								const chunkId = createNameHash(id, [id]);
+								internals.cssModuleToChunkIdMap.set(id, chunkId);
+								return chunkId;
 							}
 						}
-						return createNameForParentPages(id, meta);
+						const chunkId = createNameForParentPages(id, meta);
+						internals.cssModuleToChunkIdMap.set(id, chunkId);
+						return chunkId;
 					}
 				},
 			});
@@ -113,15 +115,6 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
 				// Skip if the chunk has no CSS, we want to handle CSS chunks only
 				if (meta.importedCss.size < 1) continue;
 
-				// In the SSR build, keep track of all CSS chunks' modules as the client build may
-				// duplicate them, e.g. for `client:load` components that render in SSR and client
-				// for hydation.
-				if (options.target === 'server') {
-					for (const id of Object.keys(chunk.modules)) {
-						internals.cssChunkModuleIds.add(id);
-					}
-				}
-
 				// For the client build, client:only styles need to be mapped
 				// over to their page. For this chunk, determine if it's a child of a
 				// client:only component and if so, add its CSS to the page it belongs to.
diff --git a/packages/astro/test/0-css.test.js b/packages/astro/test/0-css.test.js
index 158389132..76bfba296 100644
--- a/packages/astro/test/0-css.test.js
+++ b/packages/astro/test/0-css.test.js
@@ -265,6 +265,21 @@ describe('CSS', function () {
 					new RegExp(`.svelte-scss.${scopedClass}[^{]*{font-family:ComicSansMS`)
 				);
 			});
+
+			it('client:only and SSR in two pages, both should have styles', async () => {
+				const onlyHtml = await fixture.readFile('/client-only-and-ssr/only/index.html');
+				const $onlyHtml = cheerio.load(onlyHtml);
+				const onlyHtmlCssHref = $onlyHtml('link[rel=stylesheet][href^=/_astro/]').attr('href');
+				const onlyHtmlCss = await fixture.readFile(onlyHtmlCssHref.replace(/^\/?/, '/'));
+
+				const ssrHtml = await fixture.readFile('/client-only-and-ssr/ssr/index.html');
+				const $ssrHtml = cheerio.load(ssrHtml);
+				const ssrHtmlCssHref = $ssrHtml('link[rel=stylesheet][href^=/_astro/]').attr('href');
+				const ssrHtmlCss = await fixture.readFile(ssrHtmlCssHref.replace(/^\/?/, '/'));
+
+				expect(onlyHtmlCss).to.include('.svelte-only-and-ssr');
+				expect(ssrHtmlCss).to.include('.svelte-only-and-ssr');
+			});
 		});
 
 		describe('Vite features', () => {
diff --git a/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/_components/SvelteOnlyAndSsr.svelte b/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/_components/SvelteOnlyAndSsr.svelte
new file mode 100644
index 000000000..f346f9c04
--- /dev/null
+++ b/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/_components/SvelteOnlyAndSsr.svelte
@@ -0,0 +1,11 @@
+<!-- This file will be used as client:only and SSR on two different pages -->
+
+<div class="svelte-only-and-ssr">
+	Svelte only and SSR
+</div>
+
+<style>
+	.svelte-only-and-ssr {
+		background-color: green;
+	}
+</style>
diff --git a/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/only.astro b/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/only.astro
new file mode 100644
index 000000000..b409693b5
--- /dev/null
+++ b/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/only.astro
@@ -0,0 +1,7 @@
+---
+import SvelteOnlyAndSsr from './_components/SvelteOnlyAndSsr.svelte'
+---
+
+<div>
+  <SvelteOnlyAndSsr client:only />
+</div>
diff --git a/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/ssr.astro b/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/ssr.astro
new file mode 100644
index 000000000..35b7fbae0
--- /dev/null
+++ b/packages/astro/test/fixtures/0-css/src/pages/client-only-and-ssr/ssr.astro
@@ -0,0 +1,7 @@
+---
+import SvelteOnlyAndSsr from './_components/SvelteOnlyAndSsr.svelte'
+---
+
+<div>
+  <SvelteOnlyAndSsr />
+</div>