Use CSS depth to sort CSS in production (#4446)

* Use CSS depth to sort CSS in production

* Adds a changeset

.changeset/nine-pants-leave.md

@@ -0,0 +1,9 @@
+---
+'astro': patch
+---
+
+Deterministic CSS ordering
+
+This makes our CSS link order deterministic. It uses CSS depth; that is how deeply a module import the CSS comes from, in order to determine which CSS is page-level vs. component-level CSS.
+
+This is intended to match dev ordering where, because we do not bundle, the page-level CSS always comes after component-level.

packages/astro/src/core/build/generate.ts

@@ -28,7 +28,7 @@ import { createRequest } from '../request.js';
 import { matchRoute } from '../routing/match.js';
 import { getOutputFilename } from '../util.js';
 import { getOutFile, getOutFolder } from './common.js';
-import { eachPageData, getPageDataByComponent } from './internal.js';
+import { eachPageData, getPageDataByComponent, sortedCSS } from './internal.js';
 import type { PageBuildData, SingleFileBuiltModule, StaticBuildOptions } from './types';
 import { getTimeStat } from './util.js';
 
@@ -124,7 +124,7 @@ async function generatePage(
 	const renderers = ssrEntry.renderers;
 
 	const pageInfo = getPageDataByComponent(internals, pageData.route.component);
-	const linkIds: string[] = Array.from(pageInfo?.css ?? []);
+	const linkIds: string[] = sortedCSS(pageData);
 	const scripts = pageInfo?.hoistedScript ?? null;
 
 	const pageModule = ssrEntry.pageMap.get(pageData.component);
@@ -264,7 +264,7 @@ async function generatePath(
 		astroConfig.base !== '/'
 			? joinPaths(astroConfig.site?.toString() || 'http://localhost/', astroConfig.base)
 			: astroConfig.site;
-	const links = createLinkStylesheetElementSet(linkIds.reverse(), site);
+	const links = createLinkStylesheetElementSet(linkIds, site);
 	const scripts = createModuleScriptsSet(hoistedScripts ? [hoistedScripts] : [], site);
 
 	if (astroConfig._ctx.scripts.some((script) => script.stage === 'page')) {

packages/astro/src/core/build/graph.ts

@@ -5,19 +5,20 @@ import { resolvedPagesVirtualModuleId } from '../app/index.js';
 export function* walkParentInfos(
 	id: string,
 	ctx: { getModuleInfo: GetModuleInfo },
+	depth = 0,
 	seen = new Set<string>()
-): Generator<ModuleInfo, void, unknown> {
+): Generator<[ModuleInfo, number], void, unknown> {
 	seen.add(id);
 	const info = ctx.getModuleInfo(id);
 	if (info) {
-		yield info;
+		yield [info, depth];
 	}
 	const importers = (info?.importers || []).concat(info?.dynamicImporters || []);
 	for (const imp of importers) {
 		if (seen.has(imp)) {
 			continue;
 		}
-		yield* walkParentInfos(imp, ctx, seen);
+		yield* walkParentInfos(imp, ctx, ++depth, seen);
 	}
 }
 
@@ -26,10 +27,10 @@ export function* walkParentInfos(
 export function* getTopLevelPages(
 	id: string,
 	ctx: { getModuleInfo: GetModuleInfo }
-): Generator<ModuleInfo, void, unknown> {
+): Generator<[ModuleInfo, number], void, unknown> {
-	for (const info of walkParentInfos(id, ctx)) {
+	for (const res of walkParentInfos(id, ctx)) {
-		if (info?.importers[0] === resolvedPagesVirtualModuleId) {
+		if (res[0]?.importers[0] === resolvedPagesVirtualModuleId) {
-			yield info;
+			yield res;
 		}
 	}
 }

packages/astro/src/core/build/internal.ts

@@ -181,3 +181,24 @@ export function hasPageDataByViteID(internals: BuildInternals, viteid: ViteID):
 export function* eachPageData(internals: BuildInternals) {
 	yield* internals.pagesByComponent.values();
 }
+
+/**
+ * Sort a page's CSS by depth. A higher depth means that the CSS comes from shared subcomponents.
+ * A lower depth means it comes directly from the top-level page.
+ * The return of this function is an array of CSS paths, with shared CSS on top
+ * and page-level CSS on bottom.
+ */
+export function sortedCSS(pageData: PageBuildData) {
+	return Array.from(pageData.css).sort((a, b) => {
+		let depthA = a[1].depth,
+			depthB = b[1].depth;
+
+		if(depthA === -1) {
+			return -1;
+		} else if(depthB === -1) {
+			return 1;
+		} else {
+			return depthA > depthB ? -1 : 1;
+		}
+	}).map(([id]) => id);
+}

packages/astro/src/core/build/page-data.ts

@@ -53,7 +53,7 @@ export async function collectPagesData(
 				component: route.component,
 				route,
 				moduleSpecifier: '',
-				css: new Set(),
+				css: new Map(),
 				hoistedScript: undefined,
 			};
 
@@ -74,7 +74,7 @@ export async function collectPagesData(
 			component: route.component,
 			route,
 			moduleSpecifier: '',
-			css: new Set(),
+			css: new Map(),
 			hoistedScript: undefined,
 		};
 	}

packages/astro/src/core/build/types.ts

@@ -18,7 +18,7 @@ export interface PageBuildData {
 	component: ComponentPath;
 	route: RouteData;
 	moduleSpecifier: string;
-	css: Set<string>;
+	css: Map<string, { depth: number }>;
 	hoistedScript: { type: 'inline' | 'external'; value: string } | undefined;
 }
 export type AllPagesData = Record<ComponentPath, PageBuildData>;

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

@@ -22,7 +22,7 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin {
 				}
 
 				if (hoistedScripts.size) {
-					for (const pageInfo of getTopLevelPages(from, this)) {
+					for (const [pageInfo] of getTopLevelPages(from, this)) {
 						const pageId = pageInfo.id;
 						for (const hid of hoistedScripts) {
 							if (pageScripts.has(pageId)) {
@@ -98,7 +98,7 @@ export function vitePluginAnalyzer(internals: BuildInternals): VitePlugin {
 						clientOnlys.push(cid);
 					}
 
-					for (const pageInfo of getTopLevelPages(id, this)) {
+					for (const [pageInfo] of getTopLevelPages(id, this)) {
 						const pageData = getPageDataByViteID(internals, pageInfo.id);
 						if (!pageData) continue;
 

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

@@ -42,8 +42,8 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
 	}
 
 	function createNameForParentPages(id: string, ctx: { getModuleInfo: GetModuleInfo }): string {
-		const parents = Array.from(getTopLevelPages(id, ctx)).sort();
+		const parents = Array.from(getTopLevelPages(id, ctx));
-		const proposedName = parents.map((page) => nameifyPage(page.id)).join('-');
+		const proposedName = parents.map(([page]) => nameifyPage(page.id)).sort().join('-');
 
 		// We don't want absurdedly long chunk names, so if this is too long create a hashed version instead.
 		if (proposedName.length <= MAX_NAME_LENGTH) {
@@ -51,7 +51,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
 		}
 
 		const hash = crypto.createHash('sha256');
-		for (const page of parents) {
+		for (const [page] of parents) {
 			hash.update(page.id, 'utf-8');
 		}
 		return hash.digest('hex').slice(0, 8);
@@ -61,7 +61,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
 		id: string,
 		ctx: { getModuleInfo: GetModuleInfo }
 	): Generator<PageBuildData, void, unknown> {
-		for (const info of walkParentInfos(id, ctx)) {
+		for (const [info] of walkParentInfos(id, ctx)) {
 			yield* getPageDatasByClientOnlyID(internals, info.id);
 		}
 	}
@@ -107,6 +107,10 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
 
 							// Chunks that have the viteMetadata.importedCss are CSS chunks
 							if (meta.importedCss.size) {
+								//console.log(meta.importedCss, c.fileName);
+
+								debugger;
+
 								// 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.
@@ -114,7 +118,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
 									for (const [id] of Object.entries(c.modules)) {
 										for (const pageData of getParentClientOnlys(id, this)) {
 											for (const importedCssImport of meta.importedCss) {
-												pageData.css.add(importedCssImport);
+												pageData.css.set(importedCssImport, { depth: -1 });
 											}
 										}
 									}
@@ -122,11 +126,23 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
 
 								// For this CSS chunk, walk parents until you find a page. Add the CSS to that page.
 								for (const [id] of Object.entries(c.modules)) {
-									for (const pageInfo of getTopLevelPages(id, this)) {
+									debugger;
+									for (const [pageInfo, depth] of getTopLevelPages(id, this)) {
 										const pageViteID = pageInfo.id;
 										const pageData = getPageDataByViteID(internals, pageViteID);
+
 										for (const importedCssImport of meta.importedCss) {
-											pageData?.css.add(importedCssImport);
+											// CSS is prioritized based on depth. Shared CSS has a higher depth due to being imported by multiple pages.
+											// Depth info is used when sorting the links on the page.
+											if(pageData?.css.has(importedCssImport)) {
+												// eslint-disable-next-line
+												const cssInfo = pageData?.css.get(importedCssImport)!;
+												if(depth < cssInfo.depth) {
+													cssInfo.depth = depth;
+												}
+											} else {
+												pageData?.css.set(importedCssImport, { depth });
+											}	
 										}
 									}
 								}

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

@@ -12,7 +12,7 @@ import { BEFORE_HYDRATION_SCRIPT_ID, PAGE_SCRIPT_ID } from '../../vite-plugin-sc
 import { pagesVirtualModuleId } from '../app/index.js';
 import { serializeRouteData } from '../routing/index.js';
 import { addRollupInput } from './add-rollup-input.js';
-import { eachPageData } from './internal.js';
+import { eachPageData, sortedCSS } from './internal.js';
 
 export const virtualModuleId = '@astrojs-ssr-virtual-entry';
 const resolvedVirtualModuleId = '\0' + virtualModuleId;
@@ -139,7 +139,7 @@ function buildManifest(
 
 		routes.push({
 			file: '',
-			links: Array.from(pageData.css).reverse(),
+			links: sortedCSS(pageData),
 			scripts: [
 				...scripts,
 				...astroConfig._ctx.scripts

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

@@ -4,13 +4,6 @@ import { loadFixture } from './test-utils.js';
 import testAdapter from './test-adapter.js';
 
 describe('CSS production ordering', () => {
-	let staticHTML, serverHTML;
-	let staticCSS, serverCSS;
-
-	const commonConfig = Object.freeze({
-		root: './fixtures/css-order/',
-	});
-
 	function getLinks(html) {
 		let $ = cheerio.load(html);
 		let out = [];
@@ -20,42 +13,85 @@ describe('CSS production ordering', () => {
 		return out;
 	}
 
-	before(async () => {
+	/**
-		let fixture = await loadFixture({ ...commonConfig });
+	 * 
-		await fixture.build();
+	 * @param {import('./test-utils').Fixture} fixture 
-		staticHTML = await fixture.readFile('/one/index.html');
+	 * @param {string} href 
-		staticCSS = await Promise.all(
+	 * @returns {Promise<{ href: string; css: string; }>}
-			getLinks(staticHTML).map(async (href) => {
+	 */
-				const css = await fixture.readFile(href);
+	async function getLinkContent(fixture, href) {
-				return { href, css };
+		const css = await fixture.readFile(href);
-			})
+		return { href, css };
-		);
+	}
-	});
 
-	before(async () => {
+	describe('SSG and SSR parity', () => {
-		let fixture = await loadFixture({
+		let staticHTML, serverHTML;
-			...commonConfig,
+		let staticCSS, serverCSS;
-			adapter: testAdapter(),
+	
-			output: 'server',
+		const commonConfig = Object.freeze({
+			root: './fixtures/css-order/',
 		});
-		await fixture.build();
+	
 
-		const app = await fixture.loadTestAdapterApp();
+	
-		const request = new Request('http://example.com/one');
+		before(async () => {
-		const response = await app.render(request);
+			let fixture = await loadFixture({ ...commonConfig });
-		serverHTML = await response.text();
+			await fixture.build();
-		serverCSS = await Promise.all(
+			staticHTML = await fixture.readFile('/one/index.html');
-			getLinks(serverHTML).map(async (href) => {
+			staticCSS = await Promise.all(
-				const css = await fixture.readFile(`/client${href}`);
+				getLinks(staticHTML).map(href => getLinkContent(fixture, href))
-				return { href, css };
+			);
-			})
+		});
-		);
+	
+		before(async () => {
+			let fixture = await loadFixture({
+				...commonConfig,
+				adapter: testAdapter(),
+				output: 'server',
+			});
+			await fixture.build();
+	
+			const app = await fixture.loadTestAdapterApp();
+			const request = new Request('http://example.com/one');
+			const response = await app.render(request);
+			serverHTML = await response.text();
+			serverCSS = await Promise.all(
+				getLinks(serverHTML).map(async (href) => {
+					const css = await fixture.readFile(`/client${href}`);
+					return { href, css };
+				})
+			);
+		});
+	
+		it('is in the same order for output: server and static', async () => {
+			const staticContent = staticCSS.map((o) => o.css);
+			const serverContent = serverCSS.map((o) => o.css);
+	
+			expect(staticContent).to.deep.equal(serverContent);
+		});
 	});
 
-	it('is in the same order for output: server and static', async () => {
+	describe('Page vs. Shared CSS', () => {
-		const staticContent = staticCSS.map((o) => o.css);
+		/** @type {import('./test-utils').Fixture} */
-		const serverContent = serverCSS.map((o) => o.css);
+		let fixture;
+		before(async () => {
+			fixture = await loadFixture({
+				root: './fixtures/css-order/',
+			});
+			await fixture.build();
+		});
 
-		expect(staticContent).to.deep.equal(serverContent);
+		it('Page level CSS is defined lower in the page', async () => {
+			let html = await fixture.readFile('/two/index.html');
+
+			const content = await Promise.all(
+				getLinks(html).map(href => getLinkContent(fixture, href))
+			);
+
+			expect(content).to.have.a.lengthOf(2, 'there are 2 stylesheets');
+			const [,last] = content;
+
+			expect(last.css).to.match(/#00f/);
+		});
 	});
 });

packages/astro/test/fixtures/css-order/src/pages/two.astro

@@ -8,6 +8,7 @@ import CommonHead from '../components/CommonHead.astro';
 		<style>
 			body {
 				margin: 2px;
+				color: blue;
 			}
 		</style>
 	</head>