Fix: component styles within imported markdown files (#3116)

* fix: replace markdown path prefix with suffix flag

* fix: avoid non-encoded colons for flag

* fix: remove needless ?

* fix: dev server load order

* fix: production build crawl dynamic imports

* fix: remove  unused virtual_module_id const

* fix: remove unsafe "!" on getmodbyid

* fix: remove needless @id path check

* fix: add list of SSR-able file extensions

* docs: virtual_mod_id change

* fix: support id prefix on resolved ids

* fix: switch to ?mdImport flag to resolve glob imports

* tests: imported md styles for dev and build

* chore: changeset

.changeset/tidy-poems-occur.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fix: Astro components used in dynamically imported markdown (ex. Astro.glob('\*.md') will now retain their CSS styles in dev and production builds

packages/astro/src/core/render/dev/css.ts

@@ -4,12 +4,21 @@ import path from 'path';
 import { unwrapId, viteID } from '../../util.js';
 import { STYLE_EXTENSIONS } from '../util.js';
 
+/**
+ * List of file extensions signalling we can (and should) SSR ahead-of-time
+ * See usage below
+ */
+const fileExtensionsToSSR = new Set(['.md']);
+
 /** Given a filePath URL, crawl Vite’s module graph to find all style imports. */
-export function getStylesForURL(filePath: URL, viteServer: vite.ViteDevServer): Set<string> {
+export async function getStylesForURL(
+	filePath: URL,
+	viteServer: vite.ViteDevServer
+): Promise<Set<string>> {
 	const importedCssUrls = new Set<string>();
 
 	/** recursively crawl the module graph to get all style files imported by parent id */
-	function crawlCSS(_id: string, isFile: boolean, scanned = new Set<string>()) {
+	async function crawlCSS(_id: string, isFile: boolean, scanned = new Set<string>()) {
 		const id = unwrapId(_id);
 		const importedModules = new Set<vite.ModuleNode>();
 		const moduleEntriesForId = isFile
@@ -32,6 +41,16 @@ export function getStylesForURL(filePath: URL, viteServer: vite.ViteDevServer):
 			if (id === entry.id) {
 				scanned.add(id);
 				for (const importedModule of entry.importedModules) {
+					// some dynamically imported modules are *not* server rendered in time
+					// to only SSR modules that we can safely transform, we check against
+					// a list of file extensions based on our built-in vite plugins
+					if (importedModule.id) {
+						// use URL to strip special query params like "?content"
+						const { pathname } = new URL(`file://${importedModule.id}`);
+						if (fileExtensionsToSSR.has(path.extname(pathname))) {
+							await viteServer.ssrLoadModule(importedModule.id);
+						}
+					}
 					importedModules.add(importedModule);
 				}
 			}
@@ -48,11 +67,11 @@ export function getStylesForURL(filePath: URL, viteServer: vite.ViteDevServer):
 				// NOTE: We use the `url` property here. `id` would break Windows.
 				importedCssUrls.add(importedModule.url);
 			}
-			crawlCSS(importedModule.id, false, scanned);
+			await crawlCSS(importedModule.id, false, scanned);
 		}
 	}
 
 	// Crawl your import graph for CSS files, populating `importedCssUrls` as a result.
-	crawlCSS(viteID(filePath), true);
+	await crawlCSS(viteID(filePath), true);
 	return importedCssUrls;
 }

packages/astro/src/core/render/dev/index.ts

@@ -139,7 +139,7 @@ export async function render(
 	// Pass framework CSS in as link tags to be appended to the page.
 	let links = new Set<SSRElement>();
 	if (!isLegacyBuild) {
-		[...getStylesForURL(filePath, viteServer)].forEach((href) => {
+		[...(await getStylesForURL(filePath, viteServer))].forEach((href) => {
 			if (mode === 'development' && svelteStylesRE.test(href)) {
 				scripts.add({
 					props: { type: 'module', src: href },
@@ -211,7 +211,7 @@ export async function render(
 
 	// inject CSS
 	if (isLegacyBuild) {
-		[...getStylesForURL(filePath, viteServer)].forEach((href) => {
+		[...(await getStylesForURL(filePath, viteServer))].forEach((href) => {
 			if (mode === 'development' && svelteStylesRE.test(href)) {
 				tags.push({
 					tag: 'script',

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

@@ -74,7 +74,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin {
 
 		const info = ctx.getModuleInfo(id);
 		if (info) {
-			for (const importedId of info.importedIds) {
+			for (const importedId of [...info.importedIds, ...info.dynamicallyImportedIds]) {
 				if (!seen.has(importedId) && !isRawOrUrlModule(importedId)) {
 					yield* walkStyles(ctx, importedId, seen);
 				}

packages/astro/src/vite-plugin-markdown/index.ts

@@ -16,8 +16,8 @@ interface AstroPluginOptions {
 	config: AstroConfig;
 }
 
-const VIRTUAL_MODULE_ID_PREFIX = 'astro:markdown';
+const MARKDOWN_IMPORT_FLAG = '?mdImport';
-const VIRTUAL_MODULE_ID = '\0' + VIRTUAL_MODULE_ID_PREFIX;
+const MARKDOWN_CONTENT_FLAG = '?content';
 
 // TODO: Clean up some of the shared logic between this Markdown plugin and the Astro plugin.
 // Both end up connecting a `load()` hook to the Astro compiler, and share some copy-paste
@@ -53,16 +53,12 @@ export default function markdown({ config }: AstroPluginOptions): Plugin {
 		name: 'astro:markdown',
 		enforce: 'pre',
 		async resolveId(id, importer, options) {
-			// Resolve virtual modules as-is.
-			if (id.startsWith(VIRTUAL_MODULE_ID)) {
-				return id;
-			}
 			// Resolve any .md files with the `?content` cache buster. This should only come from
 			// an already-resolved JS module wrapper. Needed to prevent infinite loops in Vite.
 			// Unclear if this is expected or if cache busting is just working around a Vite bug.
-			if (id.endsWith('.md?content')) {
+			if (id.endsWith(`.md${MARKDOWN_CONTENT_FLAG}`)) {
 				const resolvedId = await this.resolve(id, importer, { skipSelf: true, ...options });
-				return resolvedId?.id.replace('?content', '');
+				return resolvedId?.id.replace(MARKDOWN_CONTENT_FLAG, '');
 			}
 			// If the markdown file is imported from another file via ESM, resolve a JS representation
 			// that defers the markdown -> HTML rendering until it is needed. This is especially useful
@@ -71,7 +67,7 @@ export default function markdown({ config }: AstroPluginOptions): Plugin {
 			if (id.endsWith('.md') && !isRootImport(importer)) {
 				const resolvedId = await this.resolve(id, importer, { skipSelf: true, ...options });
 				if (resolvedId) {
-					return VIRTUAL_MODULE_ID + resolvedId.id;
+					return resolvedId.id + MARKDOWN_IMPORT_FLAG;
 				}
 			}
 			// In all other cases, we do nothing and rely on normal Vite resolution.
@@ -81,11 +77,11 @@ export default function markdown({ config }: AstroPluginOptions): Plugin {
 			// A markdown file has been imported via ESM!
 			// Return the file's JS representation, including all Markdown
 			// frontmatter and a deferred `import() of the compiled markdown content.
-			if (id.startsWith(VIRTUAL_MODULE_ID)) {
+			if (id.endsWith(`.md${MARKDOWN_IMPORT_FLAG}`)) {
 				const sitePathname = config.site
 					? appendForwardSlash(new URL(config.base, config.site).pathname)
 					: '/';
-				const fileId = id.substring(VIRTUAL_MODULE_ID.length);
+				const fileId = id.replace(MARKDOWN_IMPORT_FLAG, '');
 				const fileUrl = fileId.includes('/pages/')
 					? fileId.replace(/^.*\/pages\//, sitePathname).replace(/(\/index)?\.md$/, '')
 					: undefined;
@@ -100,7 +96,7 @@ export default function markdown({ config }: AstroPluginOptions): Plugin {
 						
 						// Deferred
 						export default async function load() {
-							return (await import(${JSON.stringify(fileId + '?content')}));
+							return (await import(${JSON.stringify(fileId + MARKDOWN_CONTENT_FLAG)}));
 						};
 						export function Content(...args) {
 							return load().then((m) => m.default(...args))

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

@@ -0,0 +1,59 @@
+import { expect } from 'chai';
+import cheerio from 'cheerio';
+import { loadFixture } from './test-utils.js';
+
+let fixture;
+const IMPORTED_ASTRO_COMPONENT_ID = 'imported-astro-component'
+
+describe('Imported markdown CSS', function () {
+	before(async () => {
+		fixture = await loadFixture({ root: './fixtures/astro-markdown-css/' });
+	});
+	describe('build', () => {
+		let $;
+		let bundledCSS;
+
+		before(async () => {
+			this.timeout(45000); // test needs a little more time in CI
+			await fixture.build();
+
+			// get bundled CSS (will be hashed, hence DOM query)
+			const html = await fixture.readFile('/index.html');
+			$ = cheerio.load(html);
+			const bundledCSSHREF = $('link[rel=stylesheet][href^=/assets/]').attr('href');
+			bundledCSS = await fixture.readFile(bundledCSSHREF.replace(/^\/?/, '/'));
+		});
+
+		it('Compiles styles for Astro components within imported markdown', () => {
+			const importedAstroComponent = $(`#${IMPORTED_ASTRO_COMPONENT_ID}`)?.[0]
+			expect(importedAstroComponent?.name).to.equal('h2')
+			const cssClass = $(importedAstroComponent).attr('class')?.split(/\s+/)?.[0]
+
+			expect(bundledCSS).to.match(new RegExp(`h2.${cssClass}{color:#00f}`))
+		});
+	});
+	describe('dev', () => {
+		let devServer;
+		let $;
+
+		before(async () => {
+			devServer = await fixture.startDevServer();
+			const html = await fixture.fetch('/').then((res) => res.text());
+			$ = cheerio.load(html);
+		});
+		
+		after(async () => {
+			await devServer.stop();
+		});
+		
+		it('Compiles styles for Astro components within imported markdown', async () => {
+			const importedAstroComponent = $(`#${IMPORTED_ASTRO_COMPONENT_ID}`)?.[0]
+			expect(importedAstroComponent?.name).to.equal('h2')
+			const cssClass = $(importedAstroComponent).attr('class')?.split(/\s+/)?.[0]
+
+			const astroCSSHREF = $('link[rel=stylesheet][href^=/src/components/Visual.astro]').attr('href');
+			const css = await fixture.fetch(astroCSSHREF.replace(/^\/?/, '/')).then((res) => res.text());
+			expect(css).to.match(new RegExp(`h2.${cssClass}{color:#00f}`));
+		});
+	});
+});

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

@@ -0,0 +1,6 @@
+import { defineConfig } from 'astro/config';
+
+// https://astro.build/config
+export default defineConfig({
+  integrations: []
+});

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

@@ -0,0 +1,12 @@
+{
+  "name": "@test/astro-markdown-css",
+  "version": "0.0.0",
+  "private": true,
+  "scripts": {
+    "build": "astro build",
+    "dev": "astro dev"
+  },
+  "dependencies": {
+    "astro": "workspace:*"
+  }
+}

packages/astro/test/fixtures/astro-markdown-css/src/components/Visual.astro

@@ -0,0 +1,7 @@
+<h2 id="imported-astro-component">I'm a visual!</h2>
+
+<style>
+	h2 {
+		color: #00f;
+	}
+</style>

packages/astro/test/fixtures/astro-markdown-css/src/markdown/article.md

@@ -0,0 +1,9 @@
+---
+setup: import Visual from '../components/Visual.astro'
+---
+
+# Example markdown document, with a Visual
+
+<Visual />
+<Visual />
+<Visual />

packages/astro/test/fixtures/astro-markdown-css/src/markdown/article2.md

@@ -0,0 +1,9 @@
+---
+setup: import Visual from '../components/Visual.astro'
+---
+
+# Example markdown document, with a more Visuals
+
+<Visual />
+<Visual />
+<Visual />

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

@@ -0,0 +1,15 @@
+---
+const markdownDocs = await Astro.glob('../markdown/*.md')
+const article2 = await import('../markdown/article2.md')
+---
+<html lang="en">
+	<head>
+		<meta charset="utf-8" />
+		<meta name="viewport" content="width=device-width" />
+		<title>Astro</title>
+	</head>
+	<body>
+		{markdownDocs.map(markdownDoc => <><h2>{markdownDoc.url}</h2><markdownDoc.Content /></>)}
+		<article2.Content />
+	</body>
+</html>

pnpm-lock.yaml

@@ -800,6 +800,12 @@ importers:
       '@astrojs/preact': link:../../../../integrations/preact
       astro: link:../../..
 
+  packages/astro/test/fixtures/astro-markdown-css:
+    specifiers:
+      astro: workspace:*
+    dependencies:
+      astro: link:../../..
+
   packages/astro/test/fixtures/astro-markdown-drafts:
     specifiers:
       astro: workspace:*