Fix duplicated CSS modules inlining

withastro · Dec 27, 2023 · 79aa10a · 79aa10a
1 parent cf993bc
commit 79aa10a
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 24 deletions.
diff --git a/.changeset/blue-ladybugs-march.md b/.changeset/blue-ladybugs-march.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes duplicated CSS modules content when it's imported by both Astro files and framework components
diff --git a/packages/astro/src/vite-plugin-astro-server/css.ts b/packages/astro/src/vite-plugin-astro-server/css.ts
@@ -22,28 +22,46 @@ export async function getStylesForURL(
 
 	for await (const importedModule of crawlGraph(loader, viteID(filePath), true)) {
 		if (isBuildableCSSRequest(importedModule.url)) {
-			let ssrModule: Record<string, any>;
-			try {
-				// The SSR module is possibly not loaded. Load it if it's null.
-				ssrModule = importedModule.ssrModule ?? (await loader.import(importedModule.url));
-			} catch {
-				// The module may not be inline-able, e.g. SCSS partials. Skip it as it may already
-				// be inlined into other modules if it happens to be in the graph.
+			// In production, we can simply assign the styles as URLs
+			if (mode !== 'development') {
+				importedCssUrls.add(importedModule.url);
 				continue;
 			}
-			if (
-				mode === 'development' && // only inline in development
-				typeof ssrModule?.default === 'string' // ignore JS module styles
-			) {
-				importedStylesMap.set(importedModule.url, {
-					id: importedModule.id ?? importedModule.url,
-					url: importedModule.url,
-					content: ssrModule.default,
-				});
-			} else {
-				// NOTE: We use the `url` property here. `id` would break Windows.
-				importedCssUrls.add(importedModule.url);
+
+			// In dev, we inline all styles if possible
+			let css = '';
+			// If this is a plain CSS module, the default export should be a string
+			if (typeof importedModule.ssrModule?.default === 'string') {
+				css = importedModule.ssrModule.default;
 			}
+			// Else try to load it
+			else {
+				const url = new URL(importedModule.url, 'http://localhost');
+				// Mark url with ?inline so Vite will return the CSS as plain string, even for CSS modules
+				url.searchParams.set('inline', '');
+				const modId = `${decodeURI(url.pathname)}${url.search}`;
+
+				try {
+					// The SSR module is possibly not loaded. Load it if it's null.
+					const ssrModule = await loader.import(modId);
+					css = ssrModule.default;
+				} catch {
+					// Some CSS modules, e.g. from Vue files, may not work with the ?inline query.
+					// If so, we fallback to a url instead
+					if (modId.includes('.module.')) {
+						importedCssUrls.add(importedModule.url);
+					}
+					// The module may not be inline-able, e.g. SCSS partials. Skip it as it may already
+					// be inlined into other modules if it happens to be in the graph.
+					continue;
+				}
+			}
+
+			importedStylesMap.set(importedModule.url, {
+				id: importedModule.id ?? importedModule.url,
+				url: importedModule.url,
+				content: css,
+			});
 		}
 	}
 

diff --git a/packages/astro/test/0-css.test.js b/packages/astro/test/0-css.test.js
@@ -367,7 +367,7 @@ describe('CSS', function () {
 				'ReactModules.module.sass',
 			];
 			for (const style of styles) {
-				const href = $(`link[href$="${style}"]`).attr('href');
+				const href = $(`style[data-vite-dev-id$="${style}"]`).attr('data-vite-dev-id');
 				expect((await fixture.fetch(href)).status, style).to.equal(200);
 			}
 
@@ -423,7 +423,7 @@ describe('CSS', function () {
 
 		it('.module.css ordering', () => {
 			const globalStyleTag = $('style[data-vite-dev-id$="default.css"]');
-			const moduleStyleTag = $('link[href$="ModuleOrdering.module.css"]');
+			const moduleStyleTag = $('style[data-vite-dev-id$="ModuleOrdering.module.css"]');
 			const globalStyleClassIndex = globalStyleTag.index();
 			const moduleStyleClassIndex = moduleStyleTag.index();
 			// css module has higher priority than global style

diff --git a/packages/astro/test/units/dev/styles.test.js b/packages/astro/test/units/dev/styles.test.js
@@ -15,6 +15,17 @@ class TestLoader {
 	getModulesByFile(id) {
 		return this.modules.has(id) ? [this.modules.get(id)] : [];
 	}
+	import(id) {
+		// try to normalize inline CSS requests so we can map to the existing modules value
+		id = id.replace(/(\?|&)inline=?(&|$)/, (_, start, end) => (end ? start : '')).replace(/=$/, '');
+		for (const mod of this.modules.values()) {
+			for (const importedMod of mod.importedModules) {
+				if (importedMod.id === id) {
+					return importedMod.ssrModule;
+				}
+			}
+		}
+	}
 }
 
 describe('Crawling graph for CSS', () => {
@@ -35,7 +46,7 @@ describe('Crawling graph for CSS', () => {
 						id: indexId + '?astro&style.css',
 						url: indexId + '?astro&style.css',
 						importers: new Set([{ id: indexId }]),
-						ssrModule: {},
+						ssrModule: { default: '.index {}' },
 					},
 				],
 				importers: new Set(),
@@ -50,7 +61,7 @@ describe('Crawling graph for CSS', () => {
 						id: aboutId + '?astro&style.css',
 						url: aboutId + '?astro&style.css',
 						importers: new Set([{ id: aboutId }]),
-						ssrModule: {},
+						ssrModule: { default: '.about {}' },
 					},
 				],
 				importers: new Set(),
@@ -69,6 +80,6 @@ describe('Crawling graph for CSS', () => {
 			loader,
 			'development'
 		);
-		expect(res.urls.size).to.equal(1);
+		expect(res.styles.length).to.equal(1);
 	});
 });