Improve Markdown + Components usage (#3410)

* feat: use internal MDX tooling for markdown + components

* fix: improve MD + component tests

* chore: add changeset

* fix: make tsc happy

* fix(#3319): add regression test for component children

* fix(markdown): support HTML comments in markdown

* fix(#2474): ensure namespaced components are properly handled in markdown pages

* fix(#3220): ensure html in markdown pages does not have extra surrounding space

* fix(#3264): ensure that remark files pass in file information

* fix(#3254): enable experimentalStaticExtraction for `.md` pages

* fix: revert parsing change

* fix: remove `markdown.mode` option

.changeset/wicked-adults-pull.md

@@ -0,0 +1,6 @@
+---
+'astro': patch
+'@astrojs/markdown-remark': minor
+---
+
+Significantally more stable behavior for "Markdown + Components" usage, which now handles component serialization much more similarly to MDX. Also supports switching between Components and Markdown without extra newlines, removes wrapping `<p>` tags from standalone components, and improves JSX expression handling.

packages/astro/src/@types/astro.ts

@@ -513,16 +513,6 @@ export interface AstroUserConfig {
 		 */
 		drafts?: boolean;
 
-		/**
-		 * @docs
-		 * @name markdown.mode
-		 * @type {'md' | 'mdx'}
-		 * @default `mdx`
-		 * @description
-		 * Control wheater to allow components inside markdown files ('mdx') or not ('md').
-		 */
-		mode?: 'md' | 'mdx';
-
 		/**
 		 * @docs
 		 * @name markdown.shikiConfig

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

@@ -84,7 +84,7 @@ export default function markdown({ config }: AstroPluginOptions): Plugin {
 				const source = await fs.promises.readFile(fileId, 'utf8');
 				const { data: frontmatter } = matter(source);
 				return {
-					code: `   
+					code: `
 						// Static
 						export const frontmatter = ${JSON.stringify(frontmatter)};
 						export const file = ${JSON.stringify(fileId)};
@@ -122,12 +122,17 @@ export default function markdown({ config }: AstroPluginOptions): Plugin {
 				const hasInjectedScript = isPage && config._ctx.scripts.some((s) => s.stage === 'page-ssr');
 
 				// Extract special frontmatter keys
-				const { data: frontmatter, content: markdownContent } = matter(source);
-				let renderResult = await renderMarkdown(markdownContent, renderOpts);
+				let { data: frontmatter, content: markdownContent } = matter(source);
+
+				// Turn HTML comments into JS comments
+				markdownContent = markdownContent.replace(/<\s*!--([^-->]*)(.*?)-->/gs, (whole) => `{/*${whole}*/}`)
+
+				let renderResult = await renderMarkdown(markdownContent, { ...renderOpts, fileURL: fileUrl } as any);
 				let { code: astroResult, metadata } = renderResult;
 				const { layout = '', components = '', setup = '', ...content } = frontmatter;
 				content.astro = metadata;
 				const prelude = `---
+import { slug as $$slug } from '@astrojs/markdown-remark';
 ${layout ? `import Layout from '${layout}';` : ''}
 ${components ? `import * from '${components}';` : ''}
 ${hasInjectedScript ? `import '${PAGE_SSR_SCRIPT_ID}';` : ''}
@@ -151,6 +156,8 @@ ${setup}`.trim();
 					site: config.site ? new URL(config.base, config.site).toString() : undefined,
 					sourcefile: id,
 					sourcemap: 'inline',
+					// TODO: baseline flag
+					experimentalStaticExtraction: true,
 					internalURL: `/@fs${prependForwardSlash(
 						viteID(new URL('../runtime/server/index.js', import.meta.url))
 					)}`,

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

@@ -28,12 +28,57 @@ describe('Astro Markdown', () => {
 		const $ = cheerio.load(html);
 
 		expect($('h2').html()).to.equal('Blog Post with JSX expressions');
-		expect($('p').first().html()).to.equal('JSX at the start of the line!');
+
+		expect(html).to.contain('JSX at the start of the line!');
 		for (let listItem of ['test-1', 'test-2', 'test-3']) {
-			expect($(`#${listItem}`).html()).to.equal(`\n${listItem}\n`);
+			expect($(`#${listItem}`).html()).to.equal(`${listItem}`);
 		}
 	});
 
+	it('Can handle slugs with JSX expressions in markdown pages', async () => {
+		const html = await fixture.readFile('/slug/index.html');
+		const $ = cheerio.load(html);
+
+		expect($('h1').attr("id")).to.equal('my-blog-post');
+	});
+
+	it('Can handle code elements without extra spacing', async () => {
+		const html = await fixture.readFile('/code-element/index.html');
+		const $ = cheerio.load(html);
+
+		$('code').each((_, el) => {
+			expect($(el).html()).to.equal($(el).html().trim())
+		});
+	});
+
+	it('Can handle namespaced components in markdown', async () => {
+		const html = await fixture.readFile('/namespace/index.html');
+		const $ = cheerio.load(html);
+
+		expect($('h1').text()).to.equal('Hello Namespace!');
+		expect($('button').length).to.equal(1);
+	});
+
+	it('Correctly handles component children in markdown pages (#3319)', async () => {
+		const html = await fixture.readFile('/children/index.html');
+
+		expect(html).not.to.contain('<p></p>');
+	});
+
+	it('Can handle HTML comments in markdown pages', async () => {
+		const html = await fixture.readFile('/comment/index.html');
+		const $ = cheerio.load(html);
+
+		expect($('h1').text()).to.equal('It works!');
+	});
+
+	// https://github.com/withastro/astro/issues/3254
+	it('Can handle scripts in markdown pages', async () => {
+		const html = await fixture.readFile('/script/index.html');
+		console.log(html);
+		expect(html).not.to.match(new RegExp("\/src\/scripts\/test\.js"));
+	});
+
 	it('Can load more complex jsxy stuff', async () => {
 		const html = await fixture.readFile('/complex/index.html');
 		const $ = cheerio.load(html);

packages/astro/test/fixtures/astro-markdown/src/components/TextBlock.jsx

@@ -0,0 +1,20 @@
+import { h } from 'preact';
+
+const TextBlock = ({
+  title,
+  children,
+  noPadding = false,
+}) => {
+  return (
+    <div
+      className={`${
+        noPadding ? "" : "md:px-2 lg:px-4"
+      } flex-1 prose prose-headings:font-grotesk`}
+    >
+      <h3>{title}</h3>
+      <p>{children}</p>
+    </div>
+  );
+};
+
+export default TextBlock;

packages/astro/test/fixtures/astro-markdown/src/components/index.js

@@ -0,0 +1,5 @@
+import Counter from './Counter';
+
+export default {
+	Counter
+}

packages/astro/test/fixtures/astro-markdown/src/content/code-element.md

@@ -0,0 +1,3 @@
+This should have `nospace` around it.
+
+This should have <code class="custom-class">nospace</code> around it.

packages/astro/test/fixtures/astro-markdown/src/pages/children.md

@@ -0,0 +1,12 @@
+---
+setup: import TextBlock from '../components/TextBlock'
+---
+{/* https://github.com/withastro/astro/issues/3319 */}
+
+<TextBlock title="Hello world!" noPadding>
+  <ul class="not-prose">
+    <li>A</li>
+    <li>B</li>
+    <li>C</li>
+  </ul>
+</TextBlock>

packages/astro/test/fixtures/astro-markdown/src/pages/code-element.astro

@@ -0,0 +1,7 @@
+---
+const content = await Astro.glob('../content/*.md');
+---
+
+<div>
+	{content.map(({ Content }) => <Content />)}
+</div>

packages/astro/test/fixtures/astro-markdown/src/pages/comment.md

@@ -0,0 +1,2 @@
+<!-- HTML comments! -->
+# It works!

packages/astro/test/fixtures/astro-markdown/src/pages/jsx-expressions.md

@@ -8,4 +8,6 @@ list: ['test-1', 'test-2', 'test-3']
 
 {frontmatter.paragraph}
 
-{frontmatter.list.map(item => <p id={item}>{item}</p>)}
+<ul>
+  {frontmatter.list.map(item => <li id={item}>{item}</li>)}
+</ul>

packages/astro/test/fixtures/astro-markdown/src/pages/namespace.md

@@ -0,0 +1,7 @@
+---
+setup: import ns from '../components/index.js';
+---
+
+# Hello Namespace!
+
+<ns.Counter>Click me!</ns.Counter>

packages/astro/test/fixtures/astro-markdown/src/pages/script.md

@@ -0,0 +1,7 @@
+# Test
+
+## Let's try a script...
+
+This should work!
+
+<script src="/src/scripts/test.js" />

packages/astro/test/fixtures/astro-markdown/src/pages/slug.md

@@ -0,0 +1,7 @@
+---
+title: My Blog Post
+---
+
+# {frontmatter.title}
+
+Hello world

packages/astro/test/fixtures/astro-markdown/src/scripts/test.js

@@ -0,0 +1 @@
+console.log("Hello world");

packages/markdown/remark/package.json

@@ -20,7 +20,8 @@
     "build": "astro-scripts build \"src/**/*.ts\" && tsc -p tsconfig.json",
     "build:ci": "astro-scripts build \"src/**/*.ts\"",
     "postbuild": "astro-scripts copy \"src/**/*.js\"",
-    "dev": "astro-scripts dev \"src/**/*.ts\""
+    "dev": "astro-scripts dev \"src/**/*.ts\"",
+    "test": "mocha --exit --timeout 20000"
   },
   "dependencies": {
     "@astrojs/prism": "^0.4.1",
@@ -30,6 +31,7 @@
     "mdast-util-mdx-jsx": "^1.2.0",
     "mdast-util-to-string": "^3.1.0",
     "micromark-extension-mdx-jsx": "^1.0.3",
+    "micromark-extension-mdxjs": "^1.0.0",
     "prismjs": "^1.28.0",
     "rehype-raw": "^6.1.1",
     "rehype-stringify": "^9.0.3",
@@ -40,14 +42,19 @@
     "shiki": "^0.10.1",
     "unified": "^10.1.2",
     "unist-util-map": "^3.1.1",
-    "unist-util-visit": "^4.1.0"
+    "unist-util-visit": "^4.1.0",
+    "vfile": "^5.3.2"
   },
   "devDependencies": {
+    "@types/chai": "^4.3.1",
     "@types/github-slugger": "^1.3.0",
     "@types/hast": "^2.3.4",
     "@types/mdast": "^3.0.10",
+    "@types/mocha": "^9.1.1",
     "@types/prismjs": "^1.26.0",
     "@types/unist": "^2.0.6",
-    "astro-scripts": "workspace:*"
+    "astro-scripts": "workspace:*",
+    "chai": "^4.3.6",
+    "mocha": "^9.2.2"
   }
 }

packages/markdown/remark/src/index.ts

@@ -2,10 +2,10 @@ import type { MarkdownRenderingOptions, MarkdownRenderingResult } from './types'
 
 import createCollectHeaders from './rehype-collect-headers.js';
 import scopedStyles from './remark-scoped-styles.js';
-import { remarkExpressions, loadRemarkExpressions } from './remark-expressions.js';
 import rehypeExpressions from './rehype-expressions.js';
 import rehypeIslands from './rehype-islands.js';
-import { remarkJsx, loadRemarkJsx } from './remark-jsx.js';
+import remarkMdxish from './remark-mdxish.js';
+import remarkMarkAndUnravel from './remark-mark-and-unravel.js';
 import rehypeJsx from './rehype-jsx.js';
 import rehypeEscape from './rehype-escape.js';
 import remarkPrism from './remark-prism.js';
@@ -18,27 +18,33 @@ import markdown from 'remark-parse';
 import markdownToHtml from 'remark-rehype';
 import rehypeStringify from 'rehype-stringify';
 import rehypeRaw from 'rehype-raw';
+import Slugger from 'github-slugger';
+import { VFile } from 'vfile';
 
 export * from './types.js';
 
 export const DEFAULT_REMARK_PLUGINS = ['remark-gfm', 'remark-smartypants'];
 export const DEFAULT_REHYPE_PLUGINS = [];
 
+const slugger = new Slugger();
+export function slug(value: string): string {
+	return slugger.slug(value);
+}
+
 /** Shared utility for rendering markdown */
 export async function renderMarkdown(
 	content: string,
-	opts: MarkdownRenderingOptions
+	opts: MarkdownRenderingOptions = {}
 ): Promise<MarkdownRenderingResult> {
-	let { mode, syntaxHighlight, shikiConfig, remarkPlugins, rehypePlugins } = opts;
+	let { fileURL, mode = 'mdx', syntaxHighlight = 'shiki', shikiConfig = {}, remarkPlugins = [], rehypePlugins = [] } = opts;
+	const input = new VFile({ value: content, path: fileURL })
 	const scopedClassName = opts.$?.scopedClassName;
 	const isMDX = mode === 'mdx';
 	const { headers, rehypeCollectHeaders } = createCollectHeaders();
 
-	await Promise.all([loadRemarkExpressions(), loadRemarkJsx()]); // Vite bug: dynamically import() these because of CJS interop (this will cache)
-
 	let parser = unified()
 		.use(markdown)
-		.use(isMDX ? [remarkJsx, remarkExpressions] : [])
+		.use(isMDX ? [remarkMdxish, remarkMarkAndUnravel] : [])
 		.use([remarkUnwrap]);
 
 	if (remarkPlugins.length === 0 && rehypePlugins.length === 0) {
@@ -68,7 +74,13 @@ export async function renderMarkdown(
 			markdownToHtml as any,
 			{
 				allowDangerousHtml: true,
-				passThrough: ['raw', 'mdxTextExpression', 'mdxJsxTextElement', 'mdxJsxFlowElement'],
+				passThrough: [
+					'raw',
+					'mdxFlowExpression',
+					'mdxJsxFlowElement',
+					'mdxJsxTextElement',
+					'mdxTextExpression',
+				],
 			},
 		],
 	]);
@@ -87,7 +99,7 @@ export async function renderMarkdown(
 		const vfile = await parser
 			.use([rehypeCollectHeaders])
 			.use(rehypeStringify, { allowDangerousHtml: true })
-			.process(content);
+			.process(input);
 		result = vfile.toString();
 	} catch (err) {
 		console.error(err);

packages/markdown/remark/src/mdast-util-mdxish.ts

@@ -0,0 +1,18 @@
+import {
+  mdxExpressionFromMarkdown,
+  mdxExpressionToMarkdown
+} from 'mdast-util-mdx-expression'
+import {mdxJsxFromMarkdown, mdxJsxToMarkdown} from 'mdast-util-mdx-jsx'
+
+export function mdxFromMarkdown(): any {
+  return [mdxExpressionFromMarkdown, mdxJsxFromMarkdown]
+}
+
+export function mdxToMarkdown(): any {
+  return {
+    extensions: [
+      mdxExpressionToMarkdown,
+      mdxJsxToMarkdown,
+    ]
+  }
+}

packages/markdown/remark/src/rehype-collect-headers.ts

@@ -17,15 +17,38 @@ export default function createCollectHeaders() {
 				if (!level) return;
 				const depth = Number.parseInt(level);
 
+				let raw = '';
 				let text = '';
-
-				visit(node, 'text', (child) => {
-					text += child.value;
+				let isJSX = false;
+				visit(node, (child) => {
+					if (child.type === 'element') {
+						return;
+					}
+					if (child.type === 'raw') {
+						// HACK: serialized JSX from internal plugins, ignore these for slug
+						if (child.value.startsWith('\n<') || child.value.endsWith('>\n')) {
+							raw += child.value.replace(/^\n|\n$/g, '');
+							return;
+						}
+					}
+					if (child.type === 'text' || child.type === 'raw')  {
+						raw += child.value;
+						text += child.value;
+						isJSX = isJSX || child.value.includes('{');
+					}
 				});
 
+
 				node.properties = node.properties || {};
 				if (typeof node.properties.id !== 'string') {
-					node.properties.id = slugger.slug(text);
+					if (isJSX) {
+						// HACK: for ids that have JSX content, use $$slug helper to generate slug at runtime
+						node.properties.id = `$$slug(\`${text.replace(/\{/g, '${')}\`)`;
+						(node as any).type = 'raw';
+						(node as any).value = `<${node.tagName} id={${node.properties.id}}>${raw}</${node.tagName}>`;
+					} else {
+						node.properties.id = slugger.slug(text);
+					}
 				}
 
 				headers.push({ depth, slug: node.properties.id, text });