Fix components in markdown regressions (#3486)

Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
withastro · May 31, 2022 · 119ecf8 · 119ecf8
1 parent e02c72f
commit 119ecf8
Show file tree

Hide file tree

Showing 9 changed files with 171 additions and 47 deletions.
diff --git a/.changeset/large-berries-grow.md b/.changeset/large-berries-grow.md
@@ -0,0 +1,6 @@
+---
+'astro': patch
+'@astrojs/markdown-remark': patch
+---
+
+Fix components in markdown regressions
diff --git a/packages/astro/test/astro-markdown.test.js b/packages/astro/test/astro-markdown.test.js
@@ -82,7 +82,6 @@ describe('Astro Markdown', () => {
 	// 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'));
 	});
 
@@ -273,4 +272,42 @@ describe('Astro Markdown', () => {
 		expect($('code').eq(2).text()).to.contain('title: import.meta.env.TITLE');
 		expect($('blockquote').text()).to.contain('import.meta.env.TITLE');
 	});
+
+	it('Escapes HTML tags in code blocks', async () => {
+		const html = await fixture.readFile('/code-in-md/index.html');
+		const $ = cheerio.load(html);
+
+		expect($('code').eq(0).html()).to.equal('&lt;script&gt;');
+		expect($('blockquote').length).to.equal(1);
+		expect($('code').eq(1).html()).to.equal('&lt;/script&gt;');
+		expect($('pre').html()).to.contain('&gt;This should also work without any problems.&lt;');
+	});
+
+	it('Allows defining slot contents in component children', async () => {
+		const html = await fixture.readFile('/slots/index.html');
+		const $ = cheerio.load(html);
+
+		const slots = $('article').eq(0);
+		expect(slots.find('> .fragmentSlot > div').text()).to.contain('1:');
+		expect(slots.find('> .fragmentSlot > div + p').text()).to.contain('2:');
+		expect(slots.find('> .pSlot > p[title="hello"]').text()).to.contain('3:');
+		expect(slots.find('> .defaultSlot').text().replace(/\s+/g, ' ')).to.equal(`
+			4: Div in default slot
+			5: Paragraph in fragment in default slot
+			6: Regular text in default slot
+		`.replace(/\s+/g, ' '));
+
+		const nestedSlots = $('article').eq(1);
+		expect(nestedSlots.find('> .fragmentSlot').html()).to.contain('1:');
+		expect(nestedSlots.find('> .pSlot > p').text()).to.contain('2:');
+		expect(nestedSlots.find('> .defaultSlot > article').text().replace(/\s+/g, ' ')).to.equal(`
+			3: nested fragmentSlot
+			4: nested pSlot
+			5: nested text in default slot
+		`.replace(/\s+/g, ' '));
+
+		expect($('article').eq(3).text().replace(/[^❌]/g, '')).to.equal('❌❌❌');
+
+		expect($('article').eq(4).text().replace(/[^❌]/g, '')).to.equal('❌❌❌');
+	});
 });
