fix: assets referenced via js chunks are not flagged as orphaned

.changeset/kind-squids-warn.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Reintroduces css deduplication for hydrated client components. Ensures assets already added to a client chunk are not flagged as orphaned

packages/astro/src/core/build/plugins/plugin-css.ts

@@ -80,7 +80,10 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
 						// 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)!;
+							// Find the chunkId for this CSS module in the server build.
+							// If it exists, we can use it to ensure the client build matches the server
+							// build and doesn't create a duplicate chunk.
+							return internals.cssModuleToChunkIdMap.get(id);
 						}
 
 						const ctx = { getModuleInfo: meta.getModuleInfo };
@@ -93,6 +96,7 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
 								return chunkId;
 							}
 						}
+
 						const chunkId = createNameForParentPages(id, meta);
 						internals.cssModuleToChunkIdMap.set(id, chunkId);
 						return chunkId;
@@ -230,8 +234,19 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
 					sheetAddedToPage = true;
 				}
 
-				if (toBeInlined && sheetAddedToPage) {
-					// CSS is already added to all used pages, we can delete it from the bundle
+				const wasInlined = toBeInlined && sheetAddedToPage;				
+				// stylesheets already referenced as an asset by a chunk will not be inlined by
+				// this plugin, but should not be considered orphaned
+				const wasAddedToChunk = Object.values(bundle).some((chunk) =>
+					chunk.type === 'chunk' &&
+					chunk.viteMetadata?.importedAssets?.has(id)
+				);
+				const isOrphaned = !sheetAddedToPage && !wasAddedToChunk;
+
+				if (wasInlined || isOrphaned) {
+					// wasInlined : CSS is already added to all used pages
+					// isOrphaned : CSS is already used in a merged chunk
+					// we can delete it from the bundle
 					// and make sure no chunks reference it via `importedCss` (for Vite preloading)
 					// to avoid duplicate CSS.
 					delete bundle[id];

packages/astro/test/css-deduplication.test.js

@@ -0,0 +1,81 @@
+import * as assert from 'node:assert/strict';
+import { before, describe, it } from 'node:test';
+import * as cheerio from 'cheerio';
+import { loadFixture } from './test-utils.js';
+
+describe('CSS deduplication for hydrated components', () => {
+	describe('inlineStylesheets: never', () => {
+		let fixture;
+		before(async () => {
+			fixture = await loadFixture({
+				site: 'https://test.dev/',
+				root: './fixtures/css-deduplication/',
+				build: { inlineStylesheets: 'never' },
+				outDir: './dist/inline-stylesheets-never',
+			});
+			await fixture.build();
+		});
+
+		it('should not duplicate CSS for hydrated components', async () => {
+			const assets = await fixture.readdir('/_astro');
+
+			// Generated file for Counter.css
+			const COUNTER_CSS_PATH = '/_astro/index.DbgLc3FE.css';
+			let file = await fixture.readFile(COUNTER_CSS_PATH);
+			file = file.replace(/\s+/g, '');
+
+			for (const fileName of assets) {
+				const filePath = `/_astro/${fileName}`;
+				if (filePath === COUNTER_CSS_PATH || !fileName.endsWith('.css')) {
+					continue;
+				}
+
+				let r = await fixture.readFile(filePath);
+				r = r.replace(/\s+/g, '');
+				if (file.includes(r)) {
+					assert.fail(`Duplicate CSS file: ${fileName}`);
+				}
+			}
+			assert.ok(true);
+		});
+	});
+
+	describe('inlineStylesheets: always', () => {
+		let fixture;
+		before(async () => {
+			fixture = await loadFixture({
+				site: 'https://test.dev/',
+				root: './fixtures/css-deduplication/',
+				build: { inlineStylesheets: 'always' },
+				outDir: './dist/inline-stylesheets-always',
+			});
+			await fixture.build();
+		});
+
+		it('should not emit any .css file when inlineStylesheets is always', async () => {
+			const assets = await fixture.readdir('/_astro');
+			assert.ok(!assets.some((f) => f.endsWith('.css')));
+		});
+
+		it('should not duplicate CSS for hydrated components', async () => {
+			const html = await fixture.readFile('/index.html');
+			const $ = cheerio.load(html);
+
+			// Get all <style> tag contents
+			const styles = [];
+			$('style').each((_i, el) => {
+				styles.push($(el).text().replace(/\s+/g, ''));
+			});
+
+			// Ensure no <style> tag content is duplicated
+			const seen = new Set();
+			for (const style of styles) {
+				if (seen.has(style)) {
+					assert.fail('Duplicate <style> tag content found in index.html');
+				}
+				seen.add(style);
+			}
+			assert.ok(true);
+		});
+	});
+});

packages/astro/test/fixtures/css-deduplication/astro.config.mjs

@@ -0,0 +1,10 @@
+// @ts-check
+
+import react from '@astrojs/react';
+import { defineConfig } from 'astro/config';
+
+// https://astro.build/config
+export default defineConfig({
+	// Enable React to support React JSX components.
+	integrations: [react()]
+});

packages/astro/test/fixtures/css-deduplication/package.json

@@ -0,0 +1,13 @@
+{
+  "name": "@test/css-deduplication",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*",
+    "@astrojs/react": "workspace:*",
+    "@types/react": "^18.3.20",
+    "@types/react-dom": "^18.3.6",
+    "react": "^18.3.1",
+    "react-dom": "^18.3.1"
+  }
+}

packages/astro/test/fixtures/css-deduplication/src/components/Counter.css

@@ -0,0 +1,11 @@
+.counter {
+	display: grid;
+	font-size: 2em;
+	grid-template-columns: repeat(3, minmax(0, 1fr));
+	margin-top: 2em;
+	place-items: center;
+}
+
+.counter-message {
+	text-align: center;
+}

packages/astro/test/fixtures/css-deduplication/src/components/Counter.tsx

@@ -0,0 +1,25 @@
+import { useState } from 'react';
+import './Counter.css';
+
+export default function Counter({
+	children,
+	count: initialCount,
+}: {
+	children: JSX.Element;
+	count: number;
+}) {
+	const [count, setCount] = useState(initialCount);
+	const add = () => setCount((i) => i + 1);
+	const subtract = () => setCount((i) => i - 1);
+
+	return (
+		<>
+			<div className="counter">
+				<button onClick={subtract}>-</button>
+				<pre>{count}</pre>
+				<button onClick={add}>+</button>
+			</div>
+			<div className="counter-message">{children}</div>
+		</>
+	);
+}

packages/astro/test/fixtures/css-deduplication/src/pages/index.astro

@@ -0,0 +1,37 @@
+---
+// Component Imports
+import Counter from '../components/Counter';
+
+const someProps = {
+	count: 0,
+};
+
+// Full Astro Component Syntax:
+// https://docs.astro.build/basics/astro-components/
+---
+
+<html lang="en">
+	<head>
+		<meta charset="utf-8" />
+		<meta name="viewport" content="width=device-width" />
+		<meta name="generator" content={Astro.generator} />
+		<link rel="icon" type="image/svg+xml" href="/favicon.svg" />
+		<style>
+			html,
+			body {
+				font-family: system-ui;
+				margin: 0;
+			}
+			body {
+				padding: 2rem;
+			}
+		</style>
+	</head>
+	<body>
+		<main>
+			<Counter {...someProps} client:visible>
+				<h1>Hello, React!</h1>
+			</Counter>
+		</main>
+	</body>
+</html>

packages/astro/test/fixtures/css-deduplication/tsconfig.json

@@ -0,0 +1,9 @@
+{
+  "extends": "astro/tsconfigs/strict",
+  "include": [".astro/types.d.ts", "**/*"],
+  "exclude": ["dist"],
+  "compilerOptions": {
+    "jsx": "react-jsx",
+    "jsxImportSource": "react"
+  }
+}

packages/astro/test/fixtures/url-import-suffix/astro.config.mjs

@@ -0,0 +1,5 @@
+// @ts-check
+import { defineConfig } from 'astro/config';
+
+// https://astro.build/config
+export default defineConfig({});

packages/astro/test/fixtures/url-import-suffix/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "@test/url-import-suffix",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*"
+  }
+}

