Fix using optimized images in Markdown (#6604)

* fix(images): Fix using optimized images in Markdown * test(images): Update tests to be a bit more robust + new tests * chore: changeset * refactor: use spreadAttributes instead
withastro · Mar 22, 2023 · 7f7a850 · 7f7a850
1 parent cc1831c
commit 7f7a850
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 144 deletions.
diff --git a/.changeset/blue-swans-teach.md b/.changeset/blue-swans-teach.md
@@ -0,0 +1,6 @@
+---
+'astro': patch
+'@astrojs/markdown-remark': patch
+---
+
+Fix using optimized images in Markdown not working
diff --git a/packages/astro/src/vite-plugin-markdown/index.ts b/packages/astro/src/vite-plugin-markdown/index.ts
@@ -5,16 +5,10 @@ import {
 } from '@astrojs/markdown-remark/dist/internal.js';
 import fs from 'fs';
 import matter from 'gray-matter';
-import npath from 'node:path';
 import { fileURLToPath } from 'node:url';
-import type { PluginContext } from 'rollup';
-import { pathToFileURL } from 'url';
 import type { Plugin } from 'vite';
 import { normalizePath } from 'vite';
 import type { AstroSettings } from '../@types/astro';
-import { imageMetadata } from '../assets/index.js';
-import type { ImageService } from '../assets/services/service';
-import imageSize from '../assets/vendor/image-size/index.js';
 import { AstroError, AstroErrorData, MarkdownError } from '../core/errors/index.js';
 import type { LogOptions } from '../core/logger/core.js';
 import { warn } from '../core/logger/core.js';
@@ -60,29 +54,11 @@ const astroJsxRuntimeModulePath = normalizePath(
 	fileURLToPath(new URL('../jsx-runtime/index.js', import.meta.url))
 );
 
-export default function markdown({ settings, logging }: AstroPluginOptions): Plugin {
-	const markdownAssetMap = new Map<string, string>();
-
-	let imageService: ImageService | undefined = undefined;
-
-	async function resolveImage(this: PluginContext, fileId: string, path: string) {
-		const resolved = await this.resolve(path, fileId);
-		if (!resolved) return path;
-		const rel = npath.relative(normalizePath(fileURLToPath(settings.config.root)), resolved.id);
-		const buffer = await fs.promises.readFile(resolved.id);
-		// This conditional has to be here, to prevent race conditions on setting the map
-		if (markdownAssetMap.has(resolved.id)) {
-			return `ASTRO_ASSET_MD_${markdownAssetMap.get(resolved.id)!}`;
-		}
-		const file = this.emitFile({
-			type: 'asset',
-			name: rel,
-			source: buffer,
-		});
-		markdownAssetMap.set(resolved.id, file);
-		return `ASTRO_ASSET_MD_${file}`;
-	}
+const astroServerRuntimeModulePath = normalizePath(
+	fileURLToPath(new URL('../runtime/server/index.js', import.meta.url))
+);
 
+export default function markdown({ settings, logging }: AstroPluginOptions): Plugin {
 	return {
 		enforce: 'pre',
 		name: 'astro:markdown',
@@ -96,30 +72,24 @@ export default function markdown({ settings, logging }: AstroPluginOptions): Plu
 				const rawFile = await fs.promises.readFile(fileId, 'utf-8');
 				const raw = safeMatter(rawFile, id);
 
-				if (settings.config.experimental.assets) {
-					imageService = (await import(settings.config.image.service)).default;
-				}
 				const renderResult = await renderMarkdown(raw.content, {
 					...settings.config.markdown,
 					fileURL: new URL(`file://${fileId}`),
 					frontmatter: raw.data,
 					experimentalAssets: settings.config.experimental.assets,
-					imageService,
-					assetsDir: new URL('./assets/', settings.config.srcDir),
-					resolveImage: this.meta.watchMode ? undefined : resolveImage.bind(this, fileId),
-					getImageMetadata: imageSize,
 				});
 
-				this;
-
 				let html = renderResult.code;
 				const { headings } = renderResult.metadata;
-				let imagePaths: string[] = [];
+				let imagePaths: { raw: string; absolute: string }[] = [];
 				if (settings.config.experimental.assets) {
 					let paths = (renderResult.vfile.data.imagePaths as string[]) ?? [];
 					imagePaths = await Promise.all(
 						paths.map(async (imagePath) => {
-							return (await this.resolve(imagePath))?.id ?? imagePath;
+							return {
+								raw: imagePath,
+								absolute: (await this.resolve(imagePath, id))?.id ?? imagePath,
+							};
 						})
 					);
 				}
@@ -142,18 +112,26 @@ export default function markdown({ settings, logging }: AstroPluginOptions): Plu
 
 				const code = escapeViteEnvReferences(`
 				import { Fragment, jsx as h } from ${JSON.stringify(astroJsxRuntimeModulePath)};
+				import { spreadAttributes } from ${JSON.stringify(astroServerRuntimeModulePath)};
+
 				${layout ? `import Layout from ${JSON.stringify(layout)};` : ''}
-				${
-					settings.config.experimental.assets
-						? 'import { getConfiguredImageService } from "astro:assets";\ngetConfiguredImageService();'
-						: ''
+				${settings.config.experimental.assets ? 'import { getImage } from "astro:assets";' : ''}
+
+				export const images = {
+					${imagePaths.map(
+						(entry) =>
+							`'${entry.raw}': await getImage({src: (await import("${entry.absolute}")).default})`
+					)}
 				}
 
-				const images = {
-					${imagePaths.map((entry) => `'${entry}': await import('${entry}')`)}
+				function updateImageReferences(html) {
+					return html.replaceAll(
+						/__ASTRO_IMAGE_=\"(.+)\"/gm,
+						(full, imagePath) => spreadAttributes({src: images[imagePath].src, ...images[imagePath].attributes})
+					);
 				}
 
-				const html = ${JSON.stringify(html)};
+				const html = updateImageReferences(${JSON.stringify(html)});
 
 				export const frontmatter = ${JSON.stringify(frontmatter)};
 				export const file = ${JSON.stringify(fileId)};
@@ -209,37 +187,5 @@ export default function markdown({ settings, logging }: AstroPluginOptions): Plu
 				};
 			}
 		},
-		async generateBundle(_opts, bundle) {
-			for (const [, output] of Object.entries(bundle)) {
-				if (output.type === 'asset') continue;
-
-				if (markdownAssetMap.size) {
-					const optimizedPaths = new Map<string, string>();
-
-					for (const [filename, hash] of markdownAssetMap) {
-						const image = await imageMetadata(pathToFileURL(filename));
-						if (!image) {
-							continue;
-						}
-						const fileName = this.getFileName(hash);
-						image.src = npath.join(settings.config.base, fileName);
-
-						// TODO: This part recreates code we already have for content collection and normal ESM imports.
-						// It might be possible to refactor so it also uses `emitESMImage`? - erika, 2023-03-15
-						const options = { src: image };
-						const validatedOptions = imageService?.validateOptions
-							? imageService.validateOptions(options)
-							: options;
-
-						const optimized = globalThis.astroAsset.addStaticImage!(validatedOptions);
-						optimizedPaths.set(hash, optimized);
-					}
-					output.code = output.code.replaceAll(/ASTRO_ASSET_MD_([0-9a-z]{8})/gm, (_str, hash) => {
-						const optimizedName = optimizedPaths.get(hash);
-						return optimizedName || this.getFileName(hash);
-					});
-				}
-			}
-		},
 	};
 }
diff --git a/packages/astro/test/core-image.test.js b/packages/astro/test/core-image.test.js
@@ -191,7 +191,11 @@ describe('astro:image', () => {
 			it('Adds the <img> tag', () => {
 				let $img = $('img');
 				expect($img).to.have.a.lengthOf(1);
-				expect($img.attr('src').startsWith('/_image')).to.equal(true);
+
+				// Verbose test for the full URL to make sure the image went through the full pipeline
+				expect($img.attr('src')).to.equal(
+					'/_image?href=%2Fsrc%2Fassets%2Fpenguin1.jpg%3ForigWidth%3D207%26origHeight%3D243%26origFormat%3Djpg&f=webp'
+				);
 			});
 
 			it('has width and height attributes', () => {
@@ -330,6 +334,21 @@ describe('astro:image', () => {
 			expect(data).to.be.an.instanceOf(Buffer);
 		});
 
+		it('markdown images are written', async () => {
+			const html = await fixture.readFile('/post/index.html');
+			const $ = cheerio.load(html);
+			let $img = $('img');
+
+			// <img> tag
+			expect($img).to.have.a.lengthOf(1);
+			expect($img.attr('alt')).to.equal('My article cover');
+
+			// image itself
+			const src = $img.attr('src');
+			const data = await fixture.readFile(src, null);
+			expect(data).to.be.an.instanceOf(Buffer);
+		});
+
 		it('aliased images are written', async () => {
 			const html = await fixture.readFile('/alias/index.html');
 
@@ -459,5 +478,13 @@ describe('astro:image', () => {
 			const $ = cheerio.load(html);
 			expect($('#local img').attr('data-service')).to.equal('my-custom-service');
 		});
+
+		it('custom service works in Markdown', async () => {
+			const response = await fixture.fetch('/post');
+			const html = await response.text();
+
+			const $ = cheerio.load(html);
+			expect($('img').attr('data-service')).to.equal('my-custom-service');
+		});
 	});
 });
diff --git a/packages/markdown/remark/src/index.ts b/packages/markdown/remark/src/index.ts
@@ -96,7 +96,7 @@ export async function renderMarkdown(
 
 		if (opts.experimentalAssets) {
 			// Apply later in case user plugins resolve relative image paths
-			parser.use([toRemarkCollectImages(opts.resolveImage)]);
+			parser.use([toRemarkCollectImages()]);
 		}
 	}
 
@@ -116,7 +116,7 @@ export async function renderMarkdown(
 	});
 
 	if (opts.experimentalAssets) {
-		parser.use(rehypeImages(await opts.imageService, opts.assetsDir, opts.getImageMetadata));
+		parser.use(rehypeImages());
 	}
 	if (!isPerformanceBenchmark) {
 		parser.use([rehypeHeadingIds]);

diff --git a/packages/markdown/remark/src/rehype-images.ts b/packages/markdown/remark/src/rehype-images.ts
@@ -1,54 +1,19 @@
-import { join as pathJoin } from 'node:path';
-import { fileURLToPath } from 'node:url';
 import { visit } from 'unist-util-visit';
-import { pathToFileURL } from 'url';
-import type { ImageMetadata, MarkdownVFile } from './types.js';
+import type { MarkdownVFile } from './types.js';
 
-export function rehypeImages(imageService: any, assetsDir: URL | undefined, getImageMetadata: any) {
+export function rehypeImages() {
 	return () =>
 		function (tree: any, file: MarkdownVFile) {
 			visit(tree, (node) => {
-				if (!assetsDir) return;
 				if (node.type !== 'element') return;
 				if (node.tagName !== 'img') return;
 
 				if (node.properties?.src) {
 					if (file.dirname) {
 						if (!isRelativePath(node.properties.src) && !isAliasedPath(node.properties.src)) return;
 
-						let fileURL: URL;
-						if (isAliasedPath(node.properties.src)) {
-							fileURL = new URL(stripAliasPath(node.properties.src), assetsDir);
-						} else {
-							fileURL = pathToFileURL(pathJoin(file.dirname, node.properties.src));
-						}
-
-						const fileData = getImageMetadata!(fileURLToPath(fileURL)) as ImageMetadata;
-						fileURL.searchParams.append('origWidth', fileData.width.toString());
-						fileURL.searchParams.append('origHeight', fileData.height.toString());
-						fileURL.searchParams.append('origFormat', fileData.type.toString());
-
-						let options = {
-							src: {
-								src: fileURL,
-								width: fileData.width,
-								height: fileData.height,
-								format: fileData.type,
-							},
-							alt: node.properties.alt,
-						};
-
-						const validatedOptions = imageService.validateOptions
-							? imageService.validateOptions(options)
-							: options;
-
-						const imageURL = imageService.getURL(validatedOptions);
-						node.properties = Object.assign(node.properties, {
-							src: imageURL,
-							...(imageService.getHTMLAttributes !== undefined
-								? imageService.getHTMLAttributes(validatedOptions)
-								: {}),
-						});
+						node.properties['__ASTRO_IMAGE_'] = node.properties.src;
+						delete node.properties.src;
 					}
 				}
 			});
@@ -59,10 +24,6 @@ function isAliasedPath(path: string) {
 	return path.startsWith('~/assets');
 }
 
-function stripAliasPath(path: string) {
-	return path.replace('~/assets/', '');
-}
-
 function isRelativePath(path: string) {
 	return startsWithDotDotSlash(path) || startsWithDotSlash(path);
 }

diff --git a/packages/markdown/remark/src/remark-collect-images.ts b/packages/markdown/remark/src/remark-collect-images.ts
@@ -2,9 +2,7 @@ import type { Image } from 'mdast';
 import { visit } from 'unist-util-visit';
 import type { VFile } from 'vfile';
 
-type OptionalResolveImage = ((path: string) => Promise<string>) | undefined;
-
-export default function toRemarkCollectImages(resolveImage: OptionalResolveImage) {
+export default function toRemarkCollectImages() {
 	return () =>
 		async function (tree: any, vfile: VFile) {
 			if (typeof vfile?.path !== 'string') return;
@@ -13,19 +11,6 @@ export default function toRemarkCollectImages(resolveImage: OptionalResolveImage
 			visit(tree, 'image', function raiseError(node: Image) {
 				imagePaths.add(node.url);
 			});
-			if (imagePaths.size === 0) {
-				vfile.data.imagePaths = [];
-				return;
-			} else if (resolveImage) {
-				const mapping = new Map<string, string>();
-				for (const path of Array.from(imagePaths)) {
-					const id = await resolveImage(path);
-					mapping.set(path, id);
-				}
-				visit(tree, 'image', function raiseError(node: Image) {
-					node.url = mapping.get(node.url)!;
-				});
-			}
 
 			vfile.data.imagePaths = Array.from(imagePaths);
 		};

diff --git a/packages/markdown/remark/src/types.ts b/packages/markdown/remark/src/types.ts
@@ -68,10 +68,6 @@ export interface MarkdownRenderingOptions extends AstroMarkdownOptions {
 	/** Used for frontmatter injection plugins */
 	frontmatter?: Record<string, any>;
 	experimentalAssets?: boolean;
-	imageService?: any;
-	assetsDir?: URL;
-	resolveImage?: (path: string) => Promise<string>;
-	getImageMetadata?: any;
 }
 
 export interface MarkdownHeading {