diff --git a/packages/astro/test/fixtures/astro-markdown/src/components/SlotComponent.astro b/packages/astro/test/fixtures/astro-markdown/src/components/SlotComponent.astro
@@ -0,0 +1,13 @@
+<article>
+	<section class="fragmentSlot">
+		<slot name="fragmentSlot">❌ Missing content for slot "fragmentSlot"</slot>
+	</section>
+
+	<section class="pSlot">
+		<slot name="pSlot">❌ Missing content for slot "pSlot"</slot>
+	</section>
+
+	<section class="defaultSlot">
+		<slot>❌ Missing content for default slot</slot>
+	</section>
+</article>
diff --git a/packages/astro/test/fixtures/astro-markdown/src/pages/code-in-md.md b/packages/astro/test/fixtures/astro-markdown/src/pages/code-in-md.md
@@ -0,0 +1,16 @@
+# Inline code blocks
+
+`<script>` tags in **Astro** components are now built,
+bundled and optimized by default.
+
+> Markdown formatting still works between tags in inline code blocks.
+
+We can also use closing `</script>` tags without any problems.
+
+# Fenced code blocks
+
+```html
+<body>
+	<div>This should also work without any problems.</div>
+</body>
+```
diff --git a/packages/astro/test/fixtures/astro-markdown/src/pages/slots.md b/packages/astro/test/fixtures/astro-markdown/src/pages/slots.md
@@ -0,0 +1,38 @@
+---
+layout: ../layouts/content.astro
+setup: import SlotComponent from '../components/SlotComponent.astro';
+---
+
+# Component with slot contents in children
+
+<SlotComponent>
+	<div>4: Div in default slot</div>
+	<Fragment slot="fragmentSlot">
+		<div>1: Div in fragmentSlot</div>
+		<p>2: Paragraph in fragmentSlot</p>
+	</Fragment>
+	<Fragment><p>5: Paragraph in fragment in default slot</p></Fragment>
+	6: Regular text in default slot
+	<p slot="pSlot" title="hello">3: p with title as pSlot</p>
+</SlotComponent>
+
+# Component with nested component in children
+
+<SlotComponent>
+	<p slot="pSlot">2: pSlot</p>
+	<SlotComponent>
+		<p slot="pSlot">4: nested pSlot</p>
+		5: nested text in default slot
+		<Fragment slot="fragmentSlot">3: nested fragmentSlot</Fragment>
+	</SlotComponent>
+	<Fragment slot="fragmentSlot">1: fragmentSlot</Fragment>
+</SlotComponent>
+
+# Missing content due to empty children
+
+<SlotComponent>
+</SlotComponent>
+
+# Missing content due to self-closing tag
+
+<SlotComponent/>
diff --git a/packages/markdown/remark/src/rehype-escape.ts b/packages/markdown/remark/src/rehype-escape.ts
@@ -5,6 +5,11 @@ export default function rehypeEscape(): any {
 		return visit(node, 'element', (el) => {
 			if (el.tagName === 'code' || el.tagName === 'pre') {
 				el.properties['is:raw'] = true;
+				// Visit all raw children and escape HTML tags to prevent Markdown code
+				// like "This is a `<script>` tag" from actually opening a script tag
+				visit(el, 'raw', (raw) => {
+					raw.value = raw.value.replace(/</g, '&lt;').replace(/>/g, '&gt;');
+				});
 			}
 			return el;
 		});

diff --git a/packages/markdown/remark/src/rehype-jsx.ts b/packages/markdown/remark/src/rehype-jsx.ts
@@ -1,51 +1,49 @@
-import { map } from 'unist-util-map';
+import { visit } from 'unist-util-visit';
 
-const MDX_ELEMENTS = new Set(['mdxJsxFlowElement', 'mdxJsxTextElement']);
+const MDX_ELEMENTS = ['mdxJsxFlowElement', 'mdxJsxTextElement'];
 export default function rehypeJsx(): any {
 	return function (node: any): any {
-		return map(node, (child: any) => {
-			if (child.type === 'element') {
-				return { ...child, tagName: `${child.tagName}` };
-			}
-			if (MDX_ELEMENTS.has(child.type)) {
-				const attrs = child.attributes.reduce((acc: any[], entry: any) => {
-					let attr = entry.value;
-					if (attr && typeof attr === 'object') {
-						attr = `{${attr.value}}`;
-					} else if (attr && entry.type === 'mdxJsxExpressionAttribute') {
-						attr = `{${attr}}`;
-					} else if (attr === null) {
-						attr = '';
-					} else if (typeof attr === 'string') {
-						attr = `"${attr}"`;
-					}
-					if (!entry.name) {
-						return acc + ` ${attr}`;
-					}
-					return acc + ` ${entry.name}${attr ? '=' : ''}${attr}`;
-				}, '');
-
-				if (child.children.length === 0) {
-					return {
-						type: 'raw',
-						value: `<${child.name}${attrs} />`,
-					};
+		visit(node, 'element', (child: any) => {
+			child.tagName = `${child.tagName}`;
+		});
+		visit(node, MDX_ELEMENTS, (child: any, index: number | null, parent: any) => {
+			if (index === null || !Boolean(parent))
+				return;
+
+			const attrs = child.attributes.reduce((acc: any[], entry: any) => {
+				let attr = entry.value;
+				if (attr && typeof attr === 'object') {
+					attr = `{${attr.value}}`;
+				} else if (attr && entry.type === 'mdxJsxExpressionAttribute') {
+					attr = `{${attr}}`;
+				} else if (attr === null) {
+					attr = '';
+				} else if (typeof attr === 'string') {
+					attr = `"${attr}"`;
 				}
-				child.children.splice(0, 0, {
-					type: 'raw',
-					value: `\n<${child.name}${attrs}>`,
-				});
-				child.children.push({
-					type: 'raw',
-					value: `</${child.name}>\n`,
-				});
-				return {
-					...child,
-					type: 'element',
-					tagName: `Fragment`,
-				};
+				if (!entry.name) {
+					return acc + ` ${attr}`;
+				}
+				return acc + ` ${entry.name}${attr ? '=' : ''}${attr}`;
+			}, '');
+
+			if (child.children.length === 0) {
+				child.type = 'raw';
+				child.value = `<${child.name}${attrs} />`;
+				return;
 			}
-			return child;
+
+			// Replace the current child node with its children
+			// wrapped by raw opening and closing tags
+			const openingTag = {
+				type: 'raw',
+				value: `\n<${child.name}${attrs}>`,
+			};
+			const closingTag = {
+				type: 'raw',
+				value: `</${child.name}>\n`,
+			};
+			parent.children.splice(index, 1, openingTag, ...child.children, closingTag);
 		});
 	};
 }
diff --git a/packages/markdown/remark/test/components.test.js b/packages/markdown/remark/test/components.test.js
@@ -51,7 +51,18 @@ describe('components', () => {
 
 		chai
 			.expect(code)
-			.to.equal(`<Fragment>\n<Component bool={true}>Hello world!</Component>\n</Fragment>`);
+			.to.equal(`\n<Component bool={true}>Hello world!</Component>\n`);
+	});
+
+	it('should be able to nest components', async () => {
+		const { code } = await renderMarkdown(
+			`<Component bool={true}><Component>Hello world!</Component></Component>`,
+			{}
+		);
+
+		chai
+			.expect(code)
+			.to.equal(`\n<Component bool={true}>\n<Component>Hello world!</Component>\n</Component>\n`);
 	});
 
 	it('should allow markdown without many spaces', async () => {
@@ -65,7 +76,7 @@ describe('components', () => {
 		chai
 			.expect(code)
 			.to.equal(
-				`<Fragment>\n<Component><h1 id="hello-world">Hello world!</h1></Component>\n</Fragment>`
+				`\n<Component><h1 id="hello-world">Hello world!</h1></Component>\n`
 			);
 	});
 });
diff --git a/packages/markdown/remark/test/expressions.test.js b/packages/markdown/remark/test/expressions.test.js
@@ -11,7 +11,7 @@ describe('expressions', () => {
 	it('should be able to serialize expression inside component', async () => {
 		const { code } = await renderMarkdown(`<Component>{a}</Component>`, {});
 
-		chai.expect(code).to.equal(`<Fragment>\n<Component>{a}</Component>\n</Fragment>`);
+		chai.expect(code).to.equal(`\n<Component>{a}</Component>\n`);
 	});
 
 	it('should be able to serialize expression inside markdown', async () => {