From 7f7a8504b5c2df4c99d3025931860c0d50992510 Mon Sep 17 00:00:00 2001 From: Erika <3019731+Princesseuh@users.noreply.github.com> Date: Wed, 22 Mar 2023 12:19:01 +0100 Subject: [PATCH] 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 --- .changeset/blue-swans-teach.md | 6 ++ .../astro/src/vite-plugin-markdown/index.ts | 102 +++++------------- packages/astro/test/core-image.test.js | 29 ++++- packages/markdown/remark/src/index.ts | 4 +- packages/markdown/remark/src/rehype-images.ts | 47 +------- .../remark/src/remark-collect-images.ts | 17 +-- packages/markdown/remark/src/types.ts | 4 - 7 files changed, 65 insertions(+), 144 deletions(-) create mode 100644 .changeset/blue-swans-teach.md diff --git a/.changeset/blue-swans-teach.md b/.changeset/blue-swans-teach.md new file mode 100644 index 000000000000..87e955ef59f3 --- /dev/null +++ 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 index c1fade2d97c3..7f99d9e1ba73 100644 --- 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(); - - 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(); - - 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 index eb84e12506eb..ccbd5b2f83ff 100644 --- 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 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'); + + // 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 index ffae6be4f73e..f37b9ed68143 100644 --- 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 index 1d615c8b51bc..72f4757008d5 100644 --- a/packages/markdown/remark/src/rehype-images.ts +++ b/packages/markdown/remark/src/rehype-images.ts @@ -1,14 +1,10 @@ -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; @@ -16,39 +12,8 @@ export function rehypeImages(imageService: any, assetsDir: URL | undefined, getI 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 index bb6a6ddb025d..afc61c468dc5 100644 --- 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) | 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(); - 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 index 1b88f2442519..dc3bbce32ff8 100644 --- 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; experimentalAssets?: boolean; - imageService?: any; - assetsDir?: URL; - resolveImage?: (path: string) => Promise; - getImageMetadata?: any; } export interface MarkdownHeading {