From 7522bb4914f2f9e8b8f3c743bc9c941fd3aca644 Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Thu, 14 Sep 2023 18:22:16 +0800 Subject: [PATCH] Improve markdown rendering performance (#8532) --- .changeset/clever-parents-do.md | 5 + .changeset/shaggy-actors-cheat.md | 5 + .../astro/src/vite-plugin-markdown/index.ts | 52 +++--- .../remark/src/frontmatter-injection.ts | 7 + packages/markdown/remark/src/index.ts | 159 ++++++++++++------ packages/markdown/remark/src/internal.ts | 1 + packages/markdown/remark/src/load-plugins.ts | 2 +- packages/markdown/remark/src/types.ts | 22 ++- .../markdown/remark/test/autolinking.test.js | 22 +-- .../markdown/remark/test/entities.test.js | 12 +- packages/markdown/remark/test/plugins.test.js | 12 +- .../remark/test/remark-collect-images.test.js | 31 ++-- packages/markdown/remark/test/test-utils.js | 4 - 13 files changed, 209 insertions(+), 125 deletions(-) create mode 100644 .changeset/clever-parents-do.md create mode 100644 .changeset/shaggy-actors-cheat.md delete mode 100644 packages/markdown/remark/test/test-utils.js diff --git a/.changeset/clever-parents-do.md b/.changeset/clever-parents-do.md new file mode 100644 index 000000000..32be44dc8 --- /dev/null +++ b/.changeset/clever-parents-do.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Improve markdown rendering performance by sharing processor instance diff --git a/.changeset/shaggy-actors-cheat.md b/.changeset/shaggy-actors-cheat.md new file mode 100644 index 000000000..18fe5a775 --- /dev/null +++ b/.changeset/shaggy-actors-cheat.md @@ -0,0 +1,5 @@ +--- +'@astrojs/markdown-remark': minor +--- + +Export `createMarkdownProcessor` and deprecate `renderMarkdown` API diff --git a/packages/astro/src/vite-plugin-markdown/index.ts b/packages/astro/src/vite-plugin-markdown/index.ts index 41cf08e42..163baab0d 100644 --- a/packages/astro/src/vite-plugin-markdown/index.ts +++ b/packages/astro/src/vite-plugin-markdown/index.ts @@ -1,8 +1,8 @@ -import { renderMarkdown } from '@astrojs/markdown-remark'; import { + createMarkdownProcessor, InvalidAstroDataError, - safelyGetAstroData, -} from '@astrojs/markdown-remark/dist/internal.js'; + type MarkdownProcessor, +} from '@astrojs/markdown-remark'; import matter from 'gray-matter'; import fs from 'node:fs'; import path from 'node:path'; @@ -57,9 +57,14 @@ const astroErrorModulePath = normalizePath( ); export default function markdown({ settings, logger }: AstroPluginOptions): Plugin { + let processor: MarkdownProcessor; + return { enforce: 'pre', name: 'astro:markdown', + async buildStart() { + processor = await createMarkdownProcessor(settings.config.markdown); + }, // Why not the "transform" hook instead of "load" + readFile? // A: Vite transforms all "import.meta.env" references to their values before // passing to the transform hook. This lets us get the truly raw value @@ -70,33 +75,32 @@ export default function markdown({ settings, logger }: AstroPluginOptions): Plug const rawFile = await fs.promises.readFile(fileId, 'utf-8'); const raw = safeMatter(rawFile, id); - const renderResult = await renderMarkdown(raw.content, { - ...settings.config.markdown, - fileURL: new URL(`file://${fileId}`), - frontmatter: raw.data, - }); + const renderResult = await processor + .render(raw.content, { + fileURL: new URL(`file://${fileId}`), + frontmatter: raw.data, + }) + .catch((err) => { + // Improve error message for invalid astro data + if (err instanceof InvalidAstroDataError) { + throw new AstroError(AstroErrorData.InvalidFrontmatterInjectionError); + } + throw err; + }); let html = renderResult.code; - const { headings } = renderResult.metadata; + const { headings, imagePaths: rawImagePaths, frontmatter } = renderResult.metadata; // Resolve all the extracted images from the content - let imagePaths: { raw: string; resolved: string }[] = []; - if (renderResult.vfile.data.imagePaths) { - for (let imagePath of renderResult.vfile.data.imagePaths.values()) { - imagePaths.push({ - raw: imagePath, - resolved: - (await this.resolve(imagePath, id))?.id ?? path.join(path.dirname(id), imagePath), - }); - } + const imagePaths: { raw: string; resolved: string }[] = []; + for (const imagePath of rawImagePaths.values()) { + imagePaths.push({ + raw: imagePath, + resolved: + (await this.resolve(imagePath, id))?.id ?? path.join(path.dirname(id), imagePath), + }); } - const astroData = safelyGetAstroData(renderResult.vfile.data); - if (astroData instanceof InvalidAstroDataError) { - throw new AstroError(AstroErrorData.InvalidFrontmatterInjectionError); - } - - const { frontmatter } = astroData; const { layout } = frontmatter; if (frontmatter.setup) { diff --git a/packages/markdown/remark/src/frontmatter-injection.ts b/packages/markdown/remark/src/frontmatter-injection.ts index db1a2b704..4f5118ece 100644 --- a/packages/markdown/remark/src/frontmatter-injection.ts +++ b/packages/markdown/remark/src/frontmatter-injection.ts @@ -27,6 +27,13 @@ export function safelyGetAstroData(vfileData: Data): MarkdownAstroData | Invalid return astro; } +export function setAstroData(vfileData: Data, astroData: MarkdownAstroData) { + vfileData.astro = astroData; +} + +/** + * @deprecated Use `setAstroData` instead + */ export function toRemarkInitializeAstroData({ userFrontmatter, }: { diff --git a/packages/markdown/remark/src/index.ts b/packages/markdown/remark/src/index.ts index d81d1702e..41d08ec9a 100644 --- a/packages/markdown/remark/src/index.ts +++ b/packages/markdown/remark/src/index.ts @@ -1,11 +1,16 @@ import type { AstroMarkdownOptions, + MarkdownProcessor, MarkdownRenderingOptions, MarkdownRenderingResult, MarkdownVFile, } from './types.js'; -import { toRemarkInitializeAstroData } from './frontmatter-injection.js'; +import { + InvalidAstroDataError, + safelyGetAstroData, + setAstroData, +} from './frontmatter-injection.js'; import { loadPlugins } from './load-plugins.js'; import { rehypeHeadingIds } from './rehype-collect-headings.js'; import { remarkCollectImages } from './remark-collect-images.js'; @@ -15,13 +20,14 @@ import { remarkShiki } from './remark-shiki.js'; import rehypeRaw from 'rehype-raw'; import rehypeStringify from 'rehype-stringify'; import remarkGfm from 'remark-gfm'; -import markdown from 'remark-parse'; -import markdownToHtml from 'remark-rehype'; +import remarkParse from 'remark-parse'; +import remarkRehype from 'remark-rehype'; import remarkSmartypants from 'remark-smartypants'; import { unified } from 'unified'; import { VFile } from 'vfile'; import { rehypeImages } from './rehype-images.js'; +export { InvalidAstroDataError } from './frontmatter-injection.js'; export { rehypeHeadingIds } from './rehype-collect-headings.js'; export { remarkCollectImages } from './remark-collect-images.js'; export { remarkPrism } from './remark-prism.js'; @@ -45,30 +51,29 @@ export const markdownConfigDefaults: Omit, 'draft // Skip nonessential plugins during performance benchmark runs const isPerformanceBenchmark = Boolean(process.env.ASTRO_PERFORMANCE_BENCHMARK); -/** Shared utility for rendering markdown */ -export async function renderMarkdown( - content: string, - opts: MarkdownRenderingOptions -): Promise { - let { - fileURL, +/** + * Create a markdown preprocessor to render multiple markdown files + */ +export async function createMarkdownProcessor( + opts?: AstroMarkdownOptions +): Promise { + const { syntaxHighlight = markdownConfigDefaults.syntaxHighlight, shikiConfig = markdownConfigDefaults.shikiConfig, remarkPlugins = markdownConfigDefaults.remarkPlugins, rehypePlugins = markdownConfigDefaults.rehypePlugins, - remarkRehype = markdownConfigDefaults.remarkRehype, + remarkRehype: remarkRehypeOptions = markdownConfigDefaults.remarkRehype, gfm = markdownConfigDefaults.gfm, smartypants = markdownConfigDefaults.smartypants, - frontmatter: userFrontmatter = {}, - } = opts; - const input = new VFile({ value: content, path: fileURL }); + } = opts ?? {}; - let parser = unified() - .use(markdown) - .use(toRemarkInitializeAstroData({ userFrontmatter })) - .use([]); + const loadedRemarkPlugins = await Promise.all(loadPlugins(remarkPlugins)); + const loadedRehypePlugins = await Promise.all(loadPlugins(rehypePlugins)); - if (!isPerformanceBenchmark && gfm) { + const parser = unified().use(remarkParse); + + // gfm and smartypants + if (!isPerformanceBenchmark) { if (gfm) { parser.use(remarkGfm); } @@ -77,14 +82,13 @@ export async function renderMarkdown( } } - const loadedRemarkPlugins = await Promise.all(loadPlugins(remarkPlugins)); - const loadedRehypePlugins = await Promise.all(loadPlugins(rehypePlugins)); - - loadedRemarkPlugins.forEach(([plugin, pluginOpts]) => { - parser.use([[plugin, pluginOpts]]); - }); + // User remark plugins + for (const [plugin, pluginOpts] of loadedRemarkPlugins) { + parser.use(plugin, pluginOpts); + } if (!isPerformanceBenchmark) { + // Syntax highlighting if (syntaxHighlight === 'shiki') { parser.use(remarkShiki, shikiConfig); } else if (syntaxHighlight === 'prism') { @@ -95,45 +99,88 @@ export async function renderMarkdown( parser.use(remarkCollectImages); } - parser.use([ - [ - markdownToHtml as any, - { - allowDangerousHtml: true, - passThrough: [], - ...remarkRehype, - }, - ], - ]); - - loadedRehypePlugins.forEach(([plugin, pluginOpts]) => { - parser.use([[plugin, pluginOpts]]); + // Remark -> Rehype + parser.use(remarkRehype as any, { + allowDangerousHtml: true, + passThrough: [], + ...remarkRehypeOptions, }); + // User rehype plugins + for (const [plugin, pluginOpts] of loadedRehypePlugins) { + parser.use(plugin, pluginOpts); + } + + // Images / Assets support parser.use(rehypeImages()); + + // Headings if (!isPerformanceBenchmark) { - parser.use([rehypeHeadingIds]); + parser.use(rehypeHeadingIds); } - parser.use([rehypeRaw]).use(rehypeStringify, { allowDangerousHtml: true }); + // Stringify to HTML + parser.use(rehypeRaw).use(rehypeStringify, { allowDangerousHtml: true }); - let vfile: MarkdownVFile; - try { - vfile = await parser.process(input); - } catch (err) { - // Ensure that the error message contains the input filename - // to make it easier for the user to fix the issue - err = prefixError(err, `Failed to parse Markdown file "${input.path}"`); - // eslint-disable-next-line no-console - console.error(err); - throw err; - } - - const headings = vfile?.data.__astroHeadings || []; return { - metadata: { headings, source: content, html: String(vfile.value) }, - code: String(vfile.value), - vfile, + async render(content, renderOpts) { + const vfile = new VFile({ value: content, path: renderOpts?.fileURL }); + setAstroData(vfile.data, { frontmatter: renderOpts?.frontmatter ?? {} }); + + const result: MarkdownVFile = await parser.process(vfile).catch((err) => { + // Ensure that the error message contains the input filename + // to make it easier for the user to fix the issue + err = prefixError(err, `Failed to parse Markdown file "${vfile.path}"`); + // eslint-disable-next-line no-console + console.error(err); + throw err; + }); + + const astroData = safelyGetAstroData(result.data); + if (astroData instanceof InvalidAstroDataError) { + throw astroData; + } + + return { + code: String(result.value), + metadata: { + headings: result.data.__astroHeadings ?? [], + imagePaths: result.data.imagePaths ?? new Set(), + frontmatter: astroData.frontmatter ?? {}, + }, + // Compat for `renderMarkdown` only. Do not use! + __renderMarkdownCompat: { + result, + }, + }; + }, + }; +} + +/** + * Shared utility for rendering markdown + * + * @deprecated Use `createMarkdownProcessor` instead for better performance + */ +export async function renderMarkdown( + content: string, + opts: MarkdownRenderingOptions +): Promise { + const processor = await createMarkdownProcessor(opts); + + const result = await processor.render(content, { + fileURL: opts.fileURL, + frontmatter: opts.frontmatter, + }); + + return { + code: result.code, + metadata: { + headings: result.metadata.headings, + source: content, + html: result.code, + }, + vfile: (result as any).__renderMarkdownCompat.result, }; } diff --git a/packages/markdown/remark/src/internal.ts b/packages/markdown/remark/src/internal.ts index 0ab7e34bb..a0f344a3a 100644 --- a/packages/markdown/remark/src/internal.ts +++ b/packages/markdown/remark/src/internal.ts @@ -1,5 +1,6 @@ export { InvalidAstroDataError, safelyGetAstroData, + setAstroData, toRemarkInitializeAstroData, } from './frontmatter-injection.js'; diff --git a/packages/markdown/remark/src/load-plugins.ts b/packages/markdown/remark/src/load-plugins.ts index de6601e7d..8229ddff2 100644 --- a/packages/markdown/remark/src/load-plugins.ts +++ b/packages/markdown/remark/src/load-plugins.ts @@ -14,7 +14,7 @@ async function importPlugin(p: string | unified.Plugin): Promise } catch {} // Try import from user project - const resolved = await importMetaResolve(p, cwdUrlStr); + const resolved = importMetaResolve(p, cwdUrlStr); const importResult = await import(resolved); return importResult.default; } diff --git a/packages/markdown/remark/src/types.ts b/packages/markdown/remark/src/types.ts index caeebec93..bcab97041 100644 --- a/packages/markdown/remark/src/types.ts +++ b/packages/markdown/remark/src/types.ts @@ -58,13 +58,33 @@ export interface ImageMetadata { type: string; } -export interface MarkdownRenderingOptions extends AstroMarkdownOptions { +export interface MarkdownProcessor { + render: ( + content: string, + opts?: MarkdownProcessorRenderOptions + ) => Promise; +} + +export interface MarkdownProcessorRenderOptions { /** @internal */ fileURL?: URL; /** Used for frontmatter injection plugins */ frontmatter?: Record; } +export interface MarkdownProcessorRenderResult { + code: string; + metadata: { + headings: MarkdownHeading[]; + imagePaths: Set; + frontmatter: Record; + }; +} + +export interface MarkdownRenderingOptions + extends AstroMarkdownOptions, + MarkdownProcessorRenderOptions {} + export interface MarkdownHeading { depth: number; slug: string; diff --git a/packages/markdown/remark/test/autolinking.test.js b/packages/markdown/remark/test/autolinking.test.js index b1e567bb4..79d3ea767 100644 --- a/packages/markdown/remark/test/autolinking.test.js +++ b/packages/markdown/remark/test/autolinking.test.js @@ -1,14 +1,12 @@ -import { renderMarkdown } from '../dist/index.js'; +import { createMarkdownProcessor } from '../dist/index.js'; import chai from 'chai'; -import { mockRenderMarkdownParams } from './test-utils.js'; describe('autolinking', () => { - describe('plain md', () => { + describe('plain md', async () => { + const processor = await createMarkdownProcessor(); + it('autolinks URLs starting with a protocol in plain text', async () => { - const { code } = await renderMarkdown( - `See https://example.com for more.`, - mockRenderMarkdownParams - ); + const { code } = await processor.render(`See https://example.com for more.`); chai .expect(code.replace(/\n/g, '')) @@ -16,10 +14,7 @@ describe('autolinking', () => { }); it('autolinks URLs starting with "www." in plain text', async () => { - const { code } = await renderMarkdown( - `See www.example.com for more.`, - mockRenderMarkdownParams - ); + const { code } = await processor.render(`See www.example.com for more.`); chai .expect(code.trim()) @@ -27,9 +22,8 @@ describe('autolinking', () => { }); it('does not autolink URLs in code blocks', async () => { - const { code } = await renderMarkdown( - 'See `https://example.com` or `www.example.com` for more.', - mockRenderMarkdownParams + const { code } = await processor.render( + 'See `https://example.com` or `www.example.com` for more.' ); chai diff --git a/packages/markdown/remark/test/entities.test.js b/packages/markdown/remark/test/entities.test.js index acaf71be1..b2dacb79f 100644 --- a/packages/markdown/remark/test/entities.test.js +++ b/packages/markdown/remark/test/entities.test.js @@ -1,13 +1,11 @@ -import { renderMarkdown } from '../dist/index.js'; +import { createMarkdownProcessor } from '../dist/index.js'; import { expect } from 'chai'; -import { mockRenderMarkdownParams } from './test-utils.js'; -describe('entities', () => { +describe('entities', async () => { + const processor = await createMarkdownProcessor(); + it('should not unescape entities in regular Markdown', async () => { - const { code } = await renderMarkdown( - `<i>This should NOT be italic</i>`, - mockRenderMarkdownParams - ); + const { code } = await processor.render(`<i>This should NOT be italic</i>`); expect(code).to.equal(`

<i>This should NOT be italic</i>

`); }); diff --git a/packages/markdown/remark/test/plugins.test.js b/packages/markdown/remark/test/plugins.test.js index 35e5dcaf8..ce2401047 100644 --- a/packages/markdown/remark/test/plugins.test.js +++ b/packages/markdown/remark/test/plugins.test.js @@ -1,5 +1,4 @@ -import { renderMarkdown } from '../dist/index.js'; -import { mockRenderMarkdownParams } from './test-utils.js'; +import { createMarkdownProcessor } from '../dist/index.js'; import chai from 'chai'; import { fileURLToPath } from 'node:url'; @@ -8,9 +7,8 @@ describe('plugins', () => { // https://github.com/withastro/astro/issues/3264 it('should be able to get file path when passing fileURL', async () => { let context; - await renderMarkdown(`test`, { - ...mockRenderMarkdownParams, - fileURL: new URL('virtual.md', import.meta.url), + + const processor = await createMarkdownProcessor({ remarkPlugins: [ function () { const transformer = (tree, file) => { @@ -22,6 +20,10 @@ describe('plugins', () => { ], }); + await processor.render(`test`, { + fileURL: new URL('virtual.md', import.meta.url), + }); + chai.expect(typeof context).to.equal('object'); chai.expect(context.path).to.equal(fileURLToPath(new URL('virtual.md', import.meta.url))); }); diff --git a/packages/markdown/remark/test/remark-collect-images.test.js b/packages/markdown/remark/test/remark-collect-images.test.js index d5c743e20..a55336953 100644 --- a/packages/markdown/remark/test/remark-collect-images.test.js +++ b/packages/markdown/remark/test/remark-collect-images.test.js @@ -1,28 +1,33 @@ -import { renderMarkdown } from '../dist/index.js'; -import { mockRenderMarkdownParams } from './test-utils.js'; +import { createMarkdownProcessor } from '../dist/index.js'; import chai from 'chai'; -describe('collect images', () => { +describe('collect images', async () => { + const processor = await createMarkdownProcessor(); + it('should collect inline image paths', async () => { - const { code, vfile } = await renderMarkdown( - `Hello ![inline image url](./img.png)`, - mockRenderMarkdownParams - ); + const { + code, + metadata: { imagePaths }, + } = await processor.render(`Hello ![inline image url](./img.png)`, { + fileURL: 'file.md', + }); chai .expect(code) .to.equal('

Hello inline image url

'); - chai.expect(Array.from(vfile.data.imagePaths)).to.deep.equal(['./img.png']); + chai.expect(Array.from(imagePaths)).to.deep.equal(['./img.png']); }); it('should add image paths from definition', async () => { - const { code, vfile } = await renderMarkdown( - `Hello ![image ref][img-ref]\n\n[img-ref]: ./img.webp`, - mockRenderMarkdownParams - ); + const { + code, + metadata: { imagePaths }, + } = await processor.render(`Hello ![image ref][img-ref]\n\n[img-ref]: ./img.webp`, { + fileURL: 'file.md', + }); chai.expect(code).to.equal('

Hello image ref

'); - chai.expect(Array.from(vfile.data.imagePaths)).to.deep.equal(['./img.webp']); + chai.expect(Array.from(imagePaths)).to.deep.equal(['./img.webp']); }); }); diff --git a/packages/markdown/remark/test/test-utils.js b/packages/markdown/remark/test/test-utils.js deleted file mode 100644 index 76b593deb..000000000 --- a/packages/markdown/remark/test/test-utils.js +++ /dev/null @@ -1,4 +0,0 @@ -export const mockRenderMarkdownParams = { - fileURL: 'file.md', - contentDir: new URL('file:///src/content/'), -};