Updated CSS naming algorithm (#5353)

* Updated CSS naming algorithm

* Adding a changeset

* Fix windows

.changeset/shiny-baboons-trade.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Updated the CSS naming algorithm to prevent clashes

packages/astro/src/core/build/css-asset-name.ts

@@ -0,0 +1,77 @@
+import type { GetModuleInfo } from 'rollup';
+
+import crypto from 'crypto';
+import npath from 'path';
+import { getTopLevelPages } from './graph.js';
+import { AstroSettings } from '../../@types/astro';
+import { viteID } from '../util.js';
+
+// The short name for when the hash can be included
+// We could get rid of this and only use the createSlugger implementation, but this creates
+// slightly prettier names.
+export function shortHashedName(id: string, ctx: { getModuleInfo: GetModuleInfo }): string {
+	const parents = Array.from(getTopLevelPages(id, ctx));
+	const firstParentId = parents[0]?.[0].id;
+	const firstParentName = firstParentId ? npath.parse(firstParentId).name : 'index';
+
+	const hash = crypto.createHash('sha256');
+	for (const [page] of parents) {
+		hash.update(page.id, 'utf-8');
+	}
+	const h = hash.digest('hex').slice(0, 8);
+	const proposedName = firstParentName + '.' + h;
+	return proposedName;
+}
+
+export function createSlugger(settings: AstroSettings) {
+	const pagesDir = viteID(new URL('./pages', settings.config.srcDir));
+	const map = new Map<string, Map<string, number>>();
+	const sep = '-';
+	return function(id: string, ctx: { getModuleInfo: GetModuleInfo }): string {
+		const parents = Array.from(getTopLevelPages(id, ctx));
+		const allParentsKey = parents
+		.map(([page]) => page.id)
+		.sort()
+		.join('-');
+		const firstParentId = parents[0]?.[0].id || 'index';
+
+		// Use the last two segments, for ex /docs/index
+		let dir = firstParentId;
+		let key = '';
+		let i = 0;
+		while (i < 2) {
+			if(dir === pagesDir) {
+				break;
+			}
+
+			const name = npath.parse(npath.basename(dir)).name;
+			key = key.length ? (name + sep + key) : name;
+			dir = npath.dirname(dir);
+			i++;
+		}
+
+		// Keep track of how many times this was used.
+		let name = key;
+
+		// The map keeps track of how many times a key, like `pages_index` is used as the name.
+		// If the same key is used more than once we increment a number so it becomes `pages-index-1`.
+		// This guarantees that it stays unique, without sacrificing pretty names.
+		if(!map.has(key)) {
+			map.set(key, new Map([[allParentsKey, 0]]));
+		} else {
+			const inner = map.get(key)!;
+			if(inner.has(allParentsKey)) {
+				const num = inner.get(allParentsKey)!;
+				if(num > 0) {
+					name = name + sep + num;
+				}
+			} else {
+				const num = inner.size;
+				inner.set(allParentsKey, num);
+				name = name + sep + num;
+			}
+		}
+	
+		return name;
+	};
+}

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

@@ -2,12 +2,11 @@ import type { GetModuleInfo } from 'rollup';
 import type { BuildInternals } from './internal';
 import type { PageBuildData, StaticBuildOptions } from './types';
 
-import crypto from 'crypto';
 import esbuild from 'esbuild';
-import npath from 'path';
 import { Plugin as VitePlugin, ResolvedConfig } from 'vite';
 import { isCSSRequest } from '../render/util.js';
-import { getTopLevelPages, moduleIsTopLevelPage, walkParentInfos } from './graph.js';
+
+import { moduleIsTopLevelPage, walkParentInfos } from './graph.js';
 import {
 	eachPageData,
 	getPageDataByViteID,
@@ -15,6 +14,7 @@ import {
 	getPageDatasByHoistedScriptId,
 	isHoistedScript,
 } from './internal.js';
+import * as assetName from './css-asset-name.js';
 
 interface PluginOptions {
 	internals: BuildInternals;
@@ -28,20 +28,6 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
 
 	let resolvedConfig: ResolvedConfig;
 
-	function createNameForParentPages(id: string, ctx: { getModuleInfo: GetModuleInfo }): string {
-		const parents = Array.from(getTopLevelPages(id, ctx));
-		const firstParentId = parents[0]?.[0].id;
-		const firstParentName = firstParentId ? npath.parse(firstParentId).name : 'index';
-
-		const hash = crypto.createHash('sha256');
-		for (const [page] of parents) {
-			hash.update(page.id, 'utf-8');
-		}
-		const h = hash.digest('hex').slice(0, 8);
-		const proposedName = firstParentName + '.' + h;
-		return proposedName;
-	}
-
 	function* getParentClientOnlys(
 		id: string,
 		ctx: { getModuleInfo: GetModuleInfo }
@@ -57,6 +43,9 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
 
 			outputOptions(outputOptions) {
 				const manualChunks = outputOptions.manualChunks || Function.prototype;
+				const assetFileNames = outputOptions.assetFileNames;
+				const namingIncludesHash = assetFileNames?.toString().includes('[hash]');
+				const createNameForParentPages = namingIncludesHash ? assetName.shortHashedName : assetName.createSlugger(settings);
 				outputOptions.manualChunks = function (id, ...args) {
 					// Defer to user-provided `manualChunks`, if it was provided.
 					if (typeof manualChunks == 'object') {

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

@@ -19,42 +19,91 @@ const UNEXPECTED_CSS = [
 ];
 
 describe('CSS Bundling', function () {
+	/** @type {import('./test-utils').Fixture} */
 	let fixture;
 
-	before(async () => {
+	describe('defaults', () => {
-		fixture = await loadFixture({
+		before(async () => {
-			root: './fixtures/astro-css-bundling/',
+			fixture = await loadFixture({
+				root: './fixtures/astro-css-bundling/',
+			});
+			await fixture.build({ mode: 'production' });
+		});
+	
+		it('Bundles CSS', async () => {
+			const builtCSS = new Set();
+	
+			// for all HTML files…
+			for (const [filepath, css] of Object.entries(EXPECTED_CSS)) {
+				const html = await fixture.readFile(filepath);
+				const $ = cheerio.load(html);
+	
+				// test 1: assert new bundled CSS is present
+				for (const href of css) {
+					const link = $(`link[rel="stylesheet"][href^="${href}"]`);
+					expect(link.length).to.be.greaterThanOrEqual(1);
+					const outHref = link.attr('href');
+					builtCSS.add(outHref.startsWith('../') ? outHref.slice(2) : outHref);
+				}
+	
+				// test 2: assert old CSS was removed
+				for (const href of UNEXPECTED_CSS) {
+					const link = $(`link[rel="stylesheet"][href="${href}"]`);
+					expect(link).to.have.lengthOf(0);
+				}
+	
+				// test 3: assert all bundled CSS was built and contains CSS
+				for (const url of builtCSS.keys()) {
+					const bundledCss = await fixture.readFile(url);
+					expect(bundledCss).to.be.ok;
+				}
+			}
+		});
+
+		it('there are 4 css files', async () => {
+			const dir = await fixture.readdir('/assets');
+			expect(dir).to.have.a.lengthOf(4);
+		});
+
+		it('CSS includes hashes', async () => {
+			const [firstFound] = await fixture.readdir('/assets');
+			expect(firstFound).to.match(/[a-z]+\.[0-9a-z]{8}\.css/);
 		});
-		await fixture.build({ mode: 'production' });
 	});
 
-	it('Bundles CSS', async () => {
+	describe('using custom assetFileNames config', () => {
-		const builtCSS = new Set();
+		before(async () => {
+			fixture = await loadFixture({
+				root: './fixtures/astro-css-bundling/',
 
-		// for all HTML files…
+				vite: {
-		for (const [filepath, css] of Object.entries(EXPECTED_CSS)) {
+					build: {
-			const html = await fixture.readFile(filepath);
+						rollupOptions: {
-			const $ = cheerio.load(html);
+							output: {
+								assetFileNames: "assets/[name][extname]",
+								entryFileNames: "[name].js",
+							}
+						}
+					}
+				}
+			});
+			await fixture.build({ mode: 'production' });
+		});
 
-			// test 1: assert new bundled CSS is present
+		it('there are 4 css files', async () => {
-			for (const href of css) {
+			const dir = await fixture.readdir('/assets');
-				const link = $(`link[rel="stylesheet"][href^="${href}"]`);
+			expect(dir).to.have.a.lengthOf(4);
-				expect(link.length).to.be.greaterThanOrEqual(1);
+		});
-				const outHref = link.attr('href');
-				builtCSS.add(outHref.startsWith('../') ? outHref.slice(2) : outHref);
-			}
 
-			// test 2: assert old CSS was removed
+		it('CSS does not include hashes hashes', async () => {
-			for (const href of UNEXPECTED_CSS) {
+			const [firstFound] = await fixture.readdir('/assets');
-				const link = $(`link[rel="stylesheet"][href="${href}"]`);
+			expect(firstFound).to.not.match(/[a-z]+\.[0-9a-z]{8}\.css/);
-				expect(link).to.have.lengthOf(0);
+		});
-			}
 
-			// test 3: assert all bundled CSS was built and contains CSS
+		it('there are 2 index named CSS files', async () => {
-			for (const url of builtCSS.keys()) {
+			const dir = await fixture.readdir('/assets');
-				const bundledCss = await fixture.readFile(url);
+			const indexNamedFiles = dir.filter(name => name.startsWith('index'))
-				expect(bundledCss).to.be.ok;
+			expect(indexNamedFiles).to.have.a.lengthOf(2);
-			}
+		});
-		}
 	});
 });