From 93b092266febfad16a48575f8eee12d5910bf071 Mon Sep 17 00:00:00 2001
From: Bjorn Lu <bjornlu.dev@gmail.com>
Date: Mon, 9 Oct 2023 17:29:28 +0800
Subject: [PATCH] Make CSS chunk names less confusing (#8754)

---
 .changeset/sharp-insects-yawn.md              |  5 ++
 .../astro/src/core/build/css-asset-name.ts    | 54 ++++++++++++++++---
 .../src/core/build/plugins/plugin-css.ts      | 15 +-----
 3 files changed, 52 insertions(+), 22 deletions(-)
 create mode 100644 .changeset/sharp-insects-yawn.md

diff --git a/.changeset/sharp-insects-yawn.md b/.changeset/sharp-insects-yawn.md
new file mode 100644
index 000000000..742517633
--- /dev/null
+++ b/.changeset/sharp-insects-yawn.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Make CSS chunk names less confusing
diff --git a/packages/astro/src/core/build/css-asset-name.ts b/packages/astro/src/core/build/css-asset-name.ts
index 6e9e2a1c2..29fc14294 100644
--- a/packages/astro/src/core/build/css-asset-name.ts
+++ b/packages/astro/src/core/build/css-asset-name.ts
@@ -1,4 +1,4 @@
-import type { GetModuleInfo } from 'rollup';
+import type { GetModuleInfo, ModuleInfo } from 'rollup';
 
 import crypto from 'node:crypto';
 import npath from 'node:path';
@@ -6,20 +6,29 @@ import type { AstroSettings } from '../../@types/astro.js';
 import { viteID } from '../util.js';
 import { getTopLevelPages } from './graph.js';
 
+// These pages could be used as base names for the chunk hashed name, but they are confusing
+// and should be avoided it possible
+const confusingBaseNames = ['404', '500'];
+
 // 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';
+	return createNameHash(
+		getFirstParentId(parents),
+		parents.map(([page]) => page.id)
+	);
+}
 
+export function createNameHash(baseId: string | undefined, hashIds: string[]): string {
+	const baseName = baseId ? prettifyBaseName(npath.parse(baseId).name) : 'index';
 	const hash = crypto.createHash('sha256');
-	for (const [page] of parents) {
-		hash.update(page.id, 'utf-8');
+	for (const id of hashIds) {
+		hash.update(id, 'utf-8');
 	}
 	const h = hash.digest('hex').slice(0, 8);
-	const proposedName = firstParentName + '.' + h;
+	const proposedName = baseName + '.' + h;
 	return proposedName;
 }
 
@@ -34,7 +43,7 @@ export function createSlugger(settings: AstroSettings) {
 			.map(([page]) => page.id)
 			.sort()
 			.join('-');
-		const firstParentId = parents[0]?.[0].id || indexPage;
+		const firstParentId = getFirstParentId(parents) || indexPage;
 
 		// Use the last two segments, for ex /docs/index
 		let dir = firstParentId;
@@ -45,7 +54,7 @@ export function createSlugger(settings: AstroSettings) {
 				break;
 			}
 
-			const name = npath.parse(npath.basename(dir)).name;
+			const name = prettifyBaseName(npath.parse(npath.basename(dir)).name);
 			key = key.length ? name + sep + key : name;
 			dir = npath.dirname(dir);
 			i++;
@@ -76,3 +85,32 @@ export function createSlugger(settings: AstroSettings) {
 		return name;
 	};
 }
+
+/**
+ * Find the first parent id from `parents` where its name is not confusing.
+ * Returns undefined if there's no parents.
+ */
+function getFirstParentId(parents: [ModuleInfo, number, number][]) {
+	for (const parent of parents) {
+		const id = parent[0].id;
+		const baseName = npath.parse(id).name;
+		if (!confusingBaseNames.includes(baseName)) {
+			return id;
+		}
+	}
+	// If all parents are confusing, just use the first one. Or if there's no
+	// parents, this will return undefined.
+	return parents[0]?.[0].id;
+}
+
+const charsToReplaceRe = /[.\[\]]/g;
+const underscoresRe = /_+/g;
+/**
+ * Prettify base names so they're easier to read:
+ * - index -> index
+ * - [slug] -> _slug_
+ * - [...spread] -> _spread_
+ */
+function prettifyBaseName(str: string) {
+	return str.replace(charsToReplaceRe, '_').replace(underscoresRe, '_');
+}
diff --git a/packages/astro/src/core/build/plugins/plugin-css.ts b/packages/astro/src/core/build/plugins/plugin-css.ts
index 85652e13b..5e24f85d2 100644
--- a/packages/astro/src/core/build/plugins/plugin-css.ts
+++ b/packages/astro/src/core/build/plugins/plugin-css.ts
@@ -1,5 +1,3 @@
-import * as crypto from 'node:crypto';
-import * as npath from 'node:path';
 import type { GetModuleInfo } from 'rollup';
 import { type ResolvedConfig, type Plugin as VitePlugin } from 'vite';
 import { isBuildableCSSRequest } from '../../../vite-plugin-astro-server/util.js';
@@ -93,7 +91,7 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
 							if (new URL(pageInfo.id, 'file://').searchParams.has(PROPAGATED_ASSET_FLAG)) {
 								// Split delayed assets to separate modules
 								// so they can be injected where needed
-								const chunkId = createNameHash(id, [id]);
+								const chunkId = assetName.createNameHash(id, [id]);
 								internals.cssModuleToChunkIdMap.set(id, chunkId);
 								return chunkId;
 							}
@@ -272,17 +270,6 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
 
 /***** UTILITY FUNCTIONS *****/
 
-function createNameHash(baseId: string, hashIds: string[]): string {
-	const baseName = baseId ? npath.parse(baseId).name : 'index';
-	const hash = crypto.createHash('sha256');
-	for (const id of hashIds) {
-		hash.update(id, 'utf-8');
-	}
-	const h = hash.digest('hex').slice(0, 8);
-	const proposedName = baseName + '.' + h;
-	return proposedName;
-}
-
 function* getParentClientOnlys(
 	id: string,
 	ctx: { getModuleInfo: GetModuleInfo },