packages/astro/test/fixtures/url-import-suffix/src/assets/index.css

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

packages/astro/test/fixtures/url-import-suffix/src/assets/style.css

@@ -0,0 +1,3 @@
+h1 {
+  background: green;
+}

packages/astro/test/fixtures/url-import-suffix/src/pages/index.astro

@@ -0,0 +1,20 @@
+---
+import assetsUrl from '../assets/index.css?url';
+import assetsUrlNoInline from '../assets/style.css?url&no-inline';
+---
+
+<html lang="en">
+	<head>
+		<meta charset="utf-8" />
+		<link rel="icon" type="image/svg+xml" href="/favicon.svg" />
+		<meta name="viewport" content="width=device-width" />
+		<meta name="generator" content={Astro.generator} />
+		<title>Astro</title>
+		<link rel="stylesheet" href={assetsUrl} />
+		<link rel="stylesheet" href={assetsUrlNoInline} />
+	</head>
+	<body>
+		<h1>Astro</h1>
+	</body>
+</html>
+

packages/astro/test/fixtures/url-import-suffix/tsconfig.json

@@ -0,0 +1,5 @@
+{
+  "extends": "astro/tsconfigs/strict",
+  "include": [".astro/types.d.ts", "**/*"],
+  "exclude": ["dist"],
+}

packages/astro/test/url-import-suffix.test.js

@@ -0,0 +1,50 @@
+import * as assert from 'node:assert/strict';
+import { before, describe, it } from 'node:test';
+import * as cheerio from 'cheerio';
+import { loadFixture } from './test-utils.js';
+
+describe('imports using ?url suffix', () => {
+  let fixture;
+  const assetName = 'index.DqQksVyv.css'
+
+  before(async () => {
+		fixture = await loadFixture({ root: './fixtures/url-import-suffix/' });
+		await fixture.build();
+	});
+
+  it('includes the built asset in the output', async () => {
+    const assets = await fixture.readdir('/_astro');
+    assert.ok(assets.some((f) => f === assetName));
+  });
+
+  it('links the asset in the html', async () => {
+    const html = await fixture.readFile('/index.html');
+    const $ = cheerio.load(html);
+
+    const linkHref = $('link[rel="stylesheet"]').attr('href');
+    assert.ok(linkHref, `/_astro/${assetName}`);
+  });
+})
+
+describe('imports using ?url&no-inline suffix', () => {
+  let fixture;
+  const assetName = 'style.3WhucSPm.css';
+
+  before(async () => {
+		fixture = await loadFixture({ root: './fixtures/url-import-suffix/' });
+		await fixture.build();
+	});
+
+  it('includes the built asset in the output', async () => {
+    const assets = await fixture.readdir('/_astro');
+    assert.ok(assets.some((f) => f === assetName));
+  });
+
+  it('links the asset in the html', async () => {
+    const html = await fixture.readFile('/index.html');
+    const $ = cheerio.load(html);
+
+    const linkHref = $('link[rel="stylesheet"]').attr('href');
+    assert.ok(linkHref, `/_astro/${assetName}`);
+  });
